Skip to content

Commit be6fd7c

Browse files
aschackmullaschackmull
authored
Merge pull request github#6382 from bmuskalla/stringValueOfTaint
Track taint for String.valueOf(..)
2 parents c0d76da + 8ce8414 commit be6fd7c

File tree

3 files changed

+48
-31
lines changed

3 files changed

+48
-31
lines changed

‎java/ql/src/semmle/code/java/dataflow/FlowSteps.qll

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,17 +108,22 @@ abstract class TaintPreservingCallable extends Callable {
108108
private class StringTaintPreservingMethod extends TaintPreservingCallable {
109109
StringTaintPreservingMethod() {
110110
this.getDeclaringType() instanceof TypeString and
111-
this.hasName([
112-
"concat", "copyValueOf", "endsWith", "format", "formatted", "getBytes", "indent", "intern",
113-
"join", "repeat", "split", "strip", "stripIndent", "stripLeading", "stripTrailing",
114-
"substring", "toCharArray", "toLowerCase", "toString", "toUpperCase", "trim"
115-
])
111+
(
112+
this.hasName([
113+
"concat", "copyValueOf", "endsWith", "format", "formatted", "getBytes", "indent",
114+
"intern", "join", "repeat", "split", "strip", "stripIndent", "stripLeading",
115+
"stripTrailing", "substring", "toCharArray", "toLowerCase", "toString", "toUpperCase",
116+
"trim"
117+
])
118+
or
119+
this.hasName("valueOf") and this.getParameterType(0) instanceof Array
120+
)
116121
}
117122

118123
override predicate returnsTaintFrom(int arg) {
119124
arg = -1 and not this.isStatic()
120125
or
121-
this.hasName(["concat", "copyValueOf"]) and arg = 0
126+
this.hasName(["concat", "copyValueOf", "valueOf"]) and arg = 0
122127
or
123128
this.hasName(["format", "formatted", "join"]) and arg = [0 .. getNumberOfParameters()]
124129
}

‎java/ql/test/library-tests/dataflow/taint/B.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ public static void maintest() throws java.io.UnsupportedEncodingException, java.
3434
// tainted - data preserving constructors
3535
String constructed = new String(complex);
3636
sink(constructed);
37+
// tainted - data preserving method
38+
String valueOf = String.valueOf(complex.toCharArray());
39+
sink(valueOf);
40+
// tainted - data preserving method
41+
String valueOfSubstring = String.valueOf(complex.toCharArray(), 0, 1);
42+
sink(valueOfSubstring);
3743
// tainted - unsafe escape
3844
String badEscape = constructed.replaceAll("(<script>)", "");
3945
sink(badEscape);
@@ -49,7 +55,11 @@ public static void maintest() throws java.io.UnsupportedEncodingException, java.
4955
// non-whitelisted constructors don't pass taint
5056
StringWrapper herring = new StringWrapper(complex);
5157
sink(herring);
58+
// toString does not pass taint yet
59+
String valueOfObject = String.valueOf(args);
60+
sink(valueOfObject);
5261

62+
5363
// tainted equality check with constant
5464
boolean cond = "foo" == s;
5565
sink(cond);

‎java/ql/test/library-tests/dataflow/taint/test.expected

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,31 +10,33 @@
1010
| B.java:15:21:15:27 | taint(...) | B.java:30:10:30:15 | method |
1111
| B.java:15:21:15:27 | taint(...) | B.java:33:10:33:16 | complex |
1212
| B.java:15:21:15:27 | taint(...) | B.java:36:10:36:20 | constructed |
13-
| B.java:15:21:15:27 | taint(...) | B.java:39:10:39:18 | badEscape |
14-
| B.java:15:21:15:27 | taint(...) | B.java:42:10:42:14 | token |
15-
| B.java:15:21:15:27 | taint(...) | B.java:55:10:55:13 | cond |
16-
| B.java:15:21:15:27 | taint(...) | B.java:58:10:58:14 | logic |
17-
| B.java:15:21:15:27 | taint(...) | B.java:60:10:60:39 | endsWith(...) |
18-
| B.java:15:21:15:27 | taint(...) | B.java:63:10:63:14 | logic |
19-
| B.java:15:21:15:27 | taint(...) | B.java:66:10:66:14 | logic |
20-
| B.java:15:21:15:27 | taint(...) | B.java:74:10:74:16 | trimmed |
21-
| B.java:15:21:15:27 | taint(...) | B.java:76:10:76:14 | split |
22-
| B.java:15:21:15:27 | taint(...) | B.java:78:10:78:14 | lower |
23-
| B.java:15:21:15:27 | taint(...) | B.java:80:10:80:14 | upper |
24-
| B.java:15:21:15:27 | taint(...) | B.java:82:10:82:14 | bytes |
25-
| B.java:15:21:15:27 | taint(...) | B.java:84:10:84:17 | toString |
26-
| B.java:15:21:15:27 | taint(...) | B.java:86:10:86:13 | subs |
27-
| B.java:15:21:15:27 | taint(...) | B.java:88:10:88:13 | repl |
28-
| B.java:15:21:15:27 | taint(...) | B.java:90:10:90:16 | replAll |
29-
| B.java:15:21:15:27 | taint(...) | B.java:92:10:92:18 | replFirst |
30-
| B.java:15:21:15:27 | taint(...) | B.java:105:12:105:25 | serializedData |
31-
| B.java:15:21:15:27 | taint(...) | B.java:117:12:117:27 | deserializedData |
32-
| B.java:15:21:15:27 | taint(...) | B.java:126:10:126:21 | taintedArray |
33-
| B.java:15:21:15:27 | taint(...) | B.java:128:10:128:22 | taintedArray2 |
34-
| B.java:15:21:15:27 | taint(...) | B.java:130:10:130:22 | taintedArray3 |
35-
| B.java:15:21:15:27 | taint(...) | B.java:133:10:133:44 | toURL(...) |
36-
| B.java:15:21:15:27 | taint(...) | B.java:136:10:136:37 | toPath(...) |
37-
| B.java:15:21:15:27 | taint(...) | B.java:139:10:139:46 | toFile(...) |
13+
| B.java:15:21:15:27 | taint(...) | B.java:39:10:39:16 | valueOf |
14+
| B.java:15:21:15:27 | taint(...) | B.java:42:10:42:25 | valueOfSubstring |
15+
| B.java:15:21:15:27 | taint(...) | B.java:45:10:45:18 | badEscape |
16+
| B.java:15:21:15:27 | taint(...) | B.java:48:10:48:14 | token |
17+
| B.java:15:21:15:27 | taint(...) | B.java:65:10:65:13 | cond |
18+
| B.java:15:21:15:27 | taint(...) | B.java:68:10:68:14 | logic |
19+
| B.java:15:21:15:27 | taint(...) | B.java:70:10:70:39 | endsWith(...) |
20+
| B.java:15:21:15:27 | taint(...) | B.java:73:10:73:14 | logic |
21+
| B.java:15:21:15:27 | taint(...) | B.java:76:10:76:14 | logic |
22+
| B.java:15:21:15:27 | taint(...) | B.java:84:10:84:16 | trimmed |
23+
| B.java:15:21:15:27 | taint(...) | B.java:86:10:86:14 | split |
24+
| B.java:15:21:15:27 | taint(...) | B.java:88:10:88:14 | lower |
25+
| B.java:15:21:15:27 | taint(...) | B.java:90:10:90:14 | upper |
26+
| B.java:15:21:15:27 | taint(...) | B.java:92:10:92:14 | bytes |
27+
| B.java:15:21:15:27 | taint(...) | B.java:94:10:94:17 | toString |
28+
| B.java:15:21:15:27 | taint(...) | B.java:96:10:96:13 | subs |
29+
| B.java:15:21:15:27 | taint(...) | B.java:98:10:98:13 | repl |
30+
| B.java:15:21:15:27 | taint(...) | B.java:100:10:100:16 | replAll |
31+
| B.java:15:21:15:27 | taint(...) | B.java:102:10:102:18 | replFirst |
32+
| B.java:15:21:15:27 | taint(...) | B.java:115:12:115:25 | serializedData |
33+
| B.java:15:21:15:27 | taint(...) | B.java:127:12:127:27 | deserializedData |
34+
| B.java:15:21:15:27 | taint(...) | B.java:136:10:136:21 | taintedArray |
35+
| B.java:15:21:15:27 | taint(...) | B.java:138:10:138:22 | taintedArray2 |
36+
| B.java:15:21:15:27 | taint(...) | B.java:140:10:140:22 | taintedArray3 |
37+
| B.java:15:21:15:27 | taint(...) | B.java:143:10:143:44 | toURL(...) |
38+
| B.java:15:21:15:27 | taint(...) | B.java:146:10:146:37 | toPath(...) |
39+
| B.java:15:21:15:27 | taint(...) | B.java:149:10:149:46 | toFile(...) |
3840
| MethodFlow.java:7:22:7:28 | taint(...) | MethodFlow.java:8:10:8:16 | tainted |
3941
| MethodFlow.java:9:31:9:37 | taint(...) | MethodFlow.java:10:10:10:17 | tainted2 |
4042
| MethodFlow.java:11:35:11:41 | taint(...) | MethodFlow.java:12:10:12:17 | tainted3 |

0 commit comments

Comments
 (0)