Skip to content

Commit 0268dd9

Browse files
committed
Add file creation sanitizer
1 parent 9299c79 commit 0268dd9

4 files changed

Lines changed: 140 additions & 116 deletions

File tree

java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosureFromMethodCall.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class MethodAccessInsecureFileCreateTempFile extends MethodAccessInsecureFileCre
3434
this.getNumArgument() = 2
3535
or
3636
// The default temporary directory is used when the last argument of `File.createTempFile(string, string, File)` is `null`
37-
getArgument(2) instanceof NullLiteral
37+
DataFlow::localExprFlow(any(NullLiteral n), getArgument(2))
3838
)
3939
}
4040

java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosureFromSystemProperty.ql

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ import DataFlow::PathGraph
1616

1717
private class MethodFileSystemFileCreation extends Method {
1818
MethodFileSystemFileCreation() {
19-
getDeclaringType() instanceof TypeFile and
20-
hasName(["mkdir", "mkdirs", "createNewFile"])
19+
this.getDeclaringType() instanceof TypeFile and
20+
this.hasName(["mkdir", "mkdirs", "createNewFile"])
2121
}
2222
}
2323

@@ -58,7 +58,26 @@ private class FilesVulnerableCreationMethodAccess extends MethodAccess {
5858
m.hasName(["write", "newBufferedWriter", "newOutputStream"])
5959
or
6060
m.hasName(["createFile", "createDirectory", "createDirectories"]) and
61-
getNumArgument() = 1
61+
this.getNumArgument() = 1
62+
or
63+
m.hasName("newByteChannel") and
64+
this.getNumArgument() = 2
65+
)
66+
}
67+
}
68+
69+
/**
70+
* A call to a `File` method that create files/directories with a specific set of permissions explicitly set.
71+
* We can safely assume that any calls to these methods with explicit `PosixFilePermissions.asFileAttribute` contains a certain level of intentionality behind it.
72+
*/
73+
private class FilesSanitiznignCreationMethodAccess extends MethodAccess {
74+
FilesSanitiznignCreationMethodAccess() {
75+
exists(Method m |
76+
m = this.getMethod() and
77+
m.getDeclaringType().hasQualifiedName("java.nio.file", "Files")
78+
|
79+
m.hasName(["createFile", "createDirectory", "createDirectories"]) and
80+
this.getNumArgument() = 2
6281
)
6382
}
6483
}
@@ -92,10 +111,16 @@ private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Conf
92111
}
93112

94113
override predicate isSink(DataFlow::Node sink) { sink instanceof FileCreationSink }
114+
115+
override predicate isSanitizer(DataFlow::Node sanitizer) {
116+
exists(FilesSanitiznignCreationMethodAccess sanitisingMethodAccess |
117+
sanitizer.asExpr() = sanitisingMethodAccess.getArgument(0)
118+
)
119+
}
95120
}
96121

97122
from DataFlow::PathNode source, DataFlow::PathNode sink, TempDirSystemGetPropertyToCreateConfig conf
98123
where conf.hasFlowPath(source, sink)
99-
select source.getNode(), source, sink,
124+
select sink.getNode(), source, sink,
100125
"Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users.",
101126
source.getNode(), "system temp directory"
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
| Test.java:15:21:15:57 | createTempFile(...) | Local information disclosure vulnerability due to use of file readable by other local users. |
2-
| Test.java:19:21:19:63 | createTempFile(...) | Local information disclosure vulnerability due to use of file readable by other local users. |
3-
| Test.java:49:24:49:65 | createTempDir(...) | Local information disclosure vulnerability due to use of directory readable by other local users. |
1+
| Test.java:18:25:18:61 | createTempFile(...) | Local information disclosure vulnerability due to use of file readable by other local users. |
2+
| Test.java:26:25:26:67 | createTempFile(...) | Local information disclosure vulnerability due to use of file readable by other local users. |
3+
| Test.java:95:24:95:65 | createTempDir(...) | Local information disclosure vulnerability due to use of directory readable by other local users. |

0 commit comments

Comments
 (0)