Skip to content

Commit f19eb78

Browse files
atorralbaatorralba
committed
Generalize file/path taint steps
This is needed by PathSanitizer but also helps simplify ZipSlip.ql
1 parent 4e29c39 commit f19eb78

File tree

4 files changed

+25
-56
lines changed

4 files changed

+25
-56
lines changed

‎java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -361,19 +361,7 @@ private class SummaryModelCsvBase extends SummaryModelCsv {
361361
"java.net;URI;false;toURL;;;Argument[-1];ReturnValue;taint;manual",
362362
"java.net;URI;false;toString;;;Argument[-1];ReturnValue;taint;manual",
363363
"java.net;URI;false;toAsciiString;;;Argument[-1];ReturnValue;taint;manual",
364-
"java.io;File;true;toURI;;;Argument[-1];ReturnValue;taint;manual",
365-
"java.io;File;true;toPath;;;Argument[-1];ReturnValue;taint;manual",
366-
"java.io;File;true;getAbsoluteFile;;;Argument[-1];ReturnValue;taint;manual",
367-
"java.io;File;true;getCanonicalFile;;;Argument[-1];ReturnValue;taint;manual",
368-
"java.io;File;true;getAbsolutePath;;;Argument[-1];ReturnValue;taint;manual",
369-
"java.io;File;true;getCanonicalPath;;;Argument[-1];ReturnValue;taint;manual",
370364
"java.nio;ByteBuffer;false;array;();;Argument[-1];ReturnValue;taint;manual",
371-
"java.nio.file;Path;true;normalize;;;Argument[-1];ReturnValue;taint;manual",
372-
"java.nio.file;Path;true;resolve;;;Argument[-1..0];ReturnValue;taint;manual",
373-
"java.nio.file;Path;false;toFile;;;Argument[-1];ReturnValue;taint;manual",
374-
"java.nio.file;Path;true;toString;;;Argument[-1];ReturnValue;taint;manual",
375-
"java.nio.file;Path;true;toUri;;;Argument[-1];ReturnValue;taint;manual",
376-
"java.nio.file;Paths;true;get;;;Argument[0..1];ReturnValue;taint;manual",
377365
"java.io;BufferedReader;true;readLine;;;Argument[-1];ReturnValue;taint;manual",
378366
"java.io;Reader;true;read;();;Argument[-1];ReturnValue;taint;manual",
379367
// arg to return
@@ -400,8 +388,6 @@ private class SummaryModelCsvBase extends SummaryModelCsv {
400388
// arg to arg
401389
"java.lang;System;false;arraycopy;;;Argument[0];Argument[2];taint;manual",
402390
// constructor flow
403-
"java.io;File;false;File;;;Argument[0];Argument[-1];taint;manual",
404-
"java.io;File;false;File;;;Argument[1];Argument[-1];taint;manual",
405391
"java.net;URI;false;URI;(String);;Argument[0];Argument[-1];taint;manual",
406392
"java.net;URL;false;URL;(String);;Argument[0];Argument[-1];taint;manual",
407393
"javax.xml.transform.stream;StreamSource;false;StreamSource;;;Argument[0];Argument[-1];taint;manual",

‎java/ql/lib/semmle/code/java/security/Files.qll

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,28 @@ private class WriteFileSinkModels extends SinkModelCsv {
7070
]
7171
}
7272
}
73+
74+
private class FileSummaryModels extends SummaryModelCsv {
75+
override predicate row(string row) {
76+
row =
77+
[
78+
"java.io;File;false;File;;;Argument[0];Argument[-1];taint;manual",
79+
"java.io;File;false;File;;;Argument[1];Argument[-1];taint;manual",
80+
"java.io;File;true;getAbsoluteFile;;;Argument[-1];ReturnValue;taint;manual",
81+
"java.io;File;true;getAbsolutePath;;;Argument[-1];ReturnValue;taint;manual",
82+
"java.io;File;true;getCanonicalFile;;;Argument[-1];ReturnValue;taint;manual",
83+
"java.io;File;true;getCanonicalPath;;;Argument[-1];ReturnValue;taint;manual",
84+
"java.io;File;true;toPath;;;Argument[-1];ReturnValue;taint;manual",
85+
"java.io;File;true;toURI;;;Argument[-1];ReturnValue;taint;manual",
86+
"java.nio.file;Path;true;normalize;;;Argument[-1];ReturnValue;taint;manual",
87+
"java.nio.file;Path;true;resolve;;;Argument[-1..0];ReturnValue;taint;manual",
88+
"java.nio.file;Path;true;toAbsolutePath;;;Argument[-1];ReturnValue;taint;manual",
89+
"java.nio.file;Path;false;toFile;;;Argument[-1];ReturnValue;taint;manual",
90+
"java.nio.file;Path;true;toString;;;Argument[-1];ReturnValue;taint;manual",
91+
"java.nio.file;Path;true;toUri;;;Argument[-1];ReturnValue;taint;manual",
92+
"java.nio.file;Paths;true;get;;;Argument[0..1];ReturnValue;taint;manual",
93+
"java.nio.file;FileSystem;true;getPath;;;Argument[0];ReturnValue;taint;manual",
94+
"java.nio.file;FileSystem;true;getRootDirectories;;;Argument[0];ReturnValue;taint;manual"
95+
]
96+
}
97+
}

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

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -36,41 +36,6 @@ class ArchiveEntryNameMethod extends Method {
3636
}
3737
}
3838

39-
/**
40-
* Holds if `n1` to `n2` is a dataflow step that converts between `String`,
41-
* `File`, and `Path`.
42-
*/
43-
predicate filePathStep(ExprNode n1, ExprNode n2) {
44-
exists(ConstructorCall cc | cc.getConstructedType() instanceof TypeFile |
45-
n1.asExpr() = cc.getAnArgument() and
46-
n2.asExpr() = cc
47-
)
48-
or
49-
exists(MethodAccess ma, Method m |
50-
ma.getMethod() = m and
51-
n1.asExpr() = ma.getQualifier() and
52-
n2.asExpr() = ma
53-
|
54-
m.getDeclaringType() instanceof TypeFile and m.hasName("toPath")
55-
or
56-
m.getDeclaringType() instanceof TypePath and m.hasName("toAbsolutePath")
57-
or
58-
m.getDeclaringType() instanceof TypePath and m.hasName("toFile")
59-
)
60-
}
61-
62-
predicate fileTaintStep(ExprNode n1, ExprNode n2) {
63-
exists(MethodAccess ma, Method m |
64-
n1.asExpr() = ma.getQualifier() or
65-
n1.asExpr() = ma.getAnArgument()
66-
|
67-
n2.asExpr() = ma and
68-
ma.getMethod() = m and
69-
m.getDeclaringType() instanceof TypePath and
70-
m.hasName("resolve")
71-
)
72-
}
73-
7439
class ZipSlipConfiguration extends TaintTracking::Configuration {
7540
ZipSlipConfiguration() { this = "ZipSlip" }
7641

@@ -80,10 +45,6 @@ class ZipSlipConfiguration extends TaintTracking::Configuration {
8045

8146
override predicate isSink(Node sink) { sink instanceof FileCreationSink }
8247

83-
override predicate isAdditionalTaintStep(Node n1, Node n2) {
84-
filePathStep(n1, n2) or fileTaintStep(n1, n2)
85-
}
86-
8748
override predicate isSanitizer(Node node) { node instanceof PathInjectionSanitizer }
8849
}
8950

‎java/ql/test/query-tests/security/CWE-022/semmle/tests/ZipSlip.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
edges
22
| ZipTest.java:7:19:7:33 | getName(...) : String | ZipTest.java:8:31:8:34 | name : String |
3-
| ZipTest.java:7:19:7:33 | getName(...) : String | ZipTest.java:9:48:9:51 | file |
4-
| ZipTest.java:7:19:7:33 | getName(...) : String | ZipTest.java:10:49:10:52 | file |
5-
| ZipTest.java:7:19:7:33 | getName(...) : String | ZipTest.java:11:36:11:39 | file |
63
| ZipTest.java:8:17:8:35 | new File(...) : File | ZipTest.java:9:48:9:51 | file |
74
| ZipTest.java:8:17:8:35 | new File(...) : File | ZipTest.java:10:49:10:52 | file |
85
| ZipTest.java:8:17:8:35 | new File(...) : File | ZipTest.java:11:36:11:39 | file |

0 commit comments

Comments
 (0)