Skip to content

Commit de38638

Browse files
smowtonJLLeitschuh
authored andcommitted
Combine CWE-200 queries
1 parent 1f47ea5 commit de38638

8 files changed

Lines changed: 178 additions & 15 deletions

java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.inc.qhelp renamed to java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.qhelp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ directories that are shared between all users on the system.</p>
88

99
<p>On most <a href="https://en.wikipedia.org/wiki/Unix-like">unix-like</a> systems,
1010
the system temporary directory is shared between local users.
11-
If files/directories are created within the system temporary directory without using
12-
APIs that explicitly set the correct file permissions, local information disclosure
11+
If files/directories are created within the system temporary directory without using
12+
APIs that explicitly set the correct file permissions, local information disclosure
1313
can occur.</p>
1414

1515
<p>Depending upon the particular file contents exposed, this vulnerability can have a
@@ -45,4 +45,4 @@ For example: <code>PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePerm
4545
<li>OSWAP: <a href="https://owasp.org/www-community/vulnerabilities/Insecure_Temporary_File">Insecure Temporary File</a>.</li>
4646
<li>CERT: <a href="https://wiki.sei.cmu.edu/confluence/display/java/FIO00-J.+Do+not+operate+on+files+in+shared+directories">FIO00-J. Do not operate on files in shared directories</a></li>
4747
</references>
48-
</qhelp>
48+
</qhelp>
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
/**
2+
* @name Temporary Directory Local information disclosure
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 very-high
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+
17+
private class MethodFileSystemFileCreation extends Method {
18+
MethodFileSystemFileCreation() {
19+
getDeclaringType() instanceof TypeFile and
20+
hasName(["mkdir", "mkdirs", "createNewFile"])
21+
}
22+
}
23+
24+
abstract private class FileCreationSink extends DataFlow::Node { }
25+
26+
/**
27+
* Sink for tainted `File` having a file or directory creation method called on it.
28+
*/
29+
private class FileFileCreationSink extends FileCreationSink {
30+
FileFileCreationSink() {
31+
exists(MethodAccess ma |
32+
ma.getMethod() instanceof MethodFileSystemFileCreation and
33+
ma.getQualifier() = this.asExpr()
34+
)
35+
}
36+
}
37+
38+
/**
39+
* Sink for if tained File/Path having some `Files` method called on it that creates a file or directory.
40+
*/
41+
private class FilesFileCreationSink extends FileCreationSink {
42+
FilesFileCreationSink() {
43+
exists(FilesVulnerableCreationMethodAccess ma | ma.getArgument(0) = this.asExpr())
44+
}
45+
}
46+
47+
/**
48+
* Captures all of the vulnerable methods on `Files` that create files/directories without explicitly
49+
* setting the permissions.
50+
*/
51+
private class FilesVulnerableCreationMethodAccess extends MethodAccess {
52+
FilesVulnerableCreationMethodAccess() {
53+
exists(Method m |
54+
m = this.getMethod() and
55+
m.getDeclaringType().hasQualifiedName("java.nio.file", "Files")
56+
|
57+
m.hasName(["write", "newBufferedWriter", "newOutputStream"])
58+
or
59+
m.hasName(["createFile", "createDirectory", "createDirectories"]) and
60+
getNumArgument() = 1
61+
)
62+
}
63+
}
64+
65+
/**
66+
* A call to `java.io.File::createTempFile` where the the system temp dir sinks to the last argument.
67+
*/
68+
private class FileCreateTempFileSink extends FileCreationSink {
69+
FileCreateTempFileSink() {
70+
exists(MethodAccess ma |
71+
ma.getMethod() instanceof MethodFileCreateTempFile and ma.getArgument(2) = this.asExpr()
72+
)
73+
}
74+
}
75+
76+
private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Configuration {
77+
TempDirSystemGetPropertyToCreateConfig() { this = "TempDirSystemGetPropertyToCreateConfig" }
78+
79+
override predicate isSource(DataFlow::Node source) {
80+
source.asExpr() instanceof MethodAccessSystemGetPropertyTempDirTainted
81+
}
82+
83+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
84+
isAdditionalFileTaintStep(node1, node2)
85+
}
86+
87+
override predicate isSink(DataFlow::Node sink) { sink instanceof FileCreationSink }
88+
}
89+
90+
abstract class MethodAccessInsecureFileCreation extends MethodAccess {
91+
/**
92+
* Docstring describing the file system type (ie. file, directory, etc...) returned.
93+
*/
94+
abstract string getFileSystemType();
95+
}
96+
97+
/**
98+
* Insecure calls to `java.io.File::createTempFile`.
99+
*/
100+
class MethodAccessInsecureFileCreateTempFile extends MethodAccessInsecureFileCreation {
101+
MethodAccessInsecureFileCreateTempFile() {
102+
this.getMethod() instanceof MethodFileCreateTempFile and
103+
(
104+
this.getNumArgument() = 2
105+
or
106+
// Vulnerablilty exists when the last argument is `null`
107+
getArgument(2) instanceof NullLiteral
108+
)
109+
}
110+
111+
override string getFileSystemType() { result = "file" }
112+
}
113+
114+
class MethodGuavaFilesCreateTempFile extends Method {
115+
MethodGuavaFilesCreateTempFile() {
116+
getDeclaringType().hasQualifiedName("com.google.common.io", "Files") and
117+
hasName("createTempDir")
118+
}
119+
}
120+
121+
class MethodAccessInsecureGuavaFilesCreateTempFile extends MethodAccessInsecureFileCreation {
122+
MethodAccessInsecureGuavaFilesCreateTempFile() {
123+
getMethod() instanceof MethodGuavaFilesCreateTempFile
124+
}
125+
126+
override string getFileSystemType() { result = "directory" }
127+
}
128+
129+
/**
130+
* This is a hack: we include use of inherently insecure methods, which don't have any associated
131+
* flow path, in with results describing a path from reading java.io.tmpdir or similar to use
132+
* in a file creation op.
133+
*
134+
* We achieve this by making inherently-insecure method invocations both a source and a sink in
135+
* this configuration, resulting in a zero-length path which is type-compatible with the actual
136+
* path-flow results.
137+
*/
138+
class InsecureMethodPseudoConfiguration extends DataFlow::Configuration {
139+
InsecureMethodPseudoConfiguration() { this = "InsecureMethodPseudoConfiguration " }
140+
141+
override predicate isSource(DataFlow::Node node) {
142+
node.asExpr() instanceof MethodAccessInsecureFileCreation
143+
}
144+
145+
override predicate isSink(DataFlow::Node node) {
146+
node.asExpr() instanceof MethodAccessInsecureFileCreation
147+
}
148+
}
149+
150+
from DataFlow::PathNode source, DataFlow::PathNode sink, string message
151+
where
152+
any(TempDirSystemGetPropertyToCreateConfig conf).hasFlowPath(source, sink) and
153+
message =
154+
"Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users."
155+
or
156+
any(InsecureMethodPseudoConfiguration conf).hasFlowPath(source, sink) and
157+
// Note this message has no "$@" placeholder, so the "system temp directory" template parameter below is not used.
158+
message =
159+
"Local information disclosure vulnerability due to use of " +
160+
source.getNode().asExpr().(MethodAccessInsecureFileCreation).getFileSystemType() +
161+
" readable by other local users."
162+
select source.getNode(), source, sink, message, source.getNode(), "system temp directory"

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

Lines changed: 0 additions & 5 deletions
This file was deleted.

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

Lines changed: 0 additions & 5 deletions
This file was deleted.

java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosureFromSystemProperty.expected renamed to java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosure.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@ edges
3636
| Test.java:186:21:186:32 | tempDirChild : File | Test.java:186:21:186:41 | toPath(...) |
3737
| Test.java:202:29:202:104 | new File(...) : File | Test.java:202:29:202:113 | toPath(...) : Path |
3838
| Test.java:202:29:202:113 | toPath(...) : Path | Test.java:205:33:205:44 | tempDirChild |
39+
| Test.java:202:29:202:113 | toPath(...) : Path | Test.java:209:33:209:44 | tempDirChild |
3940
| Test.java:202:38:202:73 | getProperty(...) : String | Test.java:202:29:202:104 | new File(...) : File |
4041
| Test.java:214:29:214:102 | new File(...) : File | Test.java:214:29:214:111 | toPath(...) : Path |
4142
| Test.java:214:29:214:111 | toPath(...) : Path | Test.java:217:31:217:42 | tempDirChild |
43+
| Test.java:214:29:214:111 | toPath(...) : Path | Test.java:221:31:221:42 | tempDirChild |
4244
| Test.java:214:38:214:73 | getProperty(...) : String | Test.java:214:29:214:102 | new File(...) : File |
4345
| Test.java:226:29:226:100 | new File(...) : File | Test.java:229:26:229:37 | tempDirChild : File |
4446
| Test.java:226:38:226:73 | getProperty(...) : String | Test.java:226:29:226:100 | new File(...) : File |
@@ -58,6 +60,8 @@ nodes
5860
| Files.java:14:28:14:64 | new File(...) : File | semmle.label | new File(...) : File |
5961
| Files.java:14:37:14:43 | baseDir : File | semmle.label | baseDir : File |
6062
| Files.java:15:17:15:23 | tempDir | semmle.label | tempDir |
63+
| Test.java:18:25:18:61 | createTempFile(...) | semmle.label | createTempFile(...) |
64+
| Test.java:26:25:26:67 | createTempFile(...) | semmle.label | createTempFile(...) |
6165
| Test.java:34:24:34:69 | new File(...) : File | semmle.label | new File(...) : File |
6266
| Test.java:34:33:34:68 | getProperty(...) : String | semmle.label | getProperty(...) : String |
6367
| Test.java:37:63:37:69 | tempDir | semmle.label | tempDir |
@@ -71,6 +75,7 @@ nodes
7175
| Test.java:73:24:73:69 | new File(...) : File | semmle.label | new File(...) : File |
7276
| Test.java:73:33:73:68 | getProperty(...) : String | semmle.label | getProperty(...) : String |
7377
| Test.java:76:63:76:69 | tempDir | semmle.label | tempDir |
78+
| Test.java:95:24:95:65 | createTempDir(...) | semmle.label | createTempDir(...) |
7479
| Test.java:108:29:108:84 | new File(...) : File | semmle.label | new File(...) : File |
7580
| Test.java:108:38:108:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
7681
| Test.java:111:9:111:20 | tempDirChild | semmle.label | tempDirChild |
@@ -89,10 +94,12 @@ nodes
8994
| Test.java:202:29:202:113 | toPath(...) : Path | semmle.label | toPath(...) : Path |
9095
| Test.java:202:38:202:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
9196
| Test.java:205:33:205:44 | tempDirChild | semmle.label | tempDirChild |
97+
| Test.java:209:33:209:44 | tempDirChild | semmle.label | tempDirChild |
9298
| Test.java:214:29:214:102 | new File(...) : File | semmle.label | new File(...) : File |
9399
| Test.java:214:29:214:111 | toPath(...) : Path | semmle.label | toPath(...) : Path |
94100
| Test.java:214:38:214:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
95101
| Test.java:217:31:217:42 | tempDirChild | semmle.label | tempDirChild |
102+
| Test.java:221:31:221:42 | tempDirChild | semmle.label | tempDirChild |
96103
| Test.java:226:29:226:100 | new File(...) : File | semmle.label | new File(...) : File |
97104
| Test.java:226:38:226:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
98105
| Test.java:229:26:229:37 | tempDirChild : File | semmle.label | tempDirChild : File |
@@ -108,16 +115,21 @@ nodes
108115
subpaths
109116
#select
110117
| 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 |
118+
| 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 |
119+
| 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 |
111120
| 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 |
112121
| 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 |
113122
| 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 |
114123
| 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 |
124+
| 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 |
115125
| 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 |
116126
| 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 |
117127
| 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 |
118128
| 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 |
119129
| 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 |
130+
| Test.java:202:38:202:73 | getProperty(...) | Test.java:202:38:202:73 | getProperty(...) : String | Test.java:209:33:209: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 |
120131
| 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 |
132+
| Test.java:214:38:214:73 | getProperty(...) | Test.java:214:38:214:73 | getProperty(...) : String | Test.java:221:31:221: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 |
121133
| 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 |
122134
| 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 |
123135
| 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 |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-200/TempDirLocalInformationDisclosure.ql

java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosureFromMethodCall.qlref

Lines changed: 0 additions & 1 deletion
This file was deleted.

java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosureFromSystemProperty.qlref

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)