From 0c0880d83630203e82a44b24e70bd11831465263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20N=C3=BCcke?= Date: Sat, 26 Mar 2016 13:28:25 +0100 Subject: [PATCH 1/2] Properly erroring on ".." as path to FS methods. Closes #1708. --- src/main/scala/li/cil/oc/server/component/FileSystem.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/scala/li/cil/oc/server/component/FileSystem.scala b/src/main/scala/li/cil/oc/server/component/FileSystem.scala index 4c9f87fe4..ec43a01c1 100644 --- a/src/main/scala/li/cil/oc/server/component/FileSystem.scala +++ b/src/main/scala/li/cil/oc/server/component/FileSystem.scala @@ -320,7 +320,7 @@ class FileSystem(val fileSystem: IFileSystem, var label: Label, val host: Option private def clean(path: String) = { val result = com.google.common.io.Files.simplifyPath(path) - if (result.startsWith("../")) throw new FileNotFoundException(path) + if (result.startsWith("../") || result == "..") throw new FileNotFoundException(path) if (result == "/" || result == ".") "" else result } From 8087d6523a5c493e13e80ca55c11436d73c9c569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20N=C3=BCcke?= Date: Sat, 26 Mar 2016 14:00:04 +0100 Subject: [PATCH 2/2] Hardened unbuffered filesystems. Closes #1707. --- .../scala/li/cil/oc/server/fs/Buffered.scala | 4 +- .../server/fs/FileInputStreamFileSystem.scala | 10 ++--- .../fs/FileOutputStreamFileSystem.scala | 8 ++-- .../li/cil/oc/server/fs/FileSystem.scala | 41 ++++++++----------- .../oc/server/fs/InputStreamFileSystem.scala | 21 ++++++---- .../oc/server/fs/OutputStreamFileSystem.scala | 20 +++++---- .../cil/oc/server/fs/VirtualFileSystem.scala | 2 +- 7 files changed, 52 insertions(+), 54 deletions(-) diff --git a/src/main/scala/li/cil/oc/server/fs/Buffered.scala b/src/main/scala/li/cil/oc/server/fs/Buffered.scala index 1d29a032b..0c1ec256a 100644 --- a/src/main/scala/li/cil/oc/server/fs/Buffered.scala +++ b/src/main/scala/li/cil/oc/server/fs/Buffered.scala @@ -14,8 +14,6 @@ trait Buffered extends OutputStreamFileSystem { private val deletions = mutable.Map.empty[String, Long] - protected def isValidFilename(name: String) = true - // ----------------------------------------------------------------------- // override def delete(path: String) = { @@ -39,7 +37,7 @@ trait Buffered extends OutputStreamFileSystem { override def load(nbt: NBTTagCompound) = { def recurse(path: String, directory: io.File) { makeDirectory(path) - for (child <- directory.listFiles() if isValidFilename(child.getName)) { + for (child <- directory.listFiles() if FileSystem.isValidFilename(child.getName)) { val childPath = path + child.getName val childFile = new io.File(directory, child.getName) if (child.exists() && child.isDirectory && child.list() != null) { diff --git a/src/main/scala/li/cil/oc/server/fs/FileInputStreamFileSystem.scala b/src/main/scala/li/cil/oc/server/fs/FileInputStreamFileSystem.scala index 847091165..f9a6a46e7 100644 --- a/src/main/scala/li/cil/oc/server/fs/FileInputStreamFileSystem.scala +++ b/src/main/scala/li/cil/oc/server/fs/FileInputStreamFileSystem.scala @@ -23,18 +23,18 @@ trait FileInputStreamFileSystem extends InputStreamFileSystem { // ----------------------------------------------------------------------- // - override def exists(path: String) = new io.File(root, path).exists() + override def exists(path: String) = new io.File(root, FileSystem.validatePath(path)).exists() - override def size(path: String) = new io.File(root, path) match { + override def size(path: String) = new io.File(root, FileSystem.validatePath(path)) match { case file if file.isFile => file.length() case _ => 0L } - override def isDirectory(path: String) = new io.File(root, path).isDirectory + override def isDirectory(path: String) = new io.File(root, FileSystem.validatePath(path)).isDirectory - override def lastModified(path: String) = new io.File(root, path).lastModified + override def lastModified(path: String) = new io.File(root, FileSystem.validatePath(path)).lastModified - override def list(path: String) = new io.File(root, path) match { + override def list(path: String) = new io.File(root, FileSystem.validatePath(path)) match { case file if file.exists() && file.isFile => Array(file.getName) case directory if directory.exists() && directory.isDirectory && directory.list() != null => directory.listFiles().map(file => if (file.isDirectory) file.getName + "/" else file.getName) diff --git a/src/main/scala/li/cil/oc/server/fs/FileOutputStreamFileSystem.scala b/src/main/scala/li/cil/oc/server/fs/FileOutputStreamFileSystem.scala index 2ca23c721..a909e1c6c 100644 --- a/src/main/scala/li/cil/oc/server/fs/FileOutputStreamFileSystem.scala +++ b/src/main/scala/li/cil/oc/server/fs/FileOutputStreamFileSystem.scala @@ -14,15 +14,15 @@ trait FileOutputStreamFileSystem extends FileInputStreamFileSystem with OutputSt // ----------------------------------------------------------------------- // override def delete(path: String) = { - val file = new io.File(root, path) + val file = new io.File(root, FileSystem.validatePath(path)) file == root || file.delete() } - override def makeDirectory(path: String) = new io.File(root, path).mkdir() + override def makeDirectory(path: String) = new io.File(root, FileSystem.validatePath(path)).mkdir() - override def rename(from: String, to: String) = new io.File(root, from).renameTo(new io.File(root, to)) + override def rename(from: String, to: String) = new io.File(root, FileSystem.validatePath(from)).renameTo(new io.File(root, FileSystem.validatePath(to))) - override def setLastModified(path: String, time: Long) = new io.File(root, path).setLastModified(time) + override def setLastModified(path: String, time: Long) = new io.File(root, FileSystem.validatePath(path)).setLastModified(time) // ----------------------------------------------------------------------- // diff --git a/src/main/scala/li/cil/oc/server/fs/FileSystem.scala b/src/main/scala/li/cil/oc/server/fs/FileSystem.scala index e826939d7..151cc3bb3 100755 --- a/src/main/scala/li/cil/oc/server/fs/FileSystem.scala +++ b/src/main/scala/li/cil/oc/server/fs/FileSystem.scala @@ -9,9 +9,8 @@ import java.util.UUID import li.cil.oc.OpenComputers import li.cil.oc.Settings import li.cil.oc.api -import li.cil.oc.api.network.EnvironmentHost import li.cil.oc.api.fs.Label -import li.cil.oc.api.fs.Mode +import li.cil.oc.api.network.EnvironmentHost import li.cil.oc.integration.Mods import li.cil.oc.integration.computercraft.DriverComputerCraftMedia import li.cil.oc.server.component @@ -44,6 +43,20 @@ object FileSystem extends api.detail.FileSystemAPI { true }) + // Worst-case: we're on Windows or using a FAT32 partition mounted in *nix. + // Note: we allow / as the path separator and expect all \s to be converted + // accordingly before the path is passed to the file system. + private val invalidChars = """\:*?"<>|""".toSet + + def isValidFilename(name: String) = !name.exists(invalidChars.contains) + + def validatePath(path: String) = { + if (!isValidFilename(path)) { + throw new java.io.IOException("path contains invalid characters") + } + path + } + override def fromClass(clazz: Class[_], domain: String, root: String): api.fs.FileSystem = { val innerPath = ("/assets/" + domain + "/" + (root.trim + "/")).replace("//", "/") @@ -76,9 +89,9 @@ object FileSystem extends api.detail.FileSystemAPI { case _ => System.getProperty("java.class.path").split(System.getProperty("path.separator")). find(cp => { - val fsp = new io.File(new io.File(cp), innerPath) - fsp.exists() && fsp.isDirectory - }) match { + val fsp = new io.File(new io.File(cp), innerPath) + fsp.exists() && fsp.isDirectory + }) match { case None => null case Some(dir) => new ReadOnlyFileSystem(new io.File(new io.File(dir), innerPath)) } @@ -166,29 +179,11 @@ object FileSystem extends api.detail.FileSystemAPI { extends VirtualFileSystem with Buffered with Capacity { - // Worst-case: we're on Windows or using a FAT32 partition mounted in *nix. - // Note: we allow / as the path separator and expect all \s to be converted - // accordingly before the path is passed to the file system. - private val invalidChars = """\:*?"<>|""".toSet - - override protected def isValidFilename(name: String) = !name.exists(invalidChars.contains) - - override def makeDirectory(path: String) = super.makeDirectory(validatePath(path)) - - override protected def openOutputHandle(id: Int, path: String, mode: Mode) = super.openOutputHandle(id, validatePath(path), mode) - protected override def segments(path: String) = { val parts = super.segments(path) if (isCaseInsensitive) toCaseInsensitive(parts) else parts } - private def validatePath(path: String) = { - if (!isValidFilename(path)) { - throw new java.io.IOException("path contains invalid characters") - } - path - } - private def toCaseInsensitive(path: Array[String]): Array[String] = { var node = root path.map(segment => { diff --git a/src/main/scala/li/cil/oc/server/fs/InputStreamFileSystem.scala b/src/main/scala/li/cil/oc/server/fs/InputStreamFileSystem.scala index cd61f41ba..93152cb01 100644 --- a/src/main/scala/li/cil/oc/server/fs/InputStreamFileSystem.scala +++ b/src/main/scala/li/cil/oc/server/fs/InputStreamFileSystem.scala @@ -30,15 +30,18 @@ trait InputStreamFileSystem extends api.fs.FileSystem { // ----------------------------------------------------------------------- // - override def open(path: String, mode: Mode) = this.synchronized(if (mode == Mode.Read && exists(path) && !isDirectory(path)) { - val handle = Iterator.continually((Math.random() * Int.MaxValue).toInt + 1).filterNot(handles.contains).next() - openInputChannel(path) match { - case Some(channel) => - handles += handle -> new Handle(this, handle, path, channel) - handle - case _ => throw new FileNotFoundException(path) - } - } else throw new FileNotFoundException(path)) + override def open(path: String, mode: Mode) = { + FileSystem.validatePath(path) + this.synchronized(if (mode == Mode.Read && exists(path) && !isDirectory(path)) { + val handle = Iterator.continually((Math.random() * Int.MaxValue).toInt + 1).filterNot(handles.contains).next() + openInputChannel(path) match { + case Some(channel) => + handles += handle -> new Handle(this, handle, path, channel) + handle + case _ => throw new FileNotFoundException(path) + } + } else throw new FileNotFoundException(path)) + } override def getHandle(handle: Int): api.fs.Handle = this.synchronized(handles.get(handle).orNull) diff --git a/src/main/scala/li/cil/oc/server/fs/OutputStreamFileSystem.scala b/src/main/scala/li/cil/oc/server/fs/OutputStreamFileSystem.scala index e53c99063..d12b487b9 100644 --- a/src/main/scala/li/cil/oc/server/fs/OutputStreamFileSystem.scala +++ b/src/main/scala/li/cil/oc/server/fs/OutputStreamFileSystem.scala @@ -22,15 +22,17 @@ trait OutputStreamFileSystem extends InputStreamFileSystem { override def open(path: String, mode: Mode) = this.synchronized(mode match { case Mode.Read => super.open(path, mode) - case _ => if (!isDirectory(path)) { - val handle = Iterator.continually((Math.random() * Int.MaxValue).toInt + 1).filterNot(handles.contains).next() - openOutputHandle(handle, path, mode) match { - case Some(fileHandle) => - handles += handle -> fileHandle - handle - case _ => throw new FileNotFoundException(path) - } - } else throw new FileNotFoundException(path) + case _ => + FileSystem.validatePath(path) + if (!isDirectory(path)) { + val handle = Iterator.continually((Math.random() * Int.MaxValue).toInt + 1).filterNot(handles.contains).next() + openOutputHandle(handle, path, mode) match { + case Some(fileHandle) => + handles += handle -> fileHandle + handle + case _ => throw new FileNotFoundException(path) + } + } else throw new FileNotFoundException(path) }) override def getHandle(handle: Int): api.fs.Handle = this.synchronized(Option(super.getHandle(handle)).orElse(handles.get(handle)).orNull) diff --git a/src/main/scala/li/cil/oc/server/fs/VirtualFileSystem.scala b/src/main/scala/li/cil/oc/server/fs/VirtualFileSystem.scala index 7319faee5..8c6a8b201 100644 --- a/src/main/scala/li/cil/oc/server/fs/VirtualFileSystem.scala +++ b/src/main/scala/li/cil/oc/server/fs/VirtualFileSystem.scala @@ -135,7 +135,7 @@ trait VirtualFileSystem extends OutputStreamFileSystem { // ----------------------------------------------------------------------- // - protected def segments(path: String) = path.split("/").filter(_ != "") + protected def segments(path: String) = FileSystem.validatePath(path).split("/").filter(_ != "") // ----------------------------------------------------------------------- //