diff --git a/java/ql/lib/semmle/code/java/StringFormat.qll b/java/ql/lib/semmle/code/java/StringFormat.qll index 2938f5255fad..bfc893ab7566 100644 --- a/java/ql/lib/semmle/code/java/StringFormat.qll +++ b/java/ql/lib/semmle/code/java/StringFormat.qll @@ -325,7 +325,7 @@ private predicate formatStringValue(Expr e, string fmtvalue) { or exists(Field f | e = f.getAnAccess() and - f.getDeclaringType().hasQualifiedName("java.io", "File") and + f.getDeclaringType() instanceof TypeFile and fmtvalue = "x" // dummy value | f.hasName("pathSeparator") or diff --git a/java/ql/lib/semmle/code/java/frameworks/javaee/ejb/EJBRestrictions.qll b/java/ql/lib/semmle/code/java/frameworks/javaee/ejb/EJBRestrictions.qll index c62b72ab9fc3..67c66d6d8173 100644 --- a/java/ql/lib/semmle/code/java/frameworks/javaee/ejb/EJBRestrictions.qll +++ b/java/ql/lib/semmle/code/java/frameworks/javaee/ejb/EJBRestrictions.qll @@ -244,7 +244,7 @@ class SecurityManagerClass extends Class { /** A class involving file input or output. */ class FileInputOutputClass extends Class { FileInputOutputClass() { - this.hasQualifiedName("java.io", "File") or + this instanceof TypeFile or this.hasQualifiedName("java.io", "FileDescriptor") or this.hasQualifiedName("java.io", "FileInputStream") or this.hasQualifiedName("java.io", "FileOutputStream") or diff --git a/java/ql/lib/semmle/code/java/security/FileWritable.qll b/java/ql/lib/semmle/code/java/security/FileWritable.qll index bd8329647da5..09a9e37fb9b2 100644 --- a/java/ql/lib/semmle/code/java/security/FileWritable.qll +++ b/java/ql/lib/semmle/code/java/security/FileWritable.qll @@ -67,7 +67,7 @@ private VarAccess getFileForPathConversion(Expr pathExpr) { fileToPath = pathExpr and result = fileToPath.getQualifier() and fileToPath.getMethod().hasName("toPath") and - fileToPath.getMethod().getDeclaringType().hasQualifiedName("java.io", "File") + fileToPath.getMethod().getDeclaringType() instanceof TypeFile ) or // Look for the pattern `Paths.get(file.get*Path())` for converting between a `File` and a `Path`. diff --git a/java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.qhelp b/java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.qhelp new file mode 100644 index 000000000000..e3bf61107c4a --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.qhelp @@ -0,0 +1,49 @@ + + + +

Local information disclosure can occur when files/directories are written into +directories that are shared between all users on the system.

+ +

On most unix-like systems, +the system temporary directory is shared between local users. +If files/directories are created within the system temporary directory without using +APIs that explicitly set the correct file permissions, local information disclosure +can occur.

+ +

Depending upon the particular file contents exposed, this vulnerability can have a +CVSSv3.1 base score of 6.2/10.

+
+ + +

Use JDK methods that specifically protect against this vulnerability:

+ + +

Otherwise, create the file/directory by manually specifying the expected posix file permissions. +For example: PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))

+ +
+ + +

In the following example, files and directories are created with file permissions that allow other local users to read their contents.

+ + + +

In the following example, files and directories are created with file permissions that protect their contents.

+ + +
+ + +
  • OSWAP: Insecure Temporary File.
  • +
  • CERT: FIO00-J. Do not operate on files in shared directories.
  • +
    +
    \ No newline at end of file diff --git a/java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.ql b/java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.ql new file mode 100644 index 000000000000..a3faddf6ab73 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.ql @@ -0,0 +1,258 @@ +/** + * @name Local information disclosure in a temporary directory + * @description Writing information without explicit permissions to a shared temporary directory may disclose it to other users. + * @kind path-problem + * @problem.severity warning + * @precision medium + * @id java/local-temp-file-or-directory-information-disclosure + * @tags security + * external/cwe/cwe-200 + * external/cwe/cwe-732 + */ + +import java +import TempDirUtils +import DataFlow::PathGraph +import semmle.code.java.dataflow.TaintTracking2 + +abstract private class MethodFileSystemFileCreation extends Method { + MethodFileSystemFileCreation() { this.getDeclaringType() instanceof TypeFile } +} + +private class MethodFileDirectoryCreation extends MethodFileSystemFileCreation { + MethodFileDirectoryCreation() { this.hasName(["mkdir", "mkdirs"]) } +} + +private class MethodFileFileCreation extends MethodFileSystemFileCreation { + MethodFileFileCreation() { this.hasName("createNewFile") } +} + +abstract private class FileCreationSink extends DataFlow::Node { } + +/** + * The qualifier of a call to one of `File`'s file-creating or directory-creating methods, + * treated as a sink by `TempDirSystemGetPropertyToCreateConfig`. + */ +private class FileFileCreationSink extends FileCreationSink { + FileFileCreationSink() { + exists(MethodAccess ma | + ma.getMethod() instanceof MethodFileSystemFileCreation and + ma.getQualifier() = this.asExpr() + ) + } +} + +/** + * The argument to a call to one of `Files` file-creating or directory-creating methods, + * treated as a sink by `TempDirSystemGetPropertyToCreateConfig`. + */ +private class FilesFileCreationSink extends FileCreationSink { + FilesFileCreationSink() { + exists(FilesVulnerableCreationMethodAccess ma | ma.getArgument(0) = this.asExpr()) + } +} + +/** + * A call to a `Files` method that create files/directories without explicitly + * setting the newly-created file or directory's permissions. + */ +private class FilesVulnerableCreationMethodAccess extends MethodAccess { + FilesVulnerableCreationMethodAccess() { + exists(Method m | + m = this.getMethod() and + m.getDeclaringType().hasQualifiedName("java.nio.file", "Files") + | + m.hasName(["write", "newBufferedWriter", "newOutputStream"]) + or + m.hasName(["createFile", "createDirectory", "createDirectories"]) and + this.getNumArgument() = 1 + or + m.hasName("newByteChannel") and + this.getNumArgument() = 2 + ) + } +} + +/** + * A call to a `File` method that create files/directories with a specific set of permissions explicitly set. + * We can safely assume that any calls to these methods with explicit `PosixFilePermissions.asFileAttribute` + * contains a certain level of intentionality behind it. + */ +private class FilesSanitizingCreationMethodAccess extends MethodAccess { + FilesSanitizingCreationMethodAccess() { + exists(Method m | + m = this.getMethod() and + m.getDeclaringType().hasQualifiedName("java.nio.file", "Files") + | + m.hasName(["createFile", "createDirectory", "createDirectories"]) and + this.getNumArgument() = 2 + ) + } +} + +/** + * The temp directory argument to a call to `java.io.File::createTempFile`, + * treated as a sink by `TempDirSystemGetPropertyToCreateConfig`. + */ +private class FileCreateTempFileSink extends FileCreationSink { + FileCreateTempFileSink() { + exists(MethodAccess ma | + ma.getMethod() instanceof MethodFileCreateTempFile and ma.getArgument(2) = this.asExpr() + ) + } +} + +private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Configuration { + TempDirSystemGetPropertyToCreateConfig() { this = "TempDirSystemGetPropertyToCreateConfig" } + + override predicate isSource(DataFlow::Node source) { + source.asExpr() instanceof MethodAccessSystemGetPropertyTempDirTainted + } + + /** + * Find dataflow from the temp directory system property to the `File` constructor. + * Examples: + * - `new File(System.getProperty("java.io.tmpdir"))` + * - `new File(new File(System.getProperty("java.io.tmpdir")), "/child")` + */ + override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { + isAdditionalFileTaintStep(node1, node2) + } + + override predicate isSink(DataFlow::Node sink) { + sink instanceof FileCreationSink and + not any(TempDirSystemGetPropertyDirectlyToMkdirConfig config).hasFlowTo(sink) + } + + override predicate isSanitizer(DataFlow::Node sanitizer) { + exists(FilesSanitizingCreationMethodAccess sanitisingMethodAccess | + sanitizer.asExpr() = sanitisingMethodAccess.getArgument(0) + ) + } +} + +/** + * Configuration that tracks calls to to `mkdir` or `mkdirs` that are are directly on the temp directory system property. + * Examples: + * - `File tempDir = new File(System.getProperty("java.io.tmpdir")); tempDir.mkdir();` + * - `File tempDir = new File(System.getProperty("java.io.tmpdir")); tempDir.mkdirs();` + * + * These are examples of code that is simply verifying that the temp directory exists. + * As such, this code pattern is filtered out as an explicit vulnerability in + * `TempDirSystemGetPropertyToCreateConfig::isSink`. + */ +private class TempDirSystemGetPropertyDirectlyToMkdirConfig extends TaintTracking2::Configuration { + TempDirSystemGetPropertyDirectlyToMkdirConfig() { + this = "TempDirSystemGetPropertyDirectlyToMkdirConfig" + } + + override predicate isSource(DataFlow::Node node) { + exists( + MethodAccessSystemGetPropertyTempDirTainted propertyGetMethodAccess, DataFlow::Node callSite + | + DataFlow::localFlow(DataFlow::exprNode(propertyGetMethodAccess), callSite) + | + isFileConstructorArgument(callSite.asExpr(), node.asExpr(), 1) + ) + } + + override predicate isSink(DataFlow::Node node) { + exists(MethodAccess ma | ma.getMethod() instanceof MethodFileDirectoryCreation | + ma.getQualifier() = node.asExpr() + ) + } + + override predicate isSanitizer(DataFlow::Node sanitizer) { + isFileConstructorArgument(sanitizer.asExpr(), _, _) + } +} + +// +// Begin configuration for tracking single-method calls that are vulnerable. +// +/** + * A `MethodAccess` against a method that creates a temporary file or directory in a shared temporary directory. + */ +abstract class MethodAccessInsecureFileCreation extends MethodAccess { + /** + * Gets the type of entity created (e.g. `file`, `directory`, ...). + */ + abstract string getFileSystemEntityType(); +} + +/** + * An insecure call to `java.io.File.createTempFile`. + */ +class MethodAccessInsecureFileCreateTempFile extends MethodAccessInsecureFileCreation { + MethodAccessInsecureFileCreateTempFile() { + this.getMethod() instanceof MethodFileCreateTempFile and + ( + // `File.createTempFile(string, string)` always uses the default temporary directory + this.getNumArgument() = 2 + or + // The default temporary directory is used when the last argument of `File.createTempFile(string, string, File)` is `null` + DataFlow::localExprFlow(any(NullLiteral n), this.getArgument(2)) + ) + } + + override string getFileSystemEntityType() { result = "file" } +} + +/** + * The `com.google.common.io.Files.createTempDir` method. + */ +class MethodGuavaFilesCreateTempFile extends Method { + MethodGuavaFilesCreateTempFile() { + this.getDeclaringType().hasQualifiedName("com.google.common.io", "Files") and + this.hasName("createTempDir") + } +} + +/** + * A call to the `com.google.common.io.Files.createTempDir` method. + */ +class MethodAccessInsecureGuavaFilesCreateTempFile extends MethodAccessInsecureFileCreation { + MethodAccessInsecureGuavaFilesCreateTempFile() { + this.getMethod() instanceof MethodGuavaFilesCreateTempFile + } + + override string getFileSystemEntityType() { result = "directory" } +} + +/** + * A hack: we include use of inherently insecure methods, which don't have any associated + * flow path, in with results describing a path from reading `java.io.tmpdir` or similar to use + * in a file creation op. + * + * We achieve this by making inherently-insecure method invocations both a source and a sink in + * this configuration, resulting in a zero-length path which is type-compatible with the actual + * path-flow results. + */ +class InsecureMethodPseudoConfiguration extends DataFlow::Configuration { + InsecureMethodPseudoConfiguration() { this = "InsecureMethodPseudoConfiguration" } + + override predicate isSource(DataFlow::Node node) { + node.asExpr() instanceof MethodAccessInsecureFileCreation + } + + override predicate isSink(DataFlow::Node node) { + node.asExpr() instanceof MethodAccessInsecureFileCreation + } +} + +from DataFlow::PathNode source, DataFlow::PathNode sink, string message +where + ( + any(TempDirSystemGetPropertyToCreateConfig conf).hasFlowPath(source, sink) and + message = + "Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users." + or + any(InsecureMethodPseudoConfiguration conf).hasFlowPath(source, sink) and + // Note this message has no "$@" placeholder, so the "system temp directory" template parameter below is not used. + message = + "Local information disclosure vulnerability due to use of " + + source.getNode().asExpr().(MethodAccessInsecureFileCreation).getFileSystemEntityType() + + " readable by other local users." + ) and + not isPermissionsProtectedTempDirUse(sink.getNode()) +select source.getNode(), source, sink, message, source.getNode(), "system temp directory" diff --git a/java/ql/src/Security/CWE/CWE-200/TempDirUsageSafe.java b/java/ql/src/Security/CWE/CWE-200/TempDirUsageSafe.java new file mode 100644 index 000000000000..f44ead7accbe --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-200/TempDirUsageSafe.java @@ -0,0 +1,100 @@ +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.EnumSet; + +public class TempDirUsageSafe { + void exampleSafe() throws IOException { + Path temp1 = Files.createTempFile("random", ".txt"); // GOOD: File has permissions `-rw-------` + + Path temp2 = Files.createTempDirectory("random-directory"); // GOOD: File has permissions `drwx------` + + // Creating a temporary file with a non-randomly generated name + File tempChildFile = new File(System.getProperty("java.io.tmpdir"), "/child-create-file.txt"); + // Warning: This will fail on windows as it doesn't support PosixFilePermissions. + // See `exampleSafeWithWindowsSupportFile` if your code needs to support windows and unix-like systems. + Files.createFile( + tempChildFile.toPath(), + PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE)) + ); // GOOD: Good has permissions `-rw-------` + } + + /* + * An example of a safe use of createFile or createDirectory if your code must support windows and unix-like systems. + */ + void exampleSafeWithWindowsSupportFile() { + // Creating a temporary file with a non-randomly generated name + File tempChildFile = new File(System.getProperty("java.io.tmpdir"), "/child-create-file.txt"); + createTempFile(tempChildFile.toPath()); // GOOD: Good has permissions `-rw-------` + } + + static void createTempFile(Path tempDir) { + try { + if (tempDirChild.getFileSystem().supportedFileAttributeViews().contains("posix")) { + // Explicit permissions setting is only required on unix-like systems because + // the temporary directory is shared between all users. + // This is not necessary on Windows, each user has their own temp directory + final EnumSet posixFilePermissions = + EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE + ); + if (!Files.exists(tempDirChild)) { + Files.createFile( + tempDirChild, + PosixFilePermissions.asFileAttribute(posixFilePermissions) + ); // GOOD: Directory has permissions `-rw-------` + } else { + Files.setPosixFilePermissions( + tempDirChild, + posixFilePermissions + ); // GOOD: Good has permissions `-rw-------`, or will throw an exception if this fails + } + } else if (!Files.exists(tempDirChild)) { + // On Windows, we still need to create the directory, when it doesn't already exist. + Files.createDirectory(tempDirChild); // GOOD: Windows doesn't share the temp directory between users + } + } catch (IOException exception) { + throw new UncheckedIOException("Failed to create temp file", exception); + } + } + + void exampleSafeWithWindowsSupportDirectory() { + File tempDirChildDir = new File(System.getProperty("java.io.tmpdir"), "/child-dir"); + createTempDirectories(tempDirChildDir.toPath()); // GOOD: Directory has permissions `drwx------` + } + + static void createTempDirectories(Path tempDirChild) { + try { + if (tempDirChild.getFileSystem().supportedFileAttributeViews().contains("posix")) { + // Explicit permissions setting is only required on unix-like systems because + // the temporary directory is shared between all users. + // This is not necessary on Windows, each user has their own temp directory + final EnumSet posixFilePermissions = + EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE + ); + if (!Files.exists(tempDirChild)) { + Files.createDirectories( + tempDirChild, + PosixFilePermissions.asFileAttribute(posixFilePermissions) + ); // GOOD: Directory has permissions `drwx------` + } else { + Files.setPosixFilePermissions( + tempDirChild, + posixFilePermissions + ); // GOOD: Good has permissions `drwx------`, or will throw an exception if this fails + } + } else if (!Files.exists(tempDirChild)) { + // On Windows, we still need to create the directory, when it doesn't already exist. + Files.createDirectories(tempDirChild); // GOOD: Windows doesn't share the temp directory between users + } + } catch (IOException exception) { + throw new UncheckedIOException("Failed to create temp dir", exception); + } + } +} diff --git a/java/ql/src/Security/CWE/CWE-200/TempDirUsageVulnerable.java b/java/ql/src/Security/CWE/CWE-200/TempDirUsageVulnerable.java new file mode 100644 index 000000000000..7d1d212409ea --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-200/TempDirUsageVulnerable.java @@ -0,0 +1,23 @@ +import java.io.File; + +public class TempDirUsageVulnerable { + void exampleVulnerable() { + File temp1 = File.createTempFile("random", ".txt"); // BAD: File has permissions `-rw-r--r--` + + File temp2 = File.createTempFile("random", "file", null); // BAD: File has permissions `-rw-r--r--` + + File systemTempDir = new File(System.getProperty("java.io.tmpdir")); + File temp3 = File.createTempFile("random", "file", systemTempDir); // BAD: File has permissions `-rw-r--r--` + + File tempDir = com.google.common.io.Files.createTempDir(); // BAD: CVE-2020-8908: Directory has permissions `drwxr-xr-x` + + new File(System.getProperty("java.io.tmpdir"), "/child").mkdir(); // BAD: Directory has permissions `-rw-r--r--` + + File tempDirChildFile = new File(System.getProperty("java.io.tmpdir"), "/child-create-file.txt"); + Files.createFile(tempDirChildFile.toPath()); // BAD: File has permissions `-rw-r--r--` + + File tempDirChildDir = new File(System.getProperty("java.io.tmpdir"), "/child-dir"); + tempDirChildDir.mkdir(); // BAD: Directory has permissions `drwxr-xr-x` + Files.createDirectory(tempDirChildDir.toPath()); // BAD: Directory has permissions `drwxr-xr-x` + } +} diff --git a/java/ql/src/Security/CWE/CWE-200/TempDirUtils.qll b/java/ql/src/Security/CWE/CWE-200/TempDirUtils.qll new file mode 100644 index 000000000000..d2a2bcb5a6fb --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-200/TempDirUtils.qll @@ -0,0 +1,130 @@ +/** + * Provides classes and predicates for reasoning about temporary file/directory creations. + */ + +import java +import semmle.code.java.dataflow.FlowSources + +/** + * A method that returns a `String` or `File` that has been tainted by `System.getProperty("java.io.tmpdir")`. + */ +abstract class MethodAccessSystemGetPropertyTempDirTainted extends MethodAccess { } + +/** + * Method access `System.getProperty("java.io.tmpdir")`. + */ +private class MethodAccessSystemGetPropertyTempDir extends MethodAccessSystemGetPropertyTempDirTainted, + MethodAccessSystemGetProperty { + MethodAccessSystemGetPropertyTempDir() { + this.hasCompileTimeConstantGetPropertyName("java.io.tmpdir") + } +} + +/** + * A method call to the `org.apache.commons.io.FileUtils` methods `getTempDirectory` or `getTempDirectoryPath`. + */ +private class MethodAccessApacheFileUtilsTempDir extends MethodAccessSystemGetPropertyTempDirTainted { + MethodAccessApacheFileUtilsTempDir() { + exists(Method m | + m.getDeclaringType().hasQualifiedName("org.apache.commons.io", "FileUtils") and + m.hasName(["getTempDirectory", "getTempDirectoryPath"]) and + this.getMethod() = m + ) + } +} + +/** + * A `java.io.File::createTempFile` method. + */ +class MethodFileCreateTempFile extends Method { + MethodFileCreateTempFile() { + this.getDeclaringType() instanceof TypeFile and + this.hasName("createTempFile") + } +} + +/** + * Holds if `expDest` is some constructor call `new java.io.File(expSource)`, where the specific `File` constructor being used has `paramCount` parameters. + */ +predicate isFileConstructorArgument(Expr expSource, Expr exprDest, int paramCount) { + exists(ConstructorCall construtorCall | + construtorCall.getConstructedType() instanceof TypeFile and + construtorCall.getArgument(0) = expSource and + construtorCall = exprDest and + construtorCall.getConstructor().getNumberOfParameters() = paramCount + ) +} + +/** + * A `File` method where the temporary directory is still part of the root path. + */ +private class TaintFollowingFileMethod extends Method { + TaintFollowingFileMethod() { + this.getDeclaringType() instanceof TypeFile and + this.hasName(["getAbsoluteFile", "getCanonicalFile"]) + } +} + +private predicate isTaintPropagatingFileTransformation(Expr expSource, Expr exprDest) { + exists(MethodAccess fileMethodAccess | + fileMethodAccess.getMethod() instanceof TaintFollowingFileMethod and + fileMethodAccess.getQualifier() = expSource and + fileMethodAccess = exprDest + ) +} + +/** + * Holds if taint should propagate from `node1` to `node2` across some file creation or transformation operation. + * For example, `taintedFile.getCanonicalFile()` is itself tainted. + */ +predicate isAdditionalFileTaintStep(DataFlow::Node node1, DataFlow::Node node2) { + isFileConstructorArgument(node1.asExpr(), node2.asExpr(), _) or + isTaintPropagatingFileTransformation(node1.asExpr(), node2.asExpr()) +} + +/** + * A method call to `java.io.File::setReadable`. + */ +private class FileSetRedableMethodAccess extends MethodAccess { + FileSetRedableMethodAccess() { + exists(Method m | this.getMethod() = m | + m.getDeclaringType() instanceof TypeFile and + m.hasName("setReadable") + ) + } + + predicate isCallWithArguments(boolean arg1, boolean arg2) { + this.isCallWithArgument(0, arg1) and this.isCallToSecondArgumentWithValue(arg2) + } + + private predicate isCallToSecondArgumentWithValue(boolean value) { + this.getMethod().getNumberOfParameters() = 1 and value = true + or + isCallWithArgument(1, value) + } + + private predicate isCallWithArgument(int index, boolean arg) { + DataFlow::localExprFlow(any(CompileTimeConstantExpr e | e.getBooleanValue() = arg), + this.getArgument(index)) + } +} + +/** + * Hold's if temporary directory's use is protected if there is an explicit call to + * `setReadable(false, false)`, then `setRedabale(true, true)`. + */ +predicate isPermissionsProtectedTempDirUse(DataFlow::Node sink) { + exists(FileSetRedableMethodAccess setReadable1, FileSetRedableMethodAccess setReadable2 | + setReadable1.isCallWithArguments(false, false) and + setReadable2.isCallWithArguments(true, true) + | + exists(DataFlow::Node setReadableNode1, DataFlow::Node setReadableNode2 | + setReadableNode1.asExpr() = setReadable1.getQualifier() and + setReadableNode2.asExpr() = setReadable2.getQualifier() + | + DataFlow::localFlow(sink, setReadableNode1) and // Flow from sink to setReadable(false, false) + DataFlow::localFlow(sink, setReadableNode2) and // Flow from sink to setReadable(true, true) + DataFlow::localFlow(setReadableNode1, setReadableNode2) // Flow from setReadable(false, false) to setReadable(true, true) + ) + ) +} diff --git a/java/ql/src/change-notes/2022-02-04-local-temp-file-or-directory-information-disclosure.md b/java/ql/src/change-notes/2022-02-04-local-temp-file-or-directory-information-disclosure.md new file mode 100644 index 000000000000..23f3a476e793 --- /dev/null +++ b/java/ql/src/change-notes/2022-02-04-local-temp-file-or-directory-information-disclosure.md @@ -0,0 +1,6 @@ +--- +category: newQuery +--- +* A new query titled "Local information disclosure in a temporary directory" (`java/local-temp-file-or-directory-information-disclosure`) has been added. + This query finds uses of APIs that leak potentially sensitive information to other local users via the system temporary directory. + This query was originally [submitted as query by @JLLeitschuh](https://github.com/github/codeql/pull/4388). \ No newline at end of file diff --git a/java/ql/test/query-tests/security/CWE-200/semmle/tests/Files.java b/java/ql/test/query-tests/security/CWE-200/semmle/tests/Files.java new file mode 100644 index 000000000000..cc8c1a736adf --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-200/semmle/tests/Files.java @@ -0,0 +1,22 @@ +package com.google.common.io; + +import java.io.File; + +public class Files { + /** Maximum loop count when creating temp directories. */ + private static final int TEMP_DIR_ATTEMPTS = 10000; + + public static File createTempDir() { + File baseDir = new File(System.getProperty("java.io.tmpdir")); + String baseName = System.currentTimeMillis() + "-"; + + for (int counter = 0; counter < TEMP_DIR_ATTEMPTS; counter++) { + File tempDir = new File(baseDir, baseName + counter); + if (tempDir.mkdir()) { + return tempDir; + } + } + throw new IllegalStateException("Failed to create directory within " + TEMP_DIR_ATTEMPTS + " attempts (tried " + + baseName + "0 to " + baseName + (TEMP_DIR_ATTEMPTS - 1) + ')'); + } +} diff --git a/java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosure.expected b/java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosure.expected new file mode 100644 index 000000000000..7c21c3667a3f --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosure.expected @@ -0,0 +1,130 @@ +edges +| Files.java:10:24:10:69 | new File(...) : File | Files.java:14:37:14:43 | baseDir : File | +| Files.java:10:24:10:69 | new File(...) : File | Files.java:15:17:15:23 | tempDir | +| Files.java:10:33:10:68 | getProperty(...) : String | Files.java:10:24:10:69 | new File(...) : File | +| Files.java:10:33:10:68 | getProperty(...) : String | Files.java:14:37:14:43 | baseDir : File | +| Files.java:10:33:10:68 | getProperty(...) : String | Files.java:15:17:15:23 | tempDir | +| Files.java:14:28:14:64 | new File(...) : File | Files.java:15:17:15:23 | tempDir | +| Files.java:14:37:14:43 | baseDir : File | Files.java:14:28:14:64 | new File(...) : File | +| Test.java:34:24:34:69 | new File(...) : File | Test.java:37:63:37:69 | tempDir | +| Test.java:34:33:34:68 | getProperty(...) : String | Test.java:34:24:34:69 | new File(...) : File | +| Test.java:34:33:34:68 | getProperty(...) : String | Test.java:37:63:37:69 | tempDir | +| Test.java:48:29:48:94 | new File(...) : File | Test.java:51:63:51:74 | tempDirChild | +| Test.java:48:38:48:83 | new File(...) : File | Test.java:48:29:48:94 | new File(...) : File | +| Test.java:48:38:48:83 | new File(...) : File | Test.java:51:63:51:74 | tempDirChild | +| Test.java:48:47:48:82 | getProperty(...) : String | Test.java:48:38:48:83 | new File(...) : File | +| Test.java:48:47:48:82 | getProperty(...) : String | Test.java:51:63:51:74 | tempDirChild | +| Test.java:59:24:59:69 | new File(...) : File | Test.java:62:63:62:69 | tempDir | +| Test.java:59:33:59:68 | getProperty(...) : String | Test.java:59:24:59:69 | new File(...) : File | +| Test.java:59:33:59:68 | getProperty(...) : String | Test.java:62:63:62:69 | tempDir | +| Test.java:73:24:73:69 | new File(...) : File | Test.java:76:63:76:69 | tempDir | +| Test.java:73:33:73:68 | getProperty(...) : String | Test.java:73:24:73:69 | new File(...) : File | +| Test.java:73:33:73:68 | getProperty(...) : String | Test.java:76:63:76:69 | tempDir | +| Test.java:108:29:108:84 | new File(...) : File | Test.java:111:9:111:20 | tempDirChild | +| Test.java:108:38:108:73 | getProperty(...) : String | Test.java:108:29:108:84 | new File(...) : File | +| Test.java:108:38:108:73 | getProperty(...) : String | Test.java:111:9:111:20 | tempDirChild | +| Test.java:132:29:132:84 | new File(...) : File | Test.java:135:9:135:20 | tempDirChild | +| Test.java:132:38:132:73 | getProperty(...) : String | Test.java:132:29:132:84 | new File(...) : File | +| Test.java:132:38:132:73 | getProperty(...) : String | Test.java:135:9:135:20 | tempDirChild | +| Test.java:156:29:156:88 | new File(...) : File | Test.java:157:21:157:32 | tempDirChild : File | +| Test.java:156:38:156:73 | getProperty(...) : String | Test.java:156:29:156:88 | new File(...) : File | +| Test.java:156:38:156:73 | getProperty(...) : String | Test.java:157:21:157:32 | tempDirChild : File | +| Test.java:157:21:157:32 | tempDirChild : File | Test.java:157:21:157:41 | toPath(...) | +| Test.java:185:29:185:88 | new File(...) : File | Test.java:186:21:186:32 | tempDirChild : File | +| Test.java:185:38:185:73 | getProperty(...) : String | Test.java:185:29:185:88 | new File(...) : File | +| Test.java:185:38:185:73 | getProperty(...) : String | Test.java:186:21:186:32 | tempDirChild : File | +| Test.java:186:21:186:32 | tempDirChild : File | Test.java:186:21:186:41 | toPath(...) | +| Test.java:202:29:202:104 | new File(...) : File | Test.java:202:29:202:113 | toPath(...) : Path | +| Test.java:202:29:202:113 | toPath(...) : Path | Test.java:205:33:205:44 | tempDirChild | +| Test.java:202:38:202:73 | getProperty(...) : String | Test.java:202:29:202:104 | new File(...) : File | +| Test.java:214:29:214:102 | new File(...) : File | Test.java:214:29:214:111 | toPath(...) : Path | +| Test.java:214:29:214:111 | toPath(...) : Path | Test.java:217:31:217:42 | tempDirChild | +| Test.java:214:38:214:73 | getProperty(...) : String | Test.java:214:29:214:102 | new File(...) : File | +| Test.java:226:29:226:100 | new File(...) : File | Test.java:229:26:229:37 | tempDirChild : File | +| Test.java:226:38:226:73 | getProperty(...) : String | Test.java:226:29:226:100 | new File(...) : File | +| Test.java:226:38:226:73 | getProperty(...) : String | Test.java:229:26:229:37 | tempDirChild : File | +| Test.java:229:26:229:37 | tempDirChild : File | Test.java:229:26:229:46 | toPath(...) | +| Test.java:247:29:247:101 | new File(...) : File | Test.java:250:31:250:42 | tempDirChild : File | +| Test.java:247:38:247:73 | getProperty(...) : String | Test.java:247:29:247:101 | new File(...) : File | +| Test.java:247:38:247:73 | getProperty(...) : String | Test.java:250:31:250:42 | tempDirChild : File | +| Test.java:250:31:250:42 | tempDirChild : File | Test.java:250:31:250:51 | toPath(...) | +| Test.java:258:29:258:109 | new File(...) : File | Test.java:261:33:261:44 | tempDirChild : File | +| Test.java:258:38:258:73 | getProperty(...) : String | Test.java:258:29:258:109 | new File(...) : File | +| Test.java:258:38:258:73 | getProperty(...) : String | Test.java:261:33:261:44 | tempDirChild : File | +| Test.java:261:33:261:44 | tempDirChild : File | Test.java:261:33:261:53 | toPath(...) | +nodes +| Files.java:10:24:10:69 | new File(...) : File | semmle.label | new File(...) : File | +| Files.java:10:33:10:68 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Files.java:14:28:14:64 | new File(...) : File | semmle.label | new File(...) : File | +| Files.java:14:37:14:43 | baseDir : File | semmle.label | baseDir : File | +| Files.java:15:17:15:23 | tempDir | semmle.label | tempDir | +| Test.java:18:25:18:61 | createTempFile(...) | semmle.label | createTempFile(...) | +| Test.java:26:25:26:67 | createTempFile(...) | semmle.label | createTempFile(...) | +| Test.java:34:24:34:69 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:34:33:34:68 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:37:63:37:69 | tempDir | semmle.label | tempDir | +| Test.java:48:29:48:94 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:48:38:48:83 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:48:47:48:82 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:51:63:51:74 | tempDirChild | semmle.label | tempDirChild | +| Test.java:59:24:59:69 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:59:33:59:68 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:62:63:62:69 | tempDir | semmle.label | tempDir | +| Test.java:73:24:73:69 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:73:33:73:68 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:76:63:76:69 | tempDir | semmle.label | tempDir | +| Test.java:95:24:95:65 | createTempDir(...) | semmle.label | createTempDir(...) | +| Test.java:108:29:108:84 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:108:38:108:73 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:111:9:111:20 | tempDirChild | semmle.label | tempDirChild | +| Test.java:132:29:132:84 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:132:38:132:73 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:135:9:135:20 | tempDirChild | semmle.label | tempDirChild | +| Test.java:156:29:156:88 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:156:38:156:73 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:157:21:157:32 | tempDirChild : File | semmle.label | tempDirChild : File | +| Test.java:157:21:157:41 | toPath(...) | semmle.label | toPath(...) | +| Test.java:185:29:185:88 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:185:38:185:73 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:186:21:186:32 | tempDirChild : File | semmle.label | tempDirChild : File | +| Test.java:186:21:186:41 | toPath(...) | semmle.label | toPath(...) | +| Test.java:202:29:202:104 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:202:29:202:113 | toPath(...) : Path | semmle.label | toPath(...) : Path | +| Test.java:202:38:202:73 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:205:33:205:44 | tempDirChild | semmle.label | tempDirChild | +| Test.java:214:29:214:102 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:214:29:214:111 | toPath(...) : Path | semmle.label | toPath(...) : Path | +| Test.java:214:38:214:73 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:217:31:217:42 | tempDirChild | semmle.label | tempDirChild | +| Test.java:226:29:226:100 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:226:38:226:73 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:229:26:229:37 | tempDirChild : File | semmle.label | tempDirChild : File | +| Test.java:229:26:229:46 | toPath(...) | semmle.label | toPath(...) | +| Test.java:247:29:247:101 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:247:38:247:73 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:250:31:250:42 | tempDirChild : File | semmle.label | tempDirChild : File | +| Test.java:250:31:250:51 | toPath(...) | semmle.label | toPath(...) | +| Test.java:258:29:258:109 | new File(...) : File | semmle.label | new File(...) : File | +| Test.java:258:38:258:73 | getProperty(...) : String | semmle.label | getProperty(...) : String | +| Test.java:261:33:261:44 | tempDirChild : File | semmle.label | tempDirChild : File | +| Test.java:261:33:261:53 | toPath(...) | semmle.label | toPath(...) | +| Test.java:268:25:268:63 | createTempFile(...) | semmle.label | createTempFile(...) | +subpaths +#select +| Files.java:10:33:10:68 | getProperty(...) | Files.java:10:33:10:68 | getProperty(...) : String | Files.java:15:17:15:23 | tempDir | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Files.java:10:33:10:68 | getProperty(...) | system temp directory | +| Test.java:18:25:18:61 | createTempFile(...) | Test.java:18:25:18:61 | createTempFile(...) | Test.java:18:25:18:61 | createTempFile(...) | Local information disclosure vulnerability due to use of file readable by other local users. | Test.java:18:25:18:61 | createTempFile(...) | system temp directory | +| Test.java:26:25:26:67 | createTempFile(...) | Test.java:26:25:26:67 | createTempFile(...) | Test.java:26:25:26:67 | createTempFile(...) | Local information disclosure vulnerability due to use of file readable by other local users. | Test.java:26:25:26:67 | createTempFile(...) | system temp directory | +| Test.java:34:33:34:68 | getProperty(...) | Test.java:34:33:34:68 | getProperty(...) : String | Test.java:37:63:37:69 | tempDir | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:34:33:34:68 | getProperty(...) | system temp directory | +| Test.java:48:47:48:82 | getProperty(...) | Test.java:48:47:48:82 | getProperty(...) : String | Test.java:51:63:51:74 | tempDirChild | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:48:47:48:82 | getProperty(...) | system temp directory | +| Test.java:59:33:59:68 | getProperty(...) | Test.java:59:33:59:68 | getProperty(...) : String | Test.java:62:63:62:69 | tempDir | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:59:33:59:68 | getProperty(...) | system temp directory | +| Test.java:73:33:73:68 | getProperty(...) | Test.java:73:33:73:68 | getProperty(...) : String | Test.java:76:63:76:69 | tempDir | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:73:33:73:68 | getProperty(...) | system temp directory | +| Test.java:95:24:95:65 | createTempDir(...) | Test.java:95:24:95:65 | createTempDir(...) | Test.java:95:24:95:65 | createTempDir(...) | Local information disclosure vulnerability due to use of directory readable by other local users. | Test.java:95:24:95:65 | createTempDir(...) | system temp directory | +| Test.java:108:38:108:73 | getProperty(...) | Test.java:108:38:108:73 | getProperty(...) : String | Test.java:111:9:111:20 | tempDirChild | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:108:38:108:73 | getProperty(...) | system temp directory | +| Test.java:132:38:132:73 | getProperty(...) | Test.java:132:38:132:73 | getProperty(...) : String | Test.java:135:9:135:20 | tempDirChild | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:132:38:132:73 | getProperty(...) | system temp directory | +| Test.java:156:38:156:73 | getProperty(...) | Test.java:156:38:156:73 | getProperty(...) : String | Test.java:157:21:157:41 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:156:38:156:73 | getProperty(...) | system temp directory | +| Test.java:185:38:185:73 | getProperty(...) | Test.java:185:38:185:73 | getProperty(...) : String | Test.java:186:21:186:41 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:185:38:185:73 | getProperty(...) | system temp directory | +| Test.java:202:38:202:73 | getProperty(...) | Test.java:202:38:202:73 | getProperty(...) : String | Test.java:205:33:205:44 | tempDirChild | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:202:38:202:73 | getProperty(...) | system temp directory | +| Test.java:214:38:214:73 | getProperty(...) | Test.java:214:38:214:73 | getProperty(...) : String | Test.java:217:31:217:42 | tempDirChild | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:214:38:214:73 | getProperty(...) | system temp directory | +| Test.java:226:38:226:73 | getProperty(...) | Test.java:226:38:226:73 | getProperty(...) : String | Test.java:229:26:229:46 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:226:38:226:73 | getProperty(...) | system temp directory | +| Test.java:247:38:247:73 | getProperty(...) | Test.java:247:38:247:73 | getProperty(...) : String | Test.java:250:31:250:51 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:247:38:247:73 | getProperty(...) | system temp directory | +| Test.java:258:38:258:73 | getProperty(...) | Test.java:258:38:258:73 | getProperty(...) : String | Test.java:261:33:261:53 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:258:38:258:73 | getProperty(...) | system temp directory | diff --git a/java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosure.qlref b/java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosure.qlref new file mode 100644 index 000000000000..e678a2426e75 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosure.qlref @@ -0,0 +1 @@ +Security/CWE/CWE-200/TempDirLocalInformationDisclosure.ql diff --git a/java/ql/test/query-tests/security/CWE-200/semmle/tests/Test.java b/java/ql/test/query-tests/security/CWE-200/semmle/tests/Test.java new file mode 100644 index 000000000000..b5b708692f1b --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-200/semmle/tests/Test.java @@ -0,0 +1,282 @@ + +import java.util.Arrays; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.charset.StandardCharsets; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.EnumSet; + +public class Test { + + void vulnerableFileCreateTempFile() throws IOException { + // VULNERABLE VERSION: + File tempVuln = File.createTempFile("random", "file"); + + // TO MAKE SAFE REWRITE TO: + File tempSafe = Files.createTempFile("random", "file").toFile(); + } + + void vulnerableFileCreateTempFileNull() throws IOException { + // VULNERABLE VERSION: + File tempVuln = File.createTempFile("random", "file", null); + + // TO MAKE SAFE REWRITE TO: + File tempSafe = Files.createTempFile("random", "file").toFile(); + } + + void vulnerableFileCreateTempFileTainted() throws IOException { + // GIVEN: + File tempDir = new File(System.getProperty("java.io.tmpdir")); + + // VULNERABLE VERSION: + File tempVuln = File.createTempFile("random", "file", tempDir); + + // TO MAKE SAFE REWRITE TO (v1): + File tempSafe1 = Files.createTempFile(tempDir.toPath(), "random", "file").toFile(); + + // TO MAKE SAFE REWRITE TO (v2): + File tempSafe2 = Files.createTempFile("random", "file").toFile(); + } + + void vulnerableFileCreateTempFileChildTainted() throws IOException { + // GIVEN: + File tempDirChild = new File(new File(System.getProperty("java.io.tmpdir")), "/child"); + + // VULNERABLE VERSION: + File tempVuln = File.createTempFile("random", "file", tempDirChild); + + // TO MAKE SAFE REWRITE TO: + File tempSafe = Files.createTempFile(tempDirChild.toPath(), "random", "file").toFile(); + } + + void vulnerableFileCreateTempFileCanonical() throws IOException { + // GIVEN: + File tempDir = new File(System.getProperty("java.io.tmpdir")).getCanonicalFile(); + + // VULNERABLE VERSION: + File tempVuln = File.createTempFile("random", "file", tempDir); + + // TO MAKE SAFE REWRITE TO (v1): + File tempSafe1 = Files.createTempFile(tempDir.toPath(), "random", "file").toFile(); + + // TO MAKE SAFE REWRITE TO (v2): + File tempSafe2 = Files.createTempFile("random", "file").toFile(); + } + + void vulnerableFileCreateTempFileAbsolute() throws IOException { + // GIVEN: + File tempDir = new File(System.getProperty("java.io.tmpdir")).getAbsoluteFile(); + + // VULNERABLE VERSION: + File tempVuln = File.createTempFile("random", "file", tempDir); + + // TO MAKE SAFE REWRITE TO (v1): + File tempSafe1 = Files.createTempFile(tempDir.toPath(), "random", "file").toFile(); + // TO MAKE SAFE REWRITE TO (v2): + File tempSafe2 = Files.createTempFile("random", "file").toFile(); + } + + void safeFileCreateTempFileTainted() throws IOException { + /* + * Creating a temporary directoy in the current user directory is not a + * vulnerability. + */ + File currentDirectory = new File(System.getProperty("user.dir")); + File temp = File.createTempFile("random", "file", currentDirectory); + } + + void vulnerableGuavaFilesCreateTempDir() { + // VULNERABLE VERSION: + File tempDir = com.google.common.io.Files.createTempDir(); + + // TO MAKE SAFE REWRITE TO: + File tempSafe; + try { + Files.createTempDirectory("random").toFile(); + } catch (IOException e) { + throw new RuntimeException("Failed to create temporary directory", e); + } + } + + void vulnerableFileCreateTempFileMkdirTainted() { + // GIVEN: + File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child"); + + // VULNERABLE VERSION: + tempDirChild.mkdir(); + + // TO MAKE SAFE REWRITE TO (v1): + File tempSafe1; + try { + tempSafe1 = Files.createTempDirectory(tempDirChild.toPath(), "random").toFile(); + } catch (IOException e) { + throw new RuntimeException("Failed to create temporary directory", e); + } + + // TO MAKE SAFE REWRITE TO (v2): + File tempSafe2; + try { + tempSafe2 = Files.createTempDirectory("random").toFile(); + } catch (IOException e) { + throw new RuntimeException("Failed to create temporary directory", e); + } + } + + void vulnerableFileCreateTempFileMkdirsTainted() { + // GIVEN: + File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child"); + + // VULNERABLE VERSION: + tempDirChild.mkdirs(); + + // TO MAKE SAFE REWRITE TO (v1): + File tempSafe1; + try { + tempSafe1 = Files.createTempDirectory(tempDirChild.toPath(), "random").toFile(); + } catch (IOException e) { + throw new RuntimeException("Failed to create temporary directory", e); + } + + // TO MAKE SAFE REWRITE TO (v2): + File tempSafe2; + try { + tempSafe2 = Files.createTempDirectory("random").toFile(); + } catch (IOException e) { + throw new RuntimeException("Failed to create temporary directory", e); + } + } + + void vulnerableFileCreateTempFilesWrite1() throws IOException { + // VULNERABLE VERSION: + File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child.txt"); + Files.write(tempDirChild.toPath(), Arrays.asList("secret"), StandardCharsets.UTF_8, StandardOpenOption.CREATE); + + // TO MAKE SAFE REWRITE TO (v1): + // Use this version if you care that the file has the exact path of `[java.io.tmpdir]/child.txt` + try { + Path tempSafe = Paths.get(System.getProperty("java.io.tmpdir"), "child.txt"); + Files.createFile(tempSafe, PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))); + Files.write(tempSafe, Arrays.asList("secret")); + } catch (IOException e) { + throw new RuntimeException("Failed to write temporary file", e); + } + + // TO MAKE SAFE REWRITE TO (v2): + // Use this version if you don't care that the file has an exact path. This will write to a file of the name `[java.io.tmpdir]/random[random string]child.txt` + try { + Path tempSafe = Files.createTempFile("random", "child.txt"); + Files.write(tempSafe, Arrays.asList("secret"), StandardCharsets.UTF_8, StandardOpenOption.CREATE); + } catch (IOException e) { + throw new RuntimeException("Failed to write temporary file", e); + } + } + + void vulnerableFileCreateTempFilesWrite2() throws IOException { + // GIVEN: + String secret = "secret"; + byte[] byteArrray = secret.getBytes(); + + // VULNERABLE VERSION: + File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child.txt"); + Files.write(tempDirChild.toPath(), byteArrray, StandardOpenOption.CREATE); + + // TO MAKE SAFE REWRITE TO (v1): + // Use this version if you care that the file has the exact path of `[java.io.tmpdir]/child.txt` + Path tempSafe1 = Paths.get(System.getProperty("java.io.tmpdir"), "child.txt"); + Files.createFile(tempSafe1, PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))); + Files.write(tempSafe1, byteArrray); + + // TO MAKE SAFE REWRITE TO (v2): + // Use this version if you don't care that the file has an exact path. This will write to a file of the name `[java.io.tmpdir]/random[random string]child.txt` + Path tempSafe2 = Files.createTempFile("random", "child.txt"); + Files.write(tempSafe2, byteArrray); + } + + void vulnerableFileCreateTempFilesNewBufferedWriter() throws IOException { + // GIVEN: + Path tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child-buffered-writer.txt").toPath(); + + // VULNERABLE VERSION: + Files.newBufferedWriter(tempDirChild); + + // TO MAKE SAFE REWRITE TO: + Files.createFile(tempDirChild, PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))); + Files.newBufferedWriter(tempDirChild); + } + + void vulnerableFileCreateTempFilesNewOutputStream() throws IOException { + // GIVEN: + Path tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child-output-stream.txt").toPath(); + + // VULNERABLE VERSION: + Files.newOutputStream(tempDirChild).close(); + + // TO MAKE SAFE REWRITE TO: + Files.createFile(tempDirChild, PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))); + Files.newOutputStream(tempDirChild).close(); + } + + void vulnerableFileCreateTempFilesCreateFile() throws IOException { + // GIVEN: + File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child-create-file.txt"); + + // VULNERABLE VERSION: + Files.createFile(tempDirChild.toPath()); + + // TO MAKE SAFE REWRITE TO: + Files.createFile(tempDirChild.toPath(), PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))); + } + + void safeFileCreateTempFilesCreateFile() throws IOException { + // Clear permissions intentions by setting the 'OWNER_READ' and 'OWNER_WRITE' + // permissions. + File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child-create-file.txt"); + Files.createFile( + tempDirChild.toPath(), + PosixFilePermissions + .asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))); + } + + void vulnerableFileCreateDirectory() throws IOException { + // GIVEN: + File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child-create-directory"); + + // VULNERABLE VERSION: + Files.createDirectory(tempDirChild.toPath()); // Creates with permissions 'drwxr-xr-x' + + // TO MAKE SAFE REWRITE TO: + Files.createDirectory(tempDirChild.toPath(), PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))); + } + + void vulnerableFileCreateDirectories() throws IOException { + // GIVEN: + File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child-create-directories/child"); + + // VULNERABLE VERSION: + Files.createDirectories(tempDirChild.toPath()); // Creates with permissions 'drwxr-xr-x' + + // TO MAKE SAFE REWRITE TO: + Files.createDirectories(tempDirChild.toPath(), PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))); + } + + void safeFileCreationWithPermissions() throws IOException { + File tempFile = File.createTempFile("temp", "file.txt"); + tempFile.setReadable(false, false); + tempFile.setReadable(true, true); + } + + void notVulnerableCreateOnSystemPropertyDir() throws IOException { + File tempDir = new File(System.getProperty("java.io.tmpdir")); + tempDir.mkdir(); + } + + void notVulnerableCreateOnSystemPropertyDirs() throws IOException { + File tempDir = new File(System.getProperty("java.io.tmpdir")); + tempDir.mkdirs(); + } +}