|
| 1 | +/** |
| 2 | + * @name Local information disclosure in a temporary directory |
| 3 | + * @description Writing information without explicit permissions to a shared temporary directory may disclose it to other users. |
| 4 | + * @kind path-problem |
| 5 | + * @problem.severity warning |
| 6 | + * @precision medium |
| 7 | + * @id java/local-temp-file-or-directory-information-disclosure |
| 8 | + * @tags security |
| 9 | + * external/cwe/cwe-200 |
| 10 | + * external/cwe/cwe-732 |
| 11 | + */ |
| 12 | + |
| 13 | +import java |
| 14 | +import TempDirUtils |
| 15 | +import DataFlow::PathGraph |
| 16 | +import semmle.code.java.dataflow.TaintTracking2 |
| 17 | + |
| 18 | +abstract private class MethodFileSystemFileCreation extends Method { |
| 19 | + MethodFileSystemFileCreation() { this.getDeclaringType() instanceof TypeFile } |
| 20 | +} |
| 21 | + |
| 22 | +private class MethodFileDirectoryCreation extends MethodFileSystemFileCreation { |
| 23 | + MethodFileDirectoryCreation() { this.hasName(["mkdir", "mkdirs"]) } |
| 24 | +} |
| 25 | + |
| 26 | +private class MethodFileFileCreation extends MethodFileSystemFileCreation { |
| 27 | + MethodFileFileCreation() { this.hasName("createNewFile") } |
| 28 | +} |
| 29 | + |
| 30 | +abstract private class FileCreationSink extends DataFlow::Node { } |
| 31 | + |
| 32 | +/** |
| 33 | + * The qualifier of a call to one of `File`'s file-creating or directory-creating methods, |
| 34 | + * treated as a sink by `TempDirSystemGetPropertyToCreateConfig`. |
| 35 | + */ |
| 36 | +private class FileFileCreationSink extends FileCreationSink { |
| 37 | + FileFileCreationSink() { |
| 38 | + exists(MethodAccess ma | |
| 39 | + ma.getMethod() instanceof MethodFileSystemFileCreation and |
| 40 | + ma.getQualifier() = this.asExpr() |
| 41 | + ) |
| 42 | + } |
| 43 | +} |
| 44 | + |
| 45 | +/** |
| 46 | + * The argument to a call to one of `Files` file-creating or directory-creating methods, |
| 47 | + * treated as a sink by `TempDirSystemGetPropertyToCreateConfig`. |
| 48 | + */ |
| 49 | +private class FilesFileCreationSink extends FileCreationSink { |
| 50 | + FilesFileCreationSink() { |
| 51 | + exists(FilesVulnerableCreationMethodAccess ma | ma.getArgument(0) = this.asExpr()) |
| 52 | + } |
| 53 | +} |
| 54 | + |
| 55 | +/** |
| 56 | + * A call to a `Files` method that create files/directories without explicitly |
| 57 | + * setting the newly-created file or directory's permissions. |
| 58 | + */ |
| 59 | +private class FilesVulnerableCreationMethodAccess extends MethodAccess { |
| 60 | + FilesVulnerableCreationMethodAccess() { |
| 61 | + exists(Method m | |
| 62 | + m = this.getMethod() and |
| 63 | + m.getDeclaringType().hasQualifiedName("java.nio.file", "Files") |
| 64 | + | |
| 65 | + m.hasName(["write", "newBufferedWriter", "newOutputStream"]) |
| 66 | + or |
| 67 | + m.hasName(["createFile", "createDirectory", "createDirectories"]) and |
| 68 | + this.getNumArgument() = 1 |
| 69 | + or |
| 70 | + m.hasName("newByteChannel") and |
| 71 | + this.getNumArgument() = 2 |
| 72 | + ) |
| 73 | + } |
| 74 | +} |
| 75 | + |
| 76 | +/** |
| 77 | + * A call to a `File` method that create files/directories with a specific set of permissions explicitly set. |
| 78 | + * We can safely assume that any calls to these methods with explicit `PosixFilePermissions.asFileAttribute` |
| 79 | + * contains a certain level of intentionality behind it. |
| 80 | + */ |
| 81 | +private class FilesSanitizingCreationMethodAccess extends MethodAccess { |
| 82 | + FilesSanitizingCreationMethodAccess() { |
| 83 | + exists(Method m | |
| 84 | + m = this.getMethod() and |
| 85 | + m.getDeclaringType().hasQualifiedName("java.nio.file", "Files") |
| 86 | + | |
| 87 | + m.hasName(["createFile", "createDirectory", "createDirectories"]) and |
| 88 | + this.getNumArgument() = 2 |
| 89 | + ) |
| 90 | + } |
| 91 | +} |
| 92 | + |
| 93 | +/** |
| 94 | + * The temp directory argument to a call to `java.io.File::createTempFile`, |
| 95 | + * treated as a sink by `TempDirSystemGetPropertyToCreateConfig`. |
| 96 | + */ |
| 97 | +private class FileCreateTempFileSink extends FileCreationSink { |
| 98 | + FileCreateTempFileSink() { |
| 99 | + exists(MethodAccess ma | |
| 100 | + ma.getMethod() instanceof MethodFileCreateTempFile and ma.getArgument(2) = this.asExpr() |
| 101 | + ) |
| 102 | + } |
| 103 | +} |
| 104 | + |
| 105 | +private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Configuration { |
| 106 | + TempDirSystemGetPropertyToCreateConfig() { this = "TempDirSystemGetPropertyToCreateConfig" } |
| 107 | + |
| 108 | + override predicate isSource(DataFlow::Node source) { |
| 109 | + source.asExpr() instanceof MethodAccessSystemGetPropertyTempDirTainted |
| 110 | + } |
| 111 | + |
| 112 | + /** |
| 113 | + * Find dataflow from the temp directory system property to the `File` constructor. |
| 114 | + * Examples: |
| 115 | + * - `new File(System.getProperty("java.io.tmpdir"))` |
| 116 | + * - `new File(new File(System.getProperty("java.io.tmpdir")), "/child")` |
| 117 | + */ |
| 118 | + override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { |
| 119 | + isAdditionalFileTaintStep(node1, node2) |
| 120 | + } |
| 121 | + |
| 122 | + override predicate isSink(DataFlow::Node sink) { |
| 123 | + sink instanceof FileCreationSink and |
| 124 | + not any(TempDirSystemGetPropertyDirectlyToMkdirConfig config).hasFlowTo(sink) |
| 125 | + } |
| 126 | + |
| 127 | + override predicate isSanitizer(DataFlow::Node sanitizer) { |
| 128 | + exists(FilesSanitizingCreationMethodAccess sanitisingMethodAccess | |
| 129 | + sanitizer.asExpr() = sanitisingMethodAccess.getArgument(0) |
| 130 | + ) |
| 131 | + } |
| 132 | +} |
| 133 | + |
| 134 | +/** |
| 135 | + * Configuration that tracks calls to to `mkdir` or `mkdirs` that are are directly on the temp directory system property. |
| 136 | + * Examples: |
| 137 | + * - `File tempDir = new File(System.getProperty("java.io.tmpdir")); tempDir.mkdir();` |
| 138 | + * - `File tempDir = new File(System.getProperty("java.io.tmpdir")); tempDir.mkdirs();` |
| 139 | + * |
| 140 | + * These are examples of code that is simply verifying that the temp directory exists. |
| 141 | + * As such, this code pattern is filtered out as an explicit vulnerability in |
| 142 | + * `TempDirSystemGetPropertyToCreateConfig::isSink`. |
| 143 | + */ |
| 144 | +private class TempDirSystemGetPropertyDirectlyToMkdirConfig extends TaintTracking2::Configuration { |
| 145 | + TempDirSystemGetPropertyDirectlyToMkdirConfig() { |
| 146 | + this = "TempDirSystemGetPropertyDirectlyToMkdirConfig" |
| 147 | + } |
| 148 | + |
| 149 | + override predicate isSource(DataFlow::Node node) { |
| 150 | + exists( |
| 151 | + MethodAccessSystemGetPropertyTempDirTainted propertyGetMethodAccess, DataFlow::Node callSite |
| 152 | + | |
| 153 | + DataFlow::localFlow(DataFlow::exprNode(propertyGetMethodAccess), callSite) |
| 154 | + | |
| 155 | + isFileConstructorArgument(callSite.asExpr(), node.asExpr(), 1) |
| 156 | + ) |
| 157 | + } |
| 158 | + |
| 159 | + override predicate isSink(DataFlow::Node node) { |
| 160 | + exists(MethodAccess ma | ma.getMethod() instanceof MethodFileDirectoryCreation | |
| 161 | + ma.getQualifier() = node.asExpr() |
| 162 | + ) |
| 163 | + } |
| 164 | + |
| 165 | + override predicate isSanitizer(DataFlow::Node sanitizer) { |
| 166 | + isFileConstructorArgument(sanitizer.asExpr(), _, _) |
| 167 | + } |
| 168 | +} |
| 169 | + |
| 170 | +// |
| 171 | +// Begin configuration for tracking single-method calls that are vulnerable. |
| 172 | +// |
| 173 | +/** |
| 174 | + * A `MethodAccess` against a method that creates a temporary file or directory in a shared temporary directory. |
| 175 | + */ |
| 176 | +abstract class MethodAccessInsecureFileCreation extends MethodAccess { |
| 177 | + /** |
| 178 | + * Gets the type of entity created (e.g. `file`, `directory`, ...). |
| 179 | + */ |
| 180 | + abstract string getFileSystemEntityType(); |
| 181 | +} |
| 182 | + |
| 183 | +/** |
| 184 | + * An insecure call to `java.io.File.createTempFile`. |
| 185 | + */ |
| 186 | +class MethodAccessInsecureFileCreateTempFile extends MethodAccessInsecureFileCreation { |
| 187 | + MethodAccessInsecureFileCreateTempFile() { |
| 188 | + this.getMethod() instanceof MethodFileCreateTempFile and |
| 189 | + ( |
| 190 | + // `File.createTempFile(string, string)` always uses the default temporary directory |
| 191 | + this.getNumArgument() = 2 |
| 192 | + or |
| 193 | + // The default temporary directory is used when the last argument of `File.createTempFile(string, string, File)` is `null` |
| 194 | + DataFlow::localExprFlow(any(NullLiteral n), this.getArgument(2)) |
| 195 | + ) |
| 196 | + } |
| 197 | + |
| 198 | + override string getFileSystemEntityType() { result = "file" } |
| 199 | +} |
| 200 | + |
| 201 | +/** |
| 202 | + * The `com.google.common.io.Files.createTempDir` method. |
| 203 | + */ |
| 204 | +class MethodGuavaFilesCreateTempFile extends Method { |
| 205 | + MethodGuavaFilesCreateTempFile() { |
| 206 | + this.getDeclaringType().hasQualifiedName("com.google.common.io", "Files") and |
| 207 | + this.hasName("createTempDir") |
| 208 | + } |
| 209 | +} |
| 210 | + |
| 211 | +/** |
| 212 | + * A call to the `com.google.common.io.Files.createTempDir` method. |
| 213 | + */ |
| 214 | +class MethodAccessInsecureGuavaFilesCreateTempFile extends MethodAccessInsecureFileCreation { |
| 215 | + MethodAccessInsecureGuavaFilesCreateTempFile() { |
| 216 | + this.getMethod() instanceof MethodGuavaFilesCreateTempFile |
| 217 | + } |
| 218 | + |
| 219 | + override string getFileSystemEntityType() { result = "directory" } |
| 220 | +} |
| 221 | + |
| 222 | +/** |
| 223 | + * A hack: we include use of inherently insecure methods, which don't have any associated |
| 224 | + * flow path, in with results describing a path from reading `java.io.tmpdir` or similar to use |
| 225 | + * in a file creation op. |
| 226 | + * |
| 227 | + * We achieve this by making inherently-insecure method invocations both a source and a sink in |
| 228 | + * this configuration, resulting in a zero-length path which is type-compatible with the actual |
| 229 | + * path-flow results. |
| 230 | + */ |
| 231 | +class InsecureMethodPseudoConfiguration extends DataFlow::Configuration { |
| 232 | + InsecureMethodPseudoConfiguration() { this = "InsecureMethodPseudoConfiguration" } |
| 233 | + |
| 234 | + override predicate isSource(DataFlow::Node node) { |
| 235 | + node.asExpr() instanceof MethodAccessInsecureFileCreation |
| 236 | + } |
| 237 | + |
| 238 | + override predicate isSink(DataFlow::Node node) { |
| 239 | + node.asExpr() instanceof MethodAccessInsecureFileCreation |
| 240 | + } |
| 241 | +} |
| 242 | + |
| 243 | +from DataFlow::PathNode source, DataFlow::PathNode sink, string message |
| 244 | +where |
| 245 | + ( |
| 246 | + any(TempDirSystemGetPropertyToCreateConfig conf).hasFlowPath(source, sink) and |
| 247 | + message = |
| 248 | + "Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users." |
| 249 | + or |
| 250 | + any(InsecureMethodPseudoConfiguration conf).hasFlowPath(source, sink) and |
| 251 | + // Note this message has no "$@" placeholder, so the "system temp directory" template parameter below is not used. |
| 252 | + message = |
| 253 | + "Local information disclosure vulnerability due to use of " + |
| 254 | + source.getNode().asExpr().(MethodAccessInsecureFileCreation).getFileSystemEntityType() + |
| 255 | + " readable by other local users." |
| 256 | + ) and |
| 257 | + not isPermissionsProtectedTempDirUse(sink.getNode()) |
| 258 | +select source.getNode(), source, sink, message, source.getNode(), "system temp directory" |
0 commit comments