Skip to content

Commit 641c5df

Browse files
intrigusintrigus
committed
Centralize and model additional path creations.
1 parent e167b87 commit 641c5df

File tree

7 files changed

+264
-76
lines changed

7 files changed

+264
-76
lines changed

‎java/ql/src/Security/CWE/CWE-022/PathsCommon.qll

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

‎java/ql/src/Security/CWE/CWE-022/TaintedPath.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
import java
1616
import semmle.code.java.dataflow.FlowSources
17-
import PathsCommon
17+
import semmle.code.java.security.PathCreation
1818
import DataFlow::PathGraph
1919

2020
class ContainsDotDotSanitizer extends DataFlow::BarrierGuard {

‎java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
import java
1616
import semmle.code.java.dataflow.FlowSources
17-
import PathsCommon
17+
import semmle.code.java.security.PathCreation
1818
import DataFlow::PathGraph
1919

2020
class TaintedPathLocalConfig extends TaintTracking::Configuration {

‎java/ql/src/semmle/code/java/security/PathCreation.qll

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
/**
2+
* Models the different ways to create paths. Either by using `java.io.File`-related APIs or `java.nio.file.Path`-related APIs.
3+
*/
4+
5+
import java
6+
import semmle.code.java.controlflow.Guards
7+
8+
/** Models the creation of a path. */
9+
abstract class PathCreation extends Expr {
10+
/** Gets an input that is used in the creation of this path. */
11+
abstract Expr getInput();
12+
}
13+
14+
/** Models the `java.nio.file.Paths.get` method. */
15+
class PathsGet extends PathCreation, MethodAccess {
16+
PathsGet() {
17+
exists(Method m | m = this.getMethod() |
18+
m.getDeclaringType() instanceof TypePaths and
19+
m.getName() = "get"
20+
)
21+
}
22+
23+
override Expr getInput() { result = this.getAnArgument() }
24+
}
25+
26+
/** Models the `java.nio.file.FileSystem.getPath` method. */
27+
class FileSystemGetPath extends PathCreation, MethodAccess {
28+
FileSystemGetPath() {
29+
exists(Method m | m = this.getMethod() |
30+
m.getDeclaringType() instanceof TypeFileSystem and
31+
m.getName() = "getPath"
32+
)
33+
}
34+
35+
override Expr getInput() { result = this.getAnArgument() }
36+
}
37+
38+
/** Models the `new java.io.File(...)` constructor. */
39+
class FileCreation extends PathCreation, ClassInstanceExpr {
40+
FileCreation() { this.getConstructedType() instanceof TypeFile }
41+
42+
override Expr getInput() {
43+
result = this.getAnArgument() and
44+
// Relevant arguments include those that are not a `File`.
45+
not result.getType() instanceof TypeFile
46+
}
47+
}
48+
49+
/** Models the `java.nio.file.Path.resolveSibling` method. */
50+
class PathResolveSiblingCreation extends PathCreation, MethodAccess {
51+
PathResolveSiblingCreation() {
52+
exists(Method m | m = this.getMethod() |
53+
m.getDeclaringType() instanceof TypePath and
54+
m.getName() = "resolveSibling"
55+
)
56+
}
57+
58+
override Expr getInput() {
59+
result = this.getAnArgument() and
60+
// Relevant arguments are those of type `String`.
61+
result.getType() instanceof TypeString
62+
}
63+
}
64+
65+
/** Models the `java.nio.file.Path.resolve` method. */
66+
class PathResolveCreation extends PathCreation, MethodAccess {
67+
PathResolveCreation() {
68+
exists(Method m | m = this.getMethod() |
69+
m.getDeclaringType() instanceof TypePath and
70+
m.getName() = "resolve"
71+
)
72+
}
73+
74+
override Expr getInput() {
75+
result = this.getAnArgument() and
76+
// Relevant arguments are those of type `String`.
77+
result.getType() instanceof TypeString
78+
}
79+
}
80+
81+
/** Models the `java.nio.file.Path.of` method. */
82+
class PathOfCreation extends PathCreation, MethodAccess {
83+
PathOfCreation() {
84+
exists(Method m | m = this.getMethod() |
85+
m.getDeclaringType() instanceof TypePath and
86+
m.getName() = "of"
87+
)
88+
}
89+
90+
override Expr getInput() { result = this.getAnArgument() }
91+
}
92+
93+
/** Models the `new java.io.FileWriter(...)` constructor. */
94+
class FileWriterCreation extends PathCreation, ClassInstanceExpr {
95+
FileWriterCreation() { this.getConstructedType().hasQualifiedName("java.io", "FileWriter") }
96+
97+
override Expr getInput() {
98+
result = this.getAnArgument() and
99+
// Relevant arguments are those of type `String`.
100+
result.getType() instanceof TypeString
101+
}
102+
}
103+
104+
/** Models the `new java.io.FileReader(...)` constructor. */
105+
class FileReaderCreation extends PathCreation, ClassInstanceExpr {
106+
FileReaderCreation() { this.getConstructedType().hasQualifiedName("java.io", "FileReader") }
107+
108+
override Expr getInput() {
109+
result = this.getAnArgument() and
110+
// Relevant arguments are those of type `String`.
111+
result.getType() instanceof TypeString
112+
}
113+
}
114+
115+
/** Models the `new java.io.FileInputStream(...)` constructor. */
116+
class FileInputStreamCreation extends PathCreation, ClassInstanceExpr {
117+
FileInputStreamCreation() {
118+
this.getConstructedType().hasQualifiedName("java.io", "FileInputStream")
119+
}
120+
121+
override Expr getInput() {
122+
result = this.getAnArgument() and
123+
// Relevant arguments are those of type `String`.
124+
result.getType() instanceof TypeString
125+
}
126+
}
127+
128+
/** Models the `new java.io.FileOutputStream(...)` constructor. */
129+
class FileOutputStreamCreation extends PathCreation, ClassInstanceExpr {
130+
FileOutputStreamCreation() {
131+
this.getConstructedType().hasQualifiedName("java.io", "FileOutputStream")
132+
}
133+
134+
override Expr getInput() {
135+
result = this.getAnArgument() and
136+
// Relevant arguments are those of type `String`.
137+
result.getType() instanceof TypeString
138+
}
139+
}
140+
141+
private predicate inWeakCheck(Expr e) {
142+
// None of these are sufficient to guarantee that a string is safe.
143+
exists(MethodAccess m, Method def | m.getQualifier() = e and m.getMethod() = def |
144+
def.getName() = "startsWith" or
145+
def.getName() = "endsWith" or
146+
def.getName() = "isEmpty" or
147+
def.getName() = "equals"
148+
)
149+
or
150+
// Checking against `null` has no bearing on path traversal.
151+
exists(EqualityTest b | b.getAnOperand() = e | b.getAnOperand() instanceof NullLiteral)
152+
}
153+
154+
// Ignore cases where the variable has been checked somehow,
155+
// but allow some particularly obviously bad cases.
156+
predicate guarded(VarAccess e) {
157+
exists(PathCreation p | e = p.getInput()) and
158+
exists(ConditionBlock cb, Expr c |
159+
cb.getCondition().getAChildExpr*() = c and
160+
c = e.getVariable().getAnAccess() and
161+
cb.controls(e.getBasicBlock(), true) and
162+
// Disallow a few obviously bad checks.
163+
not inWeakCheck(c)
164+
)
165+
}

‎java/ql/test/library-tests/pathcreation/PathCreation.expected

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
| PathCreation.java:13:18:13:32 | new File(...) |
2+
| PathCreation.java:14:19:14:40 | new File(...) |
3+
| PathCreation.java:18:18:18:49 | new File(...) |
4+
| PathCreation.java:18:27:18:41 | new File(...) |
5+
| PathCreation.java:22:18:22:41 | new File(...) |
6+
| PathCreation.java:26:18:26:31 | of(...) |
7+
| PathCreation.java:27:19:27:39 | of(...) |
8+
| PathCreation.java:31:18:31:40 | of(...) |
9+
| PathCreation.java:35:18:35:33 | get(...) |
10+
| PathCreation.java:36:19:36:41 | get(...) |
11+
| PathCreation.java:40:18:40:42 | get(...) |
12+
| PathCreation.java:44:18:44:56 | getPath(...) |
13+
| PathCreation.java:45:19:45:64 | getPath(...) |
14+
| PathCreation.java:49:18:49:31 | of(...) |
15+
| PathCreation.java:49:18:49:53 | resolveSibling(...) |
16+
| PathCreation.java:53:18:53:31 | of(...) |
17+
| PathCreation.java:53:18:53:46 | resolve(...) |
18+
| PathCreation.java:57:25:57:45 | new FileWriter(...) |
19+
| PathCreation.java:61:25:61:45 | new FileReader(...) |
20+
| PathCreation.java:65:32:65:58 | new FileOutputStream(...) |
21+
| PathCreation.java:69:31:69:56 | new FileInputStream(...) |

‎java/ql/test/library-tests/pathcreation/PathCreation.java

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import java.io.File;
2+
import java.io.FileWriter;
3+
import java.io.FileReader;
4+
import java.io.FileOutputStream;
5+
import java.io.FileInputStream;
6+
import java.nio.file.Path;
7+
import java.nio.file.Paths;
8+
import java.nio.file.FileSystems;
9+
import java.net.URI;
10+
11+
class PathCreation {
12+
public void testNewFileWithString() {
13+
File f = new File("dir");
14+
File f2 = new File("dir", "sub");
15+
}
16+
17+
public void testNewFileWithFileString() {
18+
File f = new File(new File("dir"), "sub");
19+
}
20+
21+
public void testNewFileWithURI() {
22+
File f = new File(new URI("dir"));
23+
}
24+
25+
public void testPathOfWithString() {
26+
Path p = Path.of("dir");
27+
Path p2 = Path.of("dir", "sub");
28+
}
29+
30+
public void testPathOfWithURI() {
31+
Path p = Path.of(new URI("dir"));
32+
}
33+
34+
public void testPathsGetWithString() {
35+
Path p = Paths.get("dir");
36+
Path p2 = Paths.get("dir", "sub");
37+
}
38+
39+
public void testPathsGetWithURI() {
40+
Path p = Paths.get(new URI("dir"));
41+
}
42+
43+
public void testFileSystemGetPathWithString() {
44+
Path p = FileSystems.getDefault().getPath("dir");
45+
Path p2 = FileSystems.getDefault().getPath("dir", "sub");
46+
}
47+
48+
public void testPathResolveSiblingWithString() {
49+
Path p = Path.of("dir").resolveSibling("sub");
50+
}
51+
52+
public void testPathResolveWithString() {
53+
Path p = Path.of("dir").resolve("sub");
54+
}
55+
56+
public void testNewFileWriterWithString() {
57+
FileWriter fw = new FileWriter("dir");
58+
}
59+
60+
public void testNewFileReaderWithString() {
61+
FileReader fr = new FileReader("dir");
62+
}
63+
64+
public void testNewFileOutputStreamWithString() {
65+
FileOutputStream fos = new FileOutputStream("dir");
66+
}
67+
68+
public void testNewFileInputStreamWithString() {
69+
FileInputStream fis = new FileInputStream("dir");
70+
}
71+
}

‎java/ql/test/library-tests/pathcreation/PathCreation.ql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import java
2+
import semmle.code.java.security.PathCreation
3+
4+
from PathCreation path
5+
select path

0 commit comments

Comments
 (0)