Skip to content

Commit 3e17fdc

Browse files
aschackmullaschackmull
authored
Merge pull request github#6407 from bmuskalla/charSeqSubSeq
Java: Track taint for CharSequence#subSequence
2 parents 5b8b27a + 9d5e484 commit 3e17fdc

File tree

23 files changed

+245
-146
lines changed

23 files changed

+245
-146
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ private module Frameworks {
8989
private import semmle.code.java.frameworks.JsonJava
9090
private import semmle.code.java.frameworks.Objects
9191
private import semmle.code.java.frameworks.Optional
92+
private import semmle.code.java.frameworks.Strings
9293
private import semmle.code.java.frameworks.spring.SpringCache
9394
private import semmle.code.java.frameworks.spring.SpringHttp
9495
private import semmle.code.java.frameworks.spring.SpringUtil

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

Lines changed: 0 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -84,36 +84,6 @@ abstract class TaintPreservingCallable extends Callable {
8484
predicate transfersTaint(int src, int sink) { none() }
8585
}
8686

87-
private class StringTaintPreservingMethod extends TaintPreservingCallable {
88-
StringTaintPreservingMethod() {
89-
this.getDeclaringType() instanceof TypeString and
90-
(
91-
this.hasName([
92-
"concat", "copyValueOf", "endsWith", "format", "formatted", "getBytes", "indent",
93-
"intern", "join", "repeat", "split", "strip", "stripIndent", "stripLeading",
94-
"stripTrailing", "substring", "toCharArray", "toLowerCase", "toString", "toUpperCase",
95-
"trim"
96-
])
97-
or
98-
this.hasName("valueOf") and this.getParameterType(0) instanceof Array
99-
)
100-
}
101-
102-
override predicate returnsTaintFrom(int arg) {
103-
arg = -1 and not this.isStatic()
104-
or
105-
this.hasName(["concat", "copyValueOf", "valueOf"]) and arg = 0
106-
or
107-
this.hasName(["format", "formatted", "join"]) and arg = [0 .. getNumberOfParameters()]
108-
}
109-
}
110-
111-
private class StringTaintPreservingConstructor extends Constructor, TaintPreservingCallable {
112-
StringTaintPreservingConstructor() { this.getDeclaringType() instanceof TypeString }
113-
114-
override predicate returnsTaintFrom(int arg) { arg = 0 }
115-
}
116-
11787
private class NumberTaintPreservingCallable extends TaintPreservingCallable {
11888
int argument;
11989

@@ -133,46 +103,3 @@ private class NumberTaintPreservingCallable extends TaintPreservingCallable {
133103

134104
override predicate returnsTaintFrom(int arg) { arg = argument }
135105
}
136-
137-
/** Holds for the types `StringBuilder`, `StringBuffer`, and `StringWriter`. */
138-
private predicate stringBuilderType(RefType t) {
139-
t instanceof StringBuildingType or
140-
t.hasQualifiedName("java.io", "StringWriter")
141-
}
142-
143-
private class StringBuilderTaintPreservingCallable extends TaintPreservingCallable {
144-
StringBuilderTaintPreservingCallable() {
145-
exists(Method m |
146-
this.(Method).overrides*(m) and
147-
stringBuilderType(m.getDeclaringType()) and
148-
m.hasName(["append", "insert", "replace", "toString", "write"])
149-
)
150-
or
151-
this.(Constructor).getParameterType(0) instanceof RefType and
152-
stringBuilderType(this.getDeclaringType())
153-
}
154-
155-
override predicate returnsTaintFrom(int arg) {
156-
arg = -1 and
157-
not this instanceof Constructor
158-
or
159-
this instanceof Constructor and arg = 0
160-
or
161-
this.hasName("append") and arg = 0
162-
or
163-
this.hasName("insert") and arg = 1
164-
or
165-
this.hasName("replace") and arg = 2
166-
}
167-
168-
override predicate transfersTaint(int src, int sink) {
169-
returnsTaintFrom(src) and
170-
sink = -1 and
171-
src != -1 and
172-
not this instanceof Constructor
173-
or
174-
this.hasName("write") and
175-
src = 0 and
176-
sink = -1
177-
}
178-
}

‎java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,6 @@ private predicate localAdditionalTaintExprStep(Expr src, Expr sink) {
147147
or
148148
comparisonStep(src, sink)
149149
or
150-
stringBuilderStep(src, sink)
151-
or
152150
serializationStep(src, sink)
153151
or
154152
formatStep(src, sink)
@@ -392,15 +390,6 @@ private predicate comparisonStep(Expr tracked, Expr sink) {
392390
)
393391
}
394392

395-
/** Flow through a `StringBuilder`. */
396-
private predicate stringBuilderStep(Expr tracked, Expr sink) {
397-
exists(StringBuilderVar sbvar, MethodAccess input, int arg |
398-
input = sbvar.getAnInput(arg) and
399-
tracked = input.getArgument(arg) and
400-
sink = sbvar.getToStringCall()
401-
)
402-
}
403-
404393
/** Flow through data serialization. */
405394
private predicate serializationStep(Expr tracked, Expr sink) {
406395
exists(ObjectOutputStreamVar v, VariableAssign def |

‎java/ql/lib/semmle/code/java/frameworks/Objects.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ private class ObjectsSummaryCsv extends SummaryModelCsv {
99
[
1010
//`namespace; type; subtypes; name; signature; ext; input; output; kind`
1111
"java.util;Objects;false;requireNonNull;;;Argument[0];ReturnValue;value",
12-
"java.util;Objects;false;requireNonNullElse;;;Argument[0..1];ReturnValue;value",
12+
"java.util;Objects;false;requireNonNullElse;;;Argument[0];ReturnValue;value",
13+
"java.util;Objects;false;requireNonNullElse;;;Argument[1];ReturnValue;value",
1314
"java.util;Objects;false;requireNonNullElseGet;;;Argument[0];ReturnValue;value",
1415
"java.util;Objects;false;toString;;;Argument[1];ReturnValue;value"
1516
]

‎java/ql/lib/semmle/code/java/frameworks/Strings.qll

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/** Definitions of taint steps in String and String-related classes of the JDK */
2+
3+
import java
4+
private import semmle.code.java.dataflow.ExternalFlow
5+
6+
private class StringSummaryCsv extends SummaryModelCsv {
7+
override predicate row(string row) {
8+
row =
9+
[
10+
//`namespace; type; subtypes; name; signature; ext; input; output; kind`
11+
"java.lang;String;false;concat;(String);;Argument[0];ReturnValue;taint",
12+
"java.lang;String;false;concat;(String);;Argument[-1];ReturnValue;taint",
13+
"java.lang;String;false;copyValueOf;;;Argument[0];ReturnValue;taint",
14+
"java.lang;String;false;endsWith;;;Argument[-1];ReturnValue;taint",
15+
"java.lang;String;false;format;(Locale,String,Object[]);;Argument[1];ReturnValue;taint",
16+
"java.lang;String;false;format;(Locale,String,Object[]);;ArrayElement of Argument[2];ReturnValue;taint",
17+
"java.lang;String;false;format;(String,Object[]);;Argument[0];ReturnValue;taint",
18+
"java.lang;String;false;format;(String,Object[]);;ArrayElement of Argument[1];ReturnValue;taint",
19+
"java.lang;String;false;formatted;(Object[]);;Argument[-1];ReturnValue;taint",
20+
"java.lang;String;false;formatted;(Object[]);;ArrayElement of Argument[0];ReturnValue;taint",
21+
"java.lang;String;false;getBytes;;;Argument[-1];ReturnValue;taint",
22+
"java.lang;String;false;indent;;;Argument[-1];ReturnValue;taint",
23+
"java.lang;String;false;intern;;;Argument[-1];ReturnValue;taint",
24+
"java.lang;String;false;join;;;Argument[0..1];ReturnValue;taint",
25+
"java.lang;String;false;repeat;(int);;Argument[-1];ReturnValue;taint",
26+
"java.lang;String;false;split;;;Argument[-1];ReturnValue;taint",
27+
"java.lang;String;false;String;;;Argument[0];Argument[-1];taint",
28+
"java.lang;String;false;strip;;;Argument[-1];ReturnValue;taint",
29+
"java.lang;String;false;stripIndent;;;Argument[-1];ReturnValue;taint",
30+
"java.lang;String;false;stripLeading;;;Argument[-1];ReturnValue;taint",
31+
"java.lang;String;false;stripTrailing;;;Argument[-1];ReturnValue;taint",
32+
"java.lang;String;false;substring;;;Argument[-1];ReturnValue;taint",
33+
"java.lang;String;false;toCharArray;;;Argument[-1];ReturnValue;taint",
34+
"java.lang;String;false;toLowerCase;;;Argument[-1];ReturnValue;taint",
35+
"java.lang;String;false;toString;;;Argument[-1];ReturnValue;value",
36+
"java.lang;String;false;toUpperCase;;;Argument[-1];ReturnValue;taint",
37+
"java.lang;String;false;trim;;;Argument[-1];ReturnValue;taint",
38+
"java.lang;String;false;valueOf;(char);;Argument[0];ReturnValue;taint",
39+
"java.lang;String;false;valueOf;(char[],int,int);;Argument[0];ReturnValue;taint",
40+
"java.lang;String;false;valueOf;(char[]);;Argument[0];ReturnValue;taint",
41+
"java.io;StringWriter;true;append;;;Argument[0];Argument[-1];taint",
42+
"java.io;StringWriter;true;append;;;Argument[-1];ReturnValue;value",
43+
"java.io;StringWriter;true;write;;;Argument[0];Argument[-1];taint",
44+
"java.lang;AbstractStringBuilder;true;AbstractStringBuilder;(String);;Argument[0];Argument[-1];taint",
45+
"java.lang;AbstractStringBuilder;true;append;;;Argument[0];Argument[-1];taint",
46+
"java.lang;AbstractStringBuilder;true;append;;;Argument[-1];ReturnValue;value",
47+
"java.lang;AbstractStringBuilder;true;insert;;;Argument[1];Argument[-1];taint",
48+
"java.lang;AbstractStringBuilder;true;insert;;;Argument[-1];ReturnValue;value",
49+
"java.lang;AbstractStringBuilder;true;replace;;;Argument[-1];ReturnValue;value",
50+
"java.lang;AbstractStringBuilder;true;replace;;;Argument[2];Argument[-1];taint",
51+
"java.lang;AbstractStringBuilder;true;toString;;;Argument[-1];ReturnValue;taint",
52+
"java.lang;StringBuffer;true;StringBuffer;(CharSequence);;Argument[0];Argument[-1];taint",
53+
"java.lang;StringBuffer;true;StringBuffer;(String);;Argument[0];Argument[-1];taint",
54+
"java.lang;StringBuilder;true;StringBuilder;;;Argument[0];Argument[-1];taint",
55+
"java.lang;CharSequence;true;subSequence;;;Argument[-1];ReturnValue;taint"
56+
]
57+
}
58+
}

‎java/ql/test/experimental/query-tests/security/CWE-094/JakartaExpressionInjection.expected

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
edges
22
| JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | JakartaExpressionInjection.java:23:54:23:58 | bytes [post update] : byte[] |
3-
| JakartaExpressionInjection.java:23:54:23:58 | bytes [post update] : byte[] | JakartaExpressionInjection.java:25:31:25:40 | expression : String |
3+
| JakartaExpressionInjection.java:23:54:23:58 | bytes [post update] : byte[] | JakartaExpressionInjection.java:24:48:24:52 | bytes : byte[] |
4+
| JakartaExpressionInjection.java:24:37:24:59 | new String(...) : String | JakartaExpressionInjection.java:25:31:25:40 | expression : String |
5+
| JakartaExpressionInjection.java:24:48:24:52 | bytes : byte[] | JakartaExpressionInjection.java:24:37:24:59 | new String(...) : String |
46
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:32:24:32:33 | expression : String |
57
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:40:24:40:33 | expression : String |
68
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | JakartaExpressionInjection.java:48:24:48:33 | expression : String |
@@ -20,6 +22,8 @@ edges
2022
nodes
2123
| JakartaExpressionInjection.java:23:25:23:47 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |
2224
| JakartaExpressionInjection.java:23:54:23:58 | bytes [post update] : byte[] | semmle.label | bytes [post update] : byte[] |
25+
| JakartaExpressionInjection.java:24:37:24:59 | new String(...) : String | semmle.label | new String(...) : String |
26+
| JakartaExpressionInjection.java:24:48:24:52 | bytes : byte[] | semmle.label | bytes : byte[] |
2327
| JakartaExpressionInjection.java:25:31:25:40 | expression : String | semmle.label | expression : String |
2428
| JakartaExpressionInjection.java:32:24:32:33 | expression : String | semmle.label | expression : String |
2529
| JakartaExpressionInjection.java:34:28:34:37 | expression | semmle.label | expression |

‎java/ql/test/experimental/query-tests/security/CWE-094/JythonInjection.expected

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ edges
22
| JythonInjection.java:28:23:28:50 | getParameter(...) : String | JythonInjection.java:36:30:36:33 | code |
33
| JythonInjection.java:53:23:53:50 | getParameter(...) : String | JythonInjection.java:58:44:58:47 | code |
44
| JythonInjection.java:73:23:73:50 | getParameter(...) : String | JythonInjection.java:81:35:81:38 | code |
5-
| JythonInjection.java:97:23:97:50 | getParameter(...) : String | JythonInjection.java:106:61:106:75 | getBytes(...) |
5+
| JythonInjection.java:97:23:97:50 | getParameter(...) : String | JythonInjection.java:106:61:106:64 | code : String |
6+
| JythonInjection.java:106:61:106:64 | code : String | JythonInjection.java:106:61:106:75 | getBytes(...) |
67
nodes
78
| JythonInjection.java:28:23:28:50 | getParameter(...) : String | semmle.label | getParameter(...) : String |
89
| JythonInjection.java:36:30:36:33 | code | semmle.label | code |
@@ -11,6 +12,7 @@ nodes
1112
| JythonInjection.java:73:23:73:50 | getParameter(...) : String | semmle.label | getParameter(...) : String |
1213
| JythonInjection.java:81:35:81:38 | code | semmle.label | code |
1314
| JythonInjection.java:97:23:97:50 | getParameter(...) : String | semmle.label | getParameter(...) : String |
15+
| JythonInjection.java:106:61:106:64 | code : String | semmle.label | code : String |
1416
| JythonInjection.java:106:61:106:75 | getBytes(...) | semmle.label | getBytes(...) |
1517
| JythonInjection.java:131:40:131:63 | getInputStream(...) | semmle.label | getInputStream(...) |
1618
subpaths

‎java/ql/test/experimental/query-tests/security/CWE-094/ScriptInjection.expected

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
edges
22
| RhinoServlet.java:28:23:28:50 | getParameter(...) : String | RhinoServlet.java:32:55:32:58 | code |
33
| RhinoServlet.java:81:23:81:50 | getParameter(...) : String | RhinoServlet.java:83:54:83:57 | code |
4-
| RhinoServlet.java:88:23:88:50 | getParameter(...) : String | RhinoServlet.java:89:74:89:88 | getBytes(...) |
4+
| RhinoServlet.java:88:23:88:50 | getParameter(...) : String | RhinoServlet.java:89:74:89:77 | code : String |
5+
| RhinoServlet.java:89:74:89:77 | code : String | RhinoServlet.java:89:74:89:88 | getBytes(...) |
56
| ScriptEngineTest.java:20:44:20:55 | input : String | ScriptEngineTest.java:24:37:24:41 | input |
67
| ScriptEngineTest.java:27:51:27:62 | input : String | ScriptEngineTest.java:31:31:31:35 | input |
78
| ScriptEngineTest.java:35:58:35:69 | input : String | ScriptEngineTest.java:39:31:39:35 | input |
@@ -26,6 +27,7 @@ nodes
2627
| RhinoServlet.java:81:23:81:50 | getParameter(...) : String | semmle.label | getParameter(...) : String |
2728
| RhinoServlet.java:83:54:83:57 | code | semmle.label | code |
2829
| RhinoServlet.java:88:23:88:50 | getParameter(...) : String | semmle.label | getParameter(...) : String |
30+
| RhinoServlet.java:89:74:89:77 | code : String | semmle.label | code : String |
2931
| RhinoServlet.java:89:74:89:88 | getBytes(...) | semmle.label | getBytes(...) |
3032
| ScriptEngineTest.java:20:44:20:55 | input : String | semmle.label | input : String |
3133
| ScriptEngineTest.java:24:37:24:41 | input | semmle.label | input |

0 commit comments

Comments
 (0)