Skip to content

Commit 20d7897

Browse files
author
Sebastian Bauersfeld
committed
Address review comments.
1 parent f95663c commit 20d7897

4 files changed

Lines changed: 37 additions & 64 deletions

File tree

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -401,11 +401,6 @@ private class SummaryModelCsvBase extends SummaryModelCsv {
401401
"java.io;File;false;File;;;Argument[0];Argument[-1];taint;manual",
402402
"java.io;File;false;File;;;Argument[1];Argument[-1];taint;manual",
403403
"java.net;URI;false;URI;(String);;Argument[0];Argument[-1];taint;manual",
404-
"java.net;URI;false;URI;(String,String,String);;Argument[0..2];Argument[-1];taint;manual",
405-
"java.net;URI;false;URI;(String,String,String,int,String,String,String);;Argument[0..2];Argument[-1];taint;manual",
406-
"java.net;URI;false;URI;(String,String,String,int,String,String,String);;Argument[4..6];Argument[-1];taint;manual",
407-
"java.net;URI;false;URI;(String,String,String,String);;Argument[0..3];Argument[-1];taint;manual",
408-
"java.net;URI;false;URI;(String,String,String,String,String);;Argument[0..4];Argument[-1];taint;manual",
409404
"java.net;URL;false;URL;(String);;Argument[0];Argument[-1];taint;manual",
410405
"javax.xml.transform.stream;StreamSource;false;StreamSource;;;Argument[0];Argument[-1];taint;manual",
411406
"javax.xml.transform.sax;SAXSource;false;SAXSource;(InputSource);;Argument[0];Argument[-1];taint;manual",

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,19 @@
55
import java
66
import semmle.code.java.controlflow.Guards
77
import semmle.code.java.security.PathCreation
8+
import semmle.code.java.dataflow.ExternalFlow
9+
10+
class TaintedPathInjectionSummaries extends SummaryModelCsv {
11+
override predicate row(string row) {
12+
row =
13+
[
14+
"java.net;URI;false;URI;(String,String,String);;Argument[1];Argument[-1];taint;manual",
15+
"java.net;URI;false;URI;(String,String,String,String);;Argument[1..2];Argument[-1];taint;manual",
16+
"java.net;URI;false;URI;(String,String,String,String,String);;Argument[2];Argument[-1];taint;manual",
17+
"java.net;URI;false;URI;(String,String,String,int,String,String,String);;Argument[4];Argument[-1];taint;manual",
18+
]
19+
}
20+
}
821

922
private predicate inWeakCheck(Expr e) {
1023
// None of these are sufficient to guarantee that a string is safe.

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

Lines changed: 18 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -9,42 +9,16 @@ edges
99
| Test.java:80:31:80:32 | br : BufferedReader | Test.java:80:31:80:43 | readLine(...) : String |
1010
| Test.java:80:31:80:43 | readLine(...) : String | Test.java:82:67:82:81 | ... + ... |
1111
| Test.java:88:17:88:37 | getHostName(...) : String | Test.java:90:26:90:29 | temp |
12-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:96:20:96:20 | t : String |
13-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:96:23:96:23 | t : String |
14-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:96:26:96:26 | t : String |
15-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:97:20:97:20 | t : String |
16-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:97:23:97:23 | t : String |
1712
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:97:26:97:26 | t : String |
18-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:97:29:97:29 | t : String |
19-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:98:20:98:20 | t : String |
2013
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:98:23:98:23 | t : String |
21-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:98:26:98:26 | t : String |
22-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:98:29:98:29 | t : String |
23-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:98:32:98:32 | t : String |
24-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:99:20:99:20 | t : String |
25-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:99:23:99:23 | t : String |
26-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:99:26:99:26 | t : String |
27-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:99:32:99:32 | t : String |
28-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:99:35:99:35 | t : String |
29-
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:99:38:99:38 | t : String |
30-
| Test.java:96:20:96:20 | t : String | Test.java:96:12:96:27 | new URI(...) |
31-
| Test.java:96:23:96:23 | t : String | Test.java:96:12:96:27 | new URI(...) |
32-
| Test.java:96:26:96:26 | t : String | Test.java:96:12:96:27 | new URI(...) |
33-
| Test.java:97:20:97:20 | t : String | Test.java:97:12:97:30 | new URI(...) |
34-
| Test.java:97:23:97:23 | t : String | Test.java:97:12:97:30 | new URI(...) |
35-
| Test.java:97:26:97:26 | t : String | Test.java:97:12:97:30 | new URI(...) |
36-
| Test.java:97:29:97:29 | t : String | Test.java:97:12:97:30 | new URI(...) |
37-
| Test.java:98:20:98:20 | t : String | Test.java:98:12:98:33 | new URI(...) |
14+
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:99:29:99:29 | t : String |
15+
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:100:32:100:32 | t : String |
16+
| Test.java:95:14:95:34 | getHostName(...) : String | Test.java:101:41:101:41 | t : String |
17+
| Test.java:97:26:97:26 | t : String | Test.java:97:12:97:33 | new URI(...) |
3818
| Test.java:98:23:98:23 | t : String | Test.java:98:12:98:33 | new URI(...) |
39-
| Test.java:98:26:98:26 | t : String | Test.java:98:12:98:33 | new URI(...) |
40-
| Test.java:98:29:98:29 | t : String | Test.java:98:12:98:33 | new URI(...) |
41-
| Test.java:98:32:98:32 | t : String | Test.java:98:12:98:33 | new URI(...) |
42-
| Test.java:99:20:99:20 | t : String | Test.java:99:12:99:39 | new URI(...) |
43-
| Test.java:99:23:99:23 | t : String | Test.java:99:12:99:39 | new URI(...) |
44-
| Test.java:99:26:99:26 | t : String | Test.java:99:12:99:39 | new URI(...) |
45-
| Test.java:99:32:99:32 | t : String | Test.java:99:12:99:39 | new URI(...) |
46-
| Test.java:99:35:99:35 | t : String | Test.java:99:12:99:39 | new URI(...) |
47-
| Test.java:99:38:99:38 | t : String | Test.java:99:12:99:39 | new URI(...) |
19+
| Test.java:99:29:99:29 | t : String | Test.java:99:12:99:33 | new URI(...) |
20+
| Test.java:100:32:100:32 | t : String | Test.java:100:12:100:45 | new URI(...) |
21+
| Test.java:101:41:101:41 | t : String | Test.java:101:12:101:54 | new URI(...) |
4822
nodes
4923
| Test.java:19:18:19:38 | getHostName(...) : String | semmle.label | getHostName(...) : String |
5024
| Test.java:24:20:24:23 | temp | semmle.label | temp |
@@ -60,28 +34,16 @@ nodes
6034
| Test.java:88:17:88:37 | getHostName(...) : String | semmle.label | getHostName(...) : String |
6135
| Test.java:90:26:90:29 | temp | semmle.label | temp |
6236
| Test.java:95:14:95:34 | getHostName(...) : String | semmle.label | getHostName(...) : String |
63-
| Test.java:96:12:96:27 | new URI(...) | semmle.label | new URI(...) |
64-
| Test.java:96:20:96:20 | t : String | semmle.label | t : String |
65-
| Test.java:96:23:96:23 | t : String | semmle.label | t : String |
66-
| Test.java:96:26:96:26 | t : String | semmle.label | t : String |
67-
| Test.java:97:12:97:30 | new URI(...) | semmle.label | new URI(...) |
68-
| Test.java:97:20:97:20 | t : String | semmle.label | t : String |
69-
| Test.java:97:23:97:23 | t : String | semmle.label | t : String |
37+
| Test.java:97:12:97:33 | new URI(...) | semmle.label | new URI(...) |
7038
| Test.java:97:26:97:26 | t : String | semmle.label | t : String |
71-
| Test.java:97:29:97:29 | t : String | semmle.label | t : String |
7239
| Test.java:98:12:98:33 | new URI(...) | semmle.label | new URI(...) |
73-
| Test.java:98:20:98:20 | t : String | semmle.label | t : String |
7440
| Test.java:98:23:98:23 | t : String | semmle.label | t : String |
75-
| Test.java:98:26:98:26 | t : String | semmle.label | t : String |
76-
| Test.java:98:29:98:29 | t : String | semmle.label | t : String |
77-
| Test.java:98:32:98:32 | t : String | semmle.label | t : String |
78-
| Test.java:99:12:99:39 | new URI(...) | semmle.label | new URI(...) |
79-
| Test.java:99:20:99:20 | t : String | semmle.label | t : String |
80-
| Test.java:99:23:99:23 | t : String | semmle.label | t : String |
81-
| Test.java:99:26:99:26 | t : String | semmle.label | t : String |
82-
| Test.java:99:32:99:32 | t : String | semmle.label | t : String |
83-
| Test.java:99:35:99:35 | t : String | semmle.label | t : String |
84-
| Test.java:99:38:99:38 | t : String | semmle.label | t : String |
41+
| Test.java:99:12:99:33 | new URI(...) | semmle.label | new URI(...) |
42+
| Test.java:99:29:99:29 | t : String | semmle.label | t : String |
43+
| Test.java:100:12:100:45 | new URI(...) | semmle.label | new URI(...) |
44+
| Test.java:100:32:100:32 | t : String | semmle.label | t : String |
45+
| Test.java:101:12:101:54 | new URI(...) | semmle.label | new URI(...) |
46+
| Test.java:101:41:101:41 | t : String | semmle.label | t : String |
8547
subpaths
8648
#select
8749
| Test.java:24:11:24:24 | new File(...) | Test.java:19:18:19:38 | getHostName(...) : String | Test.java:24:20:24:23 | temp | $@ flows to here and is used in a path. | Test.java:19:18:19:38 | getHostName(...) | User-provided value |
@@ -90,7 +52,8 @@ subpaths
9052
| Test.java:34:12:34:25 | new File(...) | Test.java:19:18:19:38 | getHostName(...) : String | Test.java:34:21:34:24 | temp | $@ flows to here and is used in a path. | Test.java:19:18:19:38 | getHostName(...) | User-provided value |
9153
| Test.java:82:52:82:88 | new FileWriter(...) | Test.java:79:74:79:97 | getInputStream(...) : ServletInputStream | Test.java:82:67:82:81 | ... + ... | $@ flows to here and is used in a path. | Test.java:79:74:79:97 | getInputStream(...) | User-provided value |
9254
| Test.java:90:26:90:29 | temp | Test.java:88:17:88:37 | getHostName(...) : String | Test.java:90:26:90:29 | temp | $@ flows to here and is used in a path. | Test.java:88:17:88:37 | getHostName(...) | User-provided value |
93-
| Test.java:96:3:96:28 | new File(...) | Test.java:95:14:95:34 | getHostName(...) : String | Test.java:96:12:96:27 | new URI(...) | $@ flows to here and is used in a path. | Test.java:95:14:95:34 | getHostName(...) | User-provided value |
94-
| Test.java:97:3:97:31 | new File(...) | Test.java:95:14:95:34 | getHostName(...) : String | Test.java:97:12:97:30 | new URI(...) | $@ flows to here and is used in a path. | Test.java:95:14:95:34 | getHostName(...) | User-provided value |
55+
| Test.java:97:3:97:34 | new File(...) | Test.java:95:14:95:34 | getHostName(...) : String | Test.java:97:12:97:33 | new URI(...) | $@ flows to here and is used in a path. | Test.java:95:14:95:34 | getHostName(...) | User-provided value |
9556
| Test.java:98:3:98:34 | new File(...) | Test.java:95:14:95:34 | getHostName(...) : String | Test.java:98:12:98:33 | new URI(...) | $@ flows to here and is used in a path. | Test.java:95:14:95:34 | getHostName(...) | User-provided value |
96-
| Test.java:99:3:99:40 | new File(...) | Test.java:95:14:95:34 | getHostName(...) : String | Test.java:99:12:99:39 | new URI(...) | $@ flows to here and is used in a path. | Test.java:95:14:95:34 | getHostName(...) | User-provided value |
57+
| Test.java:99:3:99:34 | new File(...) | Test.java:95:14:95:34 | getHostName(...) : String | Test.java:99:12:99:33 | new URI(...) | $@ flows to here and is used in a path. | Test.java:95:14:95:34 | getHostName(...) | User-provided value |
58+
| Test.java:100:3:100:46 | new File(...) | Test.java:95:14:95:34 | getHostName(...) : String | Test.java:100:12:100:45 | new URI(...) | $@ flows to here and is used in a path. | Test.java:95:14:95:34 | getHostName(...) | User-provided value |
59+
| Test.java:101:3:101:55 | new File(...) | Test.java:95:14:95:34 | getHostName(...) : String | Test.java:101:12:101:54 | new URI(...) | $@ flows to here and is used in a path. | Test.java:95:14:95:34 | getHostName(...) | User-provided value |

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,11 @@ void doGet4(InetAddress address)
9393
void doGet5(InetAddress address)
9494
throws URISyntaxException {
9595
String t = address.getHostName();
96-
new File(new URI(t, t, t));
97-
new File(new URI(t, t, t, t));
98-
new File(new URI(t, t, t, t, t));
99-
new File(new URI(t, t, t, 0, t, t, t));
96+
// BAD: construct a file path with user input
97+
new File(new URI(null, t, null));
98+
new File(new URI(t, t, null, t));
99+
new File(new URI(t, null, t, t));
100+
new File(new URI(null, null, t, null, null));
101+
new File(new URI(null, null, null, 0, t, null, null));
100102
}
101103
}

0 commit comments

Comments
 (0)