Skip to content

Commit 6deb3d8

Browse files
JLLeitschuhJLLeitschuh
committed
Add java.nio.file.Files API checks
1 parent 14dfa3b commit 6deb3d8

File tree

4 files changed

+103
-17
lines changed

4 files changed

+103
-17
lines changed

‎java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure2.ql

+13-14
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,7 @@ private class MethodFileSystemFileCreation extends Method {
2222
}
2323
}
2424

25-
private class MethodFilesSystemFileCreation extends Method {
26-
MethodFilesSystemFileCreation() {
27-
getDeclaringType().hasQualifiedName("java.nio.file", "Files") and
28-
hasName("write")
29-
}
30-
}
31-
32-
private abstract class FileCreationSink extends DataFlow::Node {}
25+
abstract private class FileCreationSink extends DataFlow::Node { }
3326

3427
private class FileFileCreationSink extends FileCreationSink {
3528
FileFileCreationSink() {
@@ -42,9 +35,17 @@ private class FileFileCreationSink extends FileCreationSink {
4235

4336
private class FilesFileCreationSink extends FileCreationSink {
4437
FilesFileCreationSink() {
45-
exists(MethodAccess ma |
46-
ma.getMethod() instanceof MethodFilesSystemFileCreation and
47-
ma.getArgument(0) = this.asExpr()
38+
exists(FilesVulnerableCreationMethodAccess ma | ma.getArgument(0) = this.asExpr())
39+
}
40+
}
41+
42+
private class FilesVulnerableCreationMethodAccess extends MethodAccess {
43+
FilesVulnerableCreationMethodAccess() {
44+
getMethod().getDeclaringType().hasQualifiedName("java.nio.file", "Files") and
45+
(
46+
getMethod().hasName(["write", "newBufferedWriter", "newOutputStream"])
47+
or
48+
getMethod().hasName(["createFile", "createDirectory", "createDirectories"]) and getNumArgument() = 1
4849
)
4950
}
5051
}
@@ -60,9 +61,7 @@ private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Conf
6061
isAdditionalFileTaintStep(node1, node2)
6162
}
6263

63-
override predicate isSink(DataFlow::Node sink) {
64-
sink instanceof FileCreationSink
65-
}
64+
override predicate isSink(DataFlow::Node sink) { sink instanceof FileCreationSink }
6665
}
6766

6867
from DataFlow::PathNode source, DataFlow::PathNode sink, TempDirSystemGetPropertyToCreateConfig conf

‎java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosure1.expected

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
| Test.java:15:21:15:57 | createTempFile(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
2+
| Test.java:19:21:19:63 | createTempFile(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
3+
| Test.java:24:21:24:66 | createTempFile(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
4+
| Test.java:29:21:29:71 | createTempFile(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
5+
| Test.java:34:21:34:66 | createTempFile(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
6+
| Test.java:39:21:39:66 | createTempFile(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
7+
| Test.java:49:24:49:65 | createTempDir(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |

‎java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosure2.expected

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
edges
2+
| Files.java:10:33:10:68 | getProperty(...) : String | Files.java:15:17:15:23 | tempDir |
3+
| Test.java:53:38:53:73 | getProperty(...) : String | Test.java:54:9:54:20 | tempDirChild |
4+
| Test.java:58:38:58:73 | getProperty(...) : String | Test.java:59:9:59:20 | tempDirChild |
5+
| Test.java:63:38:63:73 | getProperty(...) : String | Test.java:64:21:64:41 | toPath(...) |
6+
| Test.java:68:38:68:73 | getProperty(...) : String | Test.java:71:21:71:41 | toPath(...) |
7+
| Test.java:75:38:75:73 | getProperty(...) : String | Test.java:76:33:76:53 | toPath(...) |
8+
| Test.java:80:38:80:73 | getProperty(...) : String | Test.java:81:31:81:51 | toPath(...) |
9+
| Test.java:85:38:85:73 | getProperty(...) : String | Test.java:86:26:86:46 | toPath(...) |
10+
| Test.java:98:38:98:73 | getProperty(...) : String | Test.java:99:31:99:51 | toPath(...) |
11+
| Test.java:103:38:103:73 | getProperty(...) : String | Test.java:104:33:104:53 | toPath(...) |
12+
nodes
13+
| Files.java:10:33:10:68 | getProperty(...) : String | semmle.label | getProperty(...) : String |
14+
| Files.java:15:17:15:23 | tempDir | semmle.label | tempDir |
15+
| Test.java:53:38:53:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
16+
| Test.java:54:9:54:20 | tempDirChild | semmle.label | tempDirChild |
17+
| Test.java:58:38:58:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
18+
| Test.java:59:9:59:20 | tempDirChild | semmle.label | tempDirChild |
19+
| Test.java:63:38:63:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
20+
| Test.java:64:21:64:41 | toPath(...) | semmle.label | toPath(...) |
21+
| Test.java:68:38:68:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
22+
| Test.java:71:21:71:41 | toPath(...) | semmle.label | toPath(...) |
23+
| Test.java:75:38:75:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
24+
| Test.java:76:33:76:53 | toPath(...) | semmle.label | toPath(...) |
25+
| Test.java:80:38:80:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
26+
| Test.java:81:31:81:51 | toPath(...) | semmle.label | toPath(...) |
27+
| Test.java:85:38:85:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
28+
| Test.java:86:26:86:46 | toPath(...) | semmle.label | toPath(...) |
29+
| Test.java:98:38:98:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
30+
| Test.java:99:31:99:51 | toPath(...) | semmle.label | toPath(...) |
31+
| Test.java:103:38:103:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
32+
| Test.java:104:33:104:53 | toPath(...) | semmle.label | toPath(...) |
33+
#select
34+
| 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 |
35+
| Test.java:53:38:53:73 | getProperty(...) | Test.java:53:38:53:73 | getProperty(...) : String | Test.java:54:9:54:20 | tempDirChild | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:53:38:53:73 | getProperty(...) | system temp directory |
36+
| Test.java:58:38:58:73 | getProperty(...) | Test.java:58:38:58:73 | getProperty(...) : String | Test.java:59:9:59:20 | tempDirChild | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:58:38:58:73 | getProperty(...) | system temp directory |
37+
| Test.java:63:38:63:73 | getProperty(...) | Test.java:63:38:63:73 | getProperty(...) : String | Test.java:64:21:64:41 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:63:38:63:73 | getProperty(...) | system temp directory |
38+
| Test.java:68:38:68:73 | getProperty(...) | Test.java:68:38:68:73 | getProperty(...) : String | Test.java:71:21:71:41 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:68:38:68:73 | getProperty(...) | system temp directory |
39+
| Test.java:75:38:75:73 | getProperty(...) | Test.java:75:38:75:73 | getProperty(...) : String | Test.java:76:33:76:53 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:75:38:75:73 | getProperty(...) | system temp directory |
40+
| Test.java:80:38:80:73 | getProperty(...) | Test.java:80:38:80:73 | getProperty(...) : String | Test.java:81:31:81:51 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:80:38:80:73 | getProperty(...) | system temp directory |
41+
| Test.java:85:38:85:73 | getProperty(...) | Test.java:85:38:85:73 | getProperty(...) : String | Test.java:86:26:86:46 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:85:38:85:73 | getProperty(...) | system temp directory |
42+
| Test.java:98:38:98:73 | getProperty(...) | Test.java:98:38:98:73 | getProperty(...) : String | Test.java:99:31:99:51 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:98:38:98:73 | getProperty(...) | system temp directory |
43+
| Test.java:103:38:103:73 | getProperty(...) | Test.java:103:38:103:73 | getProperty(...) : String | Test.java:104:33:104:53 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:103:38:103:73 | getProperty(...) | system temp directory |

‎java/ql/test/query-tests/security/CWE-200/semmle/tests/Test.java

+40-3
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11

22
import java.util.Arrays;
33
import java.io.File;
4+
import java.io.IOException;
45
import java.nio.file.Files;
56
import java.nio.charset.StandardCharsets;
67
import java.nio.file.StandardOpenOption;
8+
import java.nio.file.attribute.PosixFilePermission;
9+
import java.nio.file.attribute.PosixFilePermissions;
10+
import java.util.EnumSet;
711

812
public class Test {
913

@@ -52,18 +56,51 @@ void vulnerableFileCreateTempFileMkdirTainted() {
5256

5357
void vulnerableFileCreateTempFileMkdirsTainted() {
5458
File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child");
55-
tempDirChild.mkdir();
59+
tempDirChild.mkdirs();
5660
}
5761

5862
void vulnerableFileCreateTempFilesWrite1() {
59-
File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child");
63+
File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child.txt");
6064
Files.write(tempDirChild.toPath(), Arrays.asList("secret"), StandardCharsets.UTF_8, StandardOpenOption.CREATE);
6165
}
6266

6367
void vulnerableFileCreateTempFilesWrite2() {
64-
File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child");
68+
File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child.txt");
6569
String secret = "secret";
6670
byte[] byteArrray = secret.getBytes();
6771
Files.write(tempDirChild.toPath(), byteArrray, StandardOpenOption.CREATE);
6872
}
73+
74+
void vulnerableFileCreateTempFilesNewBufferedWriter() throws IOException {
75+
File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child-buffered-writer.txt");
76+
Files.newBufferedWriter(tempDirChild.toPath());
77+
}
78+
79+
void vulnerableFileCreateTempFilesNewOutputStream() throws IOException {
80+
File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child-output-stream.txt");
81+
Files.newOutputStream(tempDirChild.toPath()).close();
82+
}
83+
84+
void vulnerableFileCreateTempFilesCreateFile() throws IOException {
85+
File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child-create-file.txt");
86+
Files.createFile(tempDirChild.toPath());
87+
}
88+
89+
void safeFileCreateTempFilesCreateFile() throws IOException {
90+
File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child-create-file.txt");
91+
Files.createFile(
92+
tempDirChild.toPath(),
93+
PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))
94+
);
95+
}
96+
97+
void vulnerableFileCreateDirectory() throws IOException {
98+
File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child-create-directory");
99+
Files.createDirectory(tempDirChild.toPath());
100+
}
101+
102+
void vulnerableFileCreateDirectories() throws IOException {
103+
File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child-create-directories/child");
104+
Files.createDirectories(tempDirChild.toPath());
105+
}
69106
}

0 commit comments

Comments
 (0)