Skip to content

Commit 5216bba

Browse files
authored
Merge pull request #6835 from atorralba/atorralba/fix-local-and-remote-flow-tests
Java: Use InlineExpectationsTest for local and remote flow tests
2 parents 47ae76f + e3b46f2 commit 5216bba

File tree

9 files changed

+149
-150
lines changed

9 files changed

+149
-150
lines changed

java/ql/test/library-tests/dataflow/taintsources/A.java

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,36 +14,39 @@
1414
import javax.servlet.http.HttpServletResponse;
1515

1616
public class A {
17+
18+
private static void sink(Object o) {}
19+
1720
public static void main(String[] args) {
18-
String[] a = args; // user input
19-
String s = args[0]; // user input
21+
sink(args); // $hasLocalValueFlow
22+
sink(args[0]); // $hasLocalTaintFlow
2023
}
2124

2225
public static void userInput() throws SQLException, IOException, MalformedURLException {
23-
System.getenv("test"); // user input
26+
sink(System.getenv("test")); // $hasLocalValueFlow
2427
class TestServlet extends HttpServlet {
2528
@Override
2629
protected void doGet(HttpServletRequest req, HttpServletResponse resp)
27-
throws ServletException, IOException {
28-
req.getParameter("test"); // remote user input
29-
req.getHeader("test"); // remote user input
30-
req.getQueryString(); // remote user input
31-
req.getCookies()[0].getValue(); // remote user input
30+
throws ServletException, IOException {
31+
sink(req.getParameter("test")); // $hasRemoteValueFlow
32+
sink(req.getHeader("test")); // $hasRemoteValueFlow
33+
sink(req.getQueryString()); // $hasRemoteValueFlow
34+
sink(req.getCookies()[0].getValue()); // $hasRemoteValueFlow
3235
}
3336
}
34-
new Properties().getProperty("test"); // user input
35-
System.getProperty("test"); // user input
37+
sink(new Properties().getProperty("test")); // $hasLocalValueFlow
38+
sink(System.getProperty("test")); // $hasLocalValueFlow
3639
new Object() {
3740
public void test(ResultSet rs) throws SQLException {
38-
rs.getString(0); // user input
41+
sink(rs.getString(0)); // $hasLocalValueFlow
3942
}
4043
};
41-
new URL("test").openConnection().getInputStream(); // remote user input
42-
new Socket("test", 1234).getInputStream(); // remote user input
43-
InetAddress.getByName("test").getHostName(); // remote user input
44+
sink(new URL("test").openConnection().getInputStream()); // $hasRemoteValueFlow
45+
sink(new Socket("test", 1234).getInputStream()); // $hasRemoteValueFlow
46+
sink(InetAddress.getByName("test").getHostName()); // $hasRemoteValueFlow
4447

45-
System.in.read(); // user input
46-
new FileInputStream("test").read(); // user input
48+
sink(System.in); // $hasLocalValueFlow
49+
sink(new FileInputStream("test")); // $hasLocalValueFlow
4750
}
4851

4952
}

java/ql/test/library-tests/dataflow/taintsources/IntentSources.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,34 +4,39 @@
44

55
public class IntentSources extends Activity {
66

7+
private static void sink(Object o) {}
8+
79
public void test() throws java.io.IOException {
810

911
String trouble = this.getIntent().getStringExtra("key");
10-
Runtime.getRuntime().exec(trouble);
12+
sink(trouble); // $hasRemoteTaintFlow
1113

1214
}
1315

1416
public void test2() throws java.io.IOException {
1517

1618
String trouble = getIntent().getStringExtra("key");
17-
Runtime.getRuntime().exec(trouble);
19+
sink(trouble); // $hasRemoteTaintFlow
1820

1921
}
2022

2123
public void test3() throws java.io.IOException {
2224

2325
String trouble = getIntent().getExtras().getString("key");
24-
Runtime.getRuntime().exec(trouble);
26+
sink(trouble); // $hasRemoteTaintFlow
2527

2628
}
2729

2830
}
2931

32+
3033
class OtherClass {
3134

35+
private static void sink(Object o) {}
36+
3237
public void test(IntentSources is) throws java.io.IOException {
3338
String trouble = is.getIntent().getStringExtra("key");
34-
Runtime.getRuntime().exec(trouble);
39+
sink(trouble); // $hasRemoteTaintFlow
3540
}
3641

3742
}
Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
package security.library.dataflow;
22

33
public class RmiFlowImpl implements RmiFlow {
4+
5+
private static void sink(Object o) {}
6+
47
public String listDirectory(String path) throws java.io.IOException {
58
String command = "ls " + path;
6-
Runtime.getRuntime().exec(command);
9+
sink(command); // $hasRemoteTaintFlow
710
return "pretend there are some results here";
811
}
912

1013
public String notRemotable(String path) throws java.io.IOException {
1114
String command = "ls " + path;
12-
Runtime.getRuntime().exec(command);
15+
sink(command); // Safe
1316
return "pretend there are some results here";
1417
}
1518
}

java/ql/test/library-tests/dataflow/taintsources/SpringMultiPart.java

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,24 @@
44
public class SpringMultiPart {
55
MultipartFile file;
66

7+
private static void sink(Object o) {}
8+
79
public void test() throws Exception {
8-
file.getBytes();
9-
file.isEmpty();
10-
file.getInputStream();
11-
file.getResource();
12-
file.getName();
13-
file.getContentType();
14-
file.getOriginalFilename();
10+
sink(file.getBytes()); // $hasRemoteValueFlow
11+
sink(file.isEmpty()); // Safe
12+
sink(file.getInputStream()); // $hasRemoteValueFlow
13+
sink(file.getResource()); // $hasRemoteValueFlow
14+
sink(file.getName()); // $hasRemoteValueFlow
15+
sink(file.getContentType()); // $hasRemoteValueFlow
16+
sink(file.getOriginalFilename()); // $hasRemoteValueFlow
1517
}
16-
18+
1719
public void test(MultipartRequest request) {
18-
request.getFile("name");
19-
request.getFileMap();
20-
request.getFileNames();
21-
request.getFiles("name");
22-
request.getMultiFileMap();
23-
request.getMultipartContentType("name");
20+
sink(request.getFile("name"));// $hasRemoteValueFlow
21+
sink(request.getFileMap());// $hasRemoteValueFlow
22+
sink(request.getFileNames());// $hasRemoteValueFlow
23+
sink(request.getFiles("name"));// $hasRemoteValueFlow
24+
sink(request.getMultiFileMap());// $hasRemoteValueFlow
25+
sink(request.getMultipartContentType("name")); // $hasRemoteValueFlow
2426
}
2527
}

java/ql/test/library-tests/dataflow/taintsources/SpringSavedRequest.java

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,25 @@
44
public class SpringSavedRequest {
55
SavedRequest sr;
66

7+
private static void sink(Object o) {}
8+
79
public void test() {
8-
sr.getRedirectUrl();
9-
sr.getCookies();
10-
sr.getHeaderValues("name");
11-
sr.getHeaderNames();
12-
sr.getParameterValues("name");
13-
sr.getParameterMap();
10+
sink(sr.getRedirectUrl()); // $hasRemoteValueFlow
11+
sink(sr.getCookies()); // $hasRemoteValueFlow
12+
sink(sr.getHeaderValues("name")); // $hasRemoteValueFlow
13+
sink(sr.getHeaderNames()); // $hasRemoteValueFlow
14+
sink(sr.getParameterValues("name")); // $hasRemoteValueFlow
15+
sink(sr.getParameterMap()); // $hasRemoteValueFlow
1416
}
1517

1618
SimpleSavedRequest ssr;
1719

1820
public void test2() {
19-
ssr.getRedirectUrl();
20-
ssr.getCookies();
21-
ssr.getHeaderValues("name");
22-
ssr.getHeaderNames();
23-
ssr.getParameterValues("name");
24-
ssr.getParameterMap();
21+
sink(ssr.getRedirectUrl()); // $hasRemoteValueFlow
22+
sink(ssr.getCookies()); // $hasRemoteValueFlow
23+
sink(ssr.getHeaderValues("name")); // $hasRemoteValueFlow
24+
sink(ssr.getHeaderNames()); // $hasRemoteValueFlow
25+
sink(ssr.getParameterValues("name")); // $hasRemoteValueFlow
26+
sink(ssr.getParameterMap()); // $hasRemoteValueFlow
2527
}
2628
}
Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +0,0 @@
1-
| A.java:17:27:17:39 | args | A.java:17:27:17:39 | args |
2-
| A.java:17:27:17:39 | args | A.java:18:18:18:21 | args |
3-
| A.java:17:27:17:39 | args | A.java:19:16:19:19 | args |
4-
| A.java:17:27:17:39 | args | A.java:19:16:19:22 | ...[...] |
5-
| A.java:23:5:23:25 | getenv(...) | A.java:23:5:23:25 | getenv(...) |
6-
| A.java:34:5:34:40 | getProperty(...) | A.java:34:5:34:40 | getProperty(...) |
7-
| A.java:35:5:35:30 | getProperty(...) | A.java:35:5:35:30 | getProperty(...) |
8-
| A.java:38:9:38:23 | getString(...) | A.java:38:9:38:23 | getString(...) |
9-
| A.java:45:5:45:13 | System.in | A.java:45:5:45:13 | System.in |
10-
| A.java:46:5:46:31 | new FileInputStream(...) | A.java:46:5:46:31 | new FileInputStream(...) |
Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,54 @@
11
import java
2-
import semmle.code.java.dataflow.TaintTracking
32
import semmle.code.java.dataflow.FlowSources
3+
import TestUtilities.InlineExpectationsTest
44

5-
class Conf extends TaintTracking::Configuration {
6-
Conf() { this = "remote taint conf" }
7-
8-
override predicate isSource(DataFlow::Node n) {
9-
n instanceof UserInput and
10-
not n instanceof RemoteFlowSource
5+
class LocalSource extends DataFlow::Node {
6+
LocalSource() {
7+
this instanceof UserInput and
8+
not this instanceof RemoteFlowSource
119
}
10+
}
11+
12+
predicate isTestSink(DataFlow::Node n) {
13+
exists(MethodAccess ma | ma.getMethod().hasName("sink") | n.asExpr() = ma.getAnArgument())
14+
}
15+
16+
class LocalValueConf extends DataFlow::Configuration {
17+
LocalValueConf() { this = "LocalValueConf" }
18+
19+
override predicate isSource(DataFlow::Node n) { n instanceof LocalSource }
20+
21+
override predicate isSink(DataFlow::Node n) { isTestSink(n) }
22+
}
23+
24+
class LocalTaintConf extends TaintTracking::Configuration {
25+
LocalTaintConf() { this = "LocalTaintConf" }
1226

13-
override predicate isSink(DataFlow::Node n) { any() }
27+
override predicate isSource(DataFlow::Node n) { n instanceof LocalSource }
28+
29+
override predicate isSink(DataFlow::Node n) { isTestSink(n) }
1430
}
1531

16-
from DataFlow::Node src, DataFlow::Node sink, Conf conf
17-
where conf.hasFlow(src, sink)
18-
select src, sink
32+
class LocalFlowTest extends InlineExpectationsTest {
33+
LocalFlowTest() { this = "LocalFlowTest" }
34+
35+
override string getARelevantTag() { result = ["hasLocalValueFlow", "hasLocalTaintFlow"] }
36+
37+
override predicate hasActualResult(Location location, string element, string tag, string value) {
38+
tag = "hasLocalValueFlow" and
39+
exists(DataFlow::Node src, DataFlow::Node sink | any(LocalValueConf c).hasFlow(src, sink) |
40+
sink.getLocation() = location and
41+
element = sink.toString() and
42+
value = ""
43+
)
44+
or
45+
tag = "hasLocalTaintFlow" and
46+
exists(DataFlow::Node src, DataFlow::Node sink |
47+
any(LocalTaintConf c).hasFlow(src, sink) and not any(LocalValueConf c).hasFlow(src, sink)
48+
|
49+
sink.getLocation() = location and
50+
element = sink.toString() and
51+
value = ""
52+
)
53+
}
54+
}
Lines changed: 0 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,74 +0,0 @@
1-
| A.java:28:9:28:32 | getParameter(...) | A.java:28:9:28:32 | getParameter(...) |
2-
| A.java:29:9:29:29 | getHeader(...) | A.java:29:9:29:29 | getHeader(...) |
3-
| A.java:30:9:30:28 | getQueryString(...) | A.java:30:9:30:28 | getQueryString(...) |
4-
| A.java:31:9:31:38 | getValue(...) | A.java:31:9:31:38 | getValue(...) |
5-
| A.java:41:5:41:53 | getInputStream(...) | A.java:41:5:41:53 | getInputStream(...) |
6-
| A.java:42:5:42:45 | getInputStream(...) | A.java:42:5:42:45 | getInputStream(...) |
7-
| A.java:43:5:43:47 | getHostName(...) | A.java:43:5:43:47 | getHostName(...) |
8-
| IntentSources.java:9:20:9:35 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:105:19:105:32 | [summary] read: <map.value> of android.content.Intent.extras of argument -1 in getStringExtra |
9-
| IntentSources.java:9:20:9:35 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:105:19:105:32 | [summary] read: android.content.Intent.extras of argument -1 in getStringExtra |
10-
| IntentSources.java:9:20:9:35 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:105:19:105:32 | [summary] to write: return (return) in getStringExtra |
11-
| IntentSources.java:9:20:9:35 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:105:19:105:32 | parameter this |
12-
| IntentSources.java:9:20:9:35 | getIntent(...) | IntentSources.java:9:20:9:35 | getIntent(...) |
13-
| IntentSources.java:9:20:9:35 | getIntent(...) | IntentSources.java:9:20:9:57 | getStringExtra(...) |
14-
| IntentSources.java:9:20:9:35 | getIntent(...) | IntentSources.java:10:29:10:35 | trouble |
15-
| IntentSources.java:16:20:16:30 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:105:19:105:32 | [summary] read: <map.value> of android.content.Intent.extras of argument -1 in getStringExtra |
16-
| IntentSources.java:16:20:16:30 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:105:19:105:32 | [summary] read: android.content.Intent.extras of argument -1 in getStringExtra |
17-
| IntentSources.java:16:20:16:30 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:105:19:105:32 | [summary] to write: return (return) in getStringExtra |
18-
| IntentSources.java:16:20:16:30 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:105:19:105:32 | parameter this |
19-
| IntentSources.java:16:20:16:30 | getIntent(...) | IntentSources.java:16:20:16:30 | getIntent(...) |
20-
| IntentSources.java:16:20:16:30 | getIntent(...) | IntentSources.java:16:20:16:52 | getStringExtra(...) |
21-
| IntentSources.java:16:20:16:30 | getIntent(...) | IntentSources.java:17:29:17:35 | trouble |
22-
| IntentSources.java:23:20:23:30 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:33:19:33:27 | [summary] read: android.content.Intent.extras of argument -1 in getExtras |
23-
| IntentSources.java:23:20:23:30 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:33:19:33:27 | [summary] to write: return (return) in getExtras |
24-
| IntentSources.java:23:20:23:30 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:33:19:33:27 | parameter this |
25-
| IntentSources.java:23:20:23:30 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/os/BaseBundle.java:12:19:12:27 | [summary] read: <map.value> of argument -1 in getString |
26-
| IntentSources.java:23:20:23:30 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/os/BaseBundle.java:12:19:12:27 | [summary] to write: return (return) in getString |
27-
| IntentSources.java:23:20:23:30 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/os/BaseBundle.java:12:19:12:27 | parameter this |
28-
| IntentSources.java:23:20:23:30 | getIntent(...) | IntentSources.java:23:20:23:30 | getIntent(...) |
29-
| IntentSources.java:23:20:23:30 | getIntent(...) | IntentSources.java:23:20:23:42 | getExtras(...) |
30-
| IntentSources.java:23:20:23:30 | getIntent(...) | IntentSources.java:23:20:23:59 | getString(...) |
31-
| IntentSources.java:23:20:23:30 | getIntent(...) | IntentSources.java:24:29:24:35 | trouble |
32-
| IntentSources.java:33:20:33:33 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:105:19:105:32 | [summary] read: <map.value> of android.content.Intent.extras of argument -1 in getStringExtra |
33-
| IntentSources.java:33:20:33:33 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:105:19:105:32 | [summary] read: android.content.Intent.extras of argument -1 in getStringExtra |
34-
| IntentSources.java:33:20:33:33 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:105:19:105:32 | [summary] to write: return (return) in getStringExtra |
35-
| IntentSources.java:33:20:33:33 | getIntent(...) | ../../../stubs/google-android-9.0.0/android/content/Intent.java:105:19:105:32 | parameter this |
36-
| IntentSources.java:33:20:33:33 | getIntent(...) | IntentSources.java:33:20:33:33 | getIntent(...) |
37-
| IntentSources.java:33:20:33:33 | getIntent(...) | IntentSources.java:33:20:33:55 | getStringExtra(...) |
38-
| IntentSources.java:33:20:33:33 | getIntent(...) | IntentSources.java:34:29:34:35 | trouble |
39-
| PlayResource.java:19:37:19:46 | uri | PlayResource.java:19:37:19:46 | uri |
40-
| PlayResource.java:20:18:20:48 | getQueryString(...) | ../../../stubs/playframework-2.6.x/play/mvc/Results.java:634:33:634:42 | url |
41-
| PlayResource.java:20:18:20:48 | getQueryString(...) | PlayResource.java:20:18:20:48 | getQueryString(...) |
42-
| PlayResource.java:20:18:20:48 | getQueryString(...) | PlayResource.java:21:21:21:23 | url |
43-
| PlayResource.java:24:42:24:53 | token | ../../../stubs/playframework-2.6.x/play/mvc/Results.java:94:27:94:40 | content |
44-
| PlayResource.java:24:42:24:53 | token | PlayResource.java:24:42:24:53 | token |
45-
| PlayResource.java:24:42:24:53 | token | PlayResource.java:25:30:25:34 | token |
46-
| PlayResource.java:28:56:28:65 | uri | PlayResource.java:28:56:28:65 | uri |
47-
| RmiFlowImpl.java:4:30:4:40 | path | RmiFlowImpl.java:4:30:4:40 | path |
48-
| RmiFlowImpl.java:4:30:4:40 | path | RmiFlowImpl.java:5:20:5:31 | ... + ... |
49-
| RmiFlowImpl.java:4:30:4:40 | path | RmiFlowImpl.java:5:28:5:31 | path |
50-
| RmiFlowImpl.java:4:30:4:40 | path | RmiFlowImpl.java:6:29:6:35 | command |
51-
| SpringMultiPart.java:8:3:8:17 | getBytes(...) | SpringMultiPart.java:8:3:8:17 | getBytes(...) |
52-
| SpringMultiPart.java:10:3:10:23 | getInputStream(...) | SpringMultiPart.java:10:3:10:23 | getInputStream(...) |
53-
| SpringMultiPart.java:11:3:11:20 | getResource(...) | SpringMultiPart.java:11:3:11:20 | getResource(...) |
54-
| SpringMultiPart.java:12:3:12:16 | getName(...) | SpringMultiPart.java:12:3:12:16 | getName(...) |
55-
| SpringMultiPart.java:13:3:13:23 | getContentType(...) | SpringMultiPart.java:13:3:13:23 | getContentType(...) |
56-
| SpringMultiPart.java:14:3:14:28 | getOriginalFilename(...) | SpringMultiPart.java:14:3:14:28 | getOriginalFilename(...) |
57-
| SpringMultiPart.java:18:3:18:25 | getFile(...) | SpringMultiPart.java:18:3:18:25 | getFile(...) |
58-
| SpringMultiPart.java:19:3:19:22 | getFileMap(...) | SpringMultiPart.java:19:3:19:22 | getFileMap(...) |
59-
| SpringMultiPart.java:20:3:20:24 | getFileNames(...) | SpringMultiPart.java:20:3:20:24 | getFileNames(...) |
60-
| SpringMultiPart.java:21:3:21:26 | getFiles(...) | SpringMultiPart.java:21:3:21:26 | getFiles(...) |
61-
| SpringMultiPart.java:22:3:22:27 | getMultiFileMap(...) | SpringMultiPart.java:22:3:22:27 | getMultiFileMap(...) |
62-
| SpringMultiPart.java:23:3:23:41 | getMultipartContentType(...) | SpringMultiPart.java:23:3:23:41 | getMultipartContentType(...) |
63-
| SpringSavedRequest.java:8:3:8:21 | getRedirectUrl(...) | SpringSavedRequest.java:8:3:8:21 | getRedirectUrl(...) |
64-
| SpringSavedRequest.java:9:3:9:17 | getCookies(...) | SpringSavedRequest.java:9:3:9:17 | getCookies(...) |
65-
| SpringSavedRequest.java:10:3:10:28 | getHeaderValues(...) | SpringSavedRequest.java:10:3:10:28 | getHeaderValues(...) |
66-
| SpringSavedRequest.java:11:3:11:21 | getHeaderNames(...) | SpringSavedRequest.java:11:3:11:21 | getHeaderNames(...) |
67-
| SpringSavedRequest.java:12:3:12:31 | getParameterValues(...) | SpringSavedRequest.java:12:3:12:31 | getParameterValues(...) |
68-
| SpringSavedRequest.java:13:3:13:22 | getParameterMap(...) | SpringSavedRequest.java:13:3:13:22 | getParameterMap(...) |
69-
| SpringSavedRequest.java:19:3:19:22 | getRedirectUrl(...) | SpringSavedRequest.java:19:3:19:22 | getRedirectUrl(...) |
70-
| SpringSavedRequest.java:20:3:20:18 | getCookies(...) | SpringSavedRequest.java:20:3:20:18 | getCookies(...) |
71-
| SpringSavedRequest.java:21:3:21:29 | getHeaderValues(...) | SpringSavedRequest.java:21:3:21:29 | getHeaderValues(...) |
72-
| SpringSavedRequest.java:22:3:22:22 | getHeaderNames(...) | SpringSavedRequest.java:22:3:22:22 | getHeaderNames(...) |
73-
| SpringSavedRequest.java:23:3:23:32 | getParameterValues(...) | SpringSavedRequest.java:23:3:23:32 | getParameterValues(...) |
74-
| SpringSavedRequest.java:24:3:24:23 | getParameterMap(...) | SpringSavedRequest.java:24:3:24:23 | getParameterMap(...) |

0 commit comments

Comments
 (0)