Skip to content

Commit 6f32c41

Browse files
aschackmullaschackmull
committed
Java: Add a default taint sanitizer for contains-checks on lists of constants.
1 parent 7f86f8c commit 6f32c41

File tree

3 files changed

+266
-0
lines changed

3 files changed

+266
-0
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ private module Frameworks {
2828
private import semmle.code.java.frameworks.ThreadLocal
2929
private import semmle.code.java.frameworks.ratpack.RatpackExec
3030
private import semmle.code.java.frameworks.stapler.Stapler
31+
private import semmle.code.java.security.ListOfConstantsSanitizer
3132
}
3233

3334
/**
@@ -189,3 +190,8 @@ private class NumberTaintPreservingCallable extends TaintPreservingCallable {
189190
* map-key and map-value content, so that e.g. a tainted `Map` is assumed to have tainted keys and values.
190191
*/
191192
abstract class TaintInheritingContent extends DataFlow::Content { }
193+
194+
/**
195+
* A sanitizer in all global taint flow configurations but not in local taint.
196+
*/
197+
abstract class DefaultTaintSanitizer extends DataFlow::Node { }

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ private module Cached {
161161
*/
162162
cached
163163
predicate defaultTaintSanitizer(DataFlow::Node node) {
164+
node instanceof DefaultTaintSanitizer or
164165
// Ignore paths through test code.
165166
node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass or
166167
node.asExpr() instanceof ValidatedVariableAccess

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

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
/**
2+
* Provides a default taint sanitizer identifying comparisons against lists of
3+
* compile-time constants.
4+
*/
5+
6+
import java
7+
private import codeql.typeflow.UniversalFlow as UniversalFlow
8+
private import semmle.code.java.Collections
9+
private import semmle.code.java.controlflow.Guards
10+
private import semmle.code.java.dataflow.internal.BaseSSA
11+
private import semmle.code.java.dataflow.TaintTracking
12+
private import semmle.code.java.dataflow.TypeFlow
13+
private import semmle.code.java.dispatch.VirtualDispatch
14+
15+
private class FlowNode = FlowStepsInput::FlowNode;
16+
17+
/**
18+
* Holds if `n2` is an unmodifiable collection constructed from input `n1`,
19+
* which is either another collection or a number of elements.
20+
*/
21+
private predicate unmodifiableCollectionStep(FlowNode n1, FlowNode n2) {
22+
exists(Call c, Callable tgt |
23+
n2.asExpr() = c and
24+
n1.asExpr() = c.getAnArgument() and
25+
c.getCallee().getSourceDeclaration() = tgt
26+
|
27+
tgt.hasQualifiedName("java.util", "Collections",
28+
["unmodifiableCollection", "unmodifiableList", "unmodifiableSet"])
29+
or
30+
tgt.hasQualifiedName("java.util", ["List", "Set"], ["copyOf", "of"])
31+
or
32+
tgt.hasQualifiedName("com.google.common.collect", ["ImmutableList", "ImmutableSet"],
33+
["copyOf", "of"])
34+
)
35+
}
36+
37+
/**
38+
* Holds if `n2` is a collection or array constructed from input `n1`, which is
39+
* either a collection, an array, or a number of elements.
40+
*/
41+
private predicate collectionStep(FlowNode n1, FlowNode n2) {
42+
n2.asExpr().(ArrayInit).getAnInit() = n1.asExpr()
43+
or
44+
n2.asExpr().(ArrayCreationExpr).getInit() = n1.asExpr()
45+
or
46+
unmodifiableCollectionStep(n1, n2)
47+
or
48+
exists(Call c, Callable tgt |
49+
n2.asExpr() = c and
50+
n1.asExpr() = c.getAnArgument() and
51+
c.getCallee().getSourceDeclaration() = tgt
52+
|
53+
tgt.hasQualifiedName("java.util", "Arrays", "asList")
54+
or
55+
tgt.isStatic() and
56+
tgt.hasName(["copyOf", "of"]) and
57+
tgt.getDeclaringType().getASourceSupertype+().hasQualifiedName("java.util", "Collection")
58+
or
59+
tgt instanceof Constructor and
60+
tgt.getNumberOfParameters() = 1 and
61+
tgt.getParameterType(0) instanceof CollectionType and
62+
tgt.getDeclaringType() instanceof CollectionType
63+
)
64+
}
65+
66+
private module BaseUniversalFlow = UniversalFlow::Make<Location, FlowStepsInput>;
67+
68+
private module UnmodifiableProp implements BaseUniversalFlow::NullaryPropertySig {
69+
predicate hasPropertyBase(FlowNode n) { unmodifiableCollectionStep(_, n) }
70+
}
71+
72+
/** Holds if the given node is an unmodifiable collection. */
73+
private predicate unmodifiableCollection =
74+
BaseUniversalFlow::FlowNullary<UnmodifiableProp>::hasProperty/1;
75+
76+
/**
77+
* Holds if `v` is a collection or array with an access, `coll`, at which the
78+
* element `e` gets added.
79+
*/
80+
private predicate collectionAddition(Variable v, VarAccess coll, Expr e) {
81+
exists(MethodCall mc, Method m, int arg |
82+
mc.getMethod().getSourceDeclaration().overridesOrInstantiates*(m) and
83+
mc.getQualifier() = coll and
84+
v.getAnAccess() = coll and
85+
mc.getArgument(arg) = e
86+
|
87+
m.hasQualifiedName("java.util", "Collection", ["add", "addAll"]) and
88+
m.getNumberOfParameters() = 1 and
89+
arg = 0
90+
or
91+
m.hasQualifiedName("java.util", "List", ["add", "addAll"]) and
92+
m.getNumberOfParameters() = 2 and
93+
arg = 1
94+
)
95+
or
96+
v.getAnAccess() = coll and
97+
exists(Assignment assign | assign.getSource() = e |
98+
coll = assign.getDest().(ArrayAccess).getArray()
99+
)
100+
}
101+
102+
/**
103+
* Holds if `n` represents a definition of `v` and `v` is a collection or
104+
* array that has additions occurring as side-effects after its definition.
105+
*/
106+
private predicate nodeWithAddition(FlowNode n, Variable v) {
107+
collectionAddition(v, _, _) and
108+
(
109+
n.asField() = v
110+
or
111+
n.asSsa().getSourceVariable().getVariable() = v and
112+
(n.asSsa() instanceof BaseSsaUpdate or n.asSsa().(BaseSsaImplicitInit).isParameterDefinition(_))
113+
)
114+
}
115+
116+
/** Holds if `c` does not add elements to the given collection. */
117+
private predicate safeCallable(Callable c) {
118+
c instanceof CollectionQueryMethod
119+
or
120+
c instanceof CollectionMethod and
121+
c.hasName(["clear", "remove", "removeAll", "stream", "iterator", "toArray"])
122+
or
123+
c.hasQualifiedName("org.apache.commons.lang3", "StringUtils", "join")
124+
}
125+
126+
/**
127+
* Holds if `n` might be mutated in ways that adds elements that are not
128+
* tracked by the `collectionAddition` predicate.
129+
*/
130+
private predicate collectionWithPossibleMutation(FlowNode n) {
131+
not unmodifiableCollection(n) and
132+
(
133+
exists(Expr e |
134+
n.asExpr() = e and
135+
(e.getType() instanceof CollectionType or e.getType() instanceof Array) and
136+
not collectionAddition(_, e, _) and
137+
not collectionStep(n, _)
138+
|
139+
exists(ArrayAccess aa | e = aa.getArray())
140+
or
141+
exists(Call c, Callable tgt | c.getAnArgument() = e or c.getQualifier() = e |
142+
tgt = c.getCallee().getSourceDeclaration() and
143+
not safeCallable(tgt)
144+
)
145+
)
146+
or
147+
exists(FlowNode mid |
148+
FlowStepsInput::step(n, mid) and
149+
collectionWithPossibleMutation(mid)
150+
)
151+
)
152+
}
153+
154+
/**
155+
* A collection constructor that constructs an empty mutable collection.
156+
*/
157+
private class EmptyCollectionConstructor extends Constructor {
158+
EmptyCollectionConstructor() {
159+
this.getDeclaringType() instanceof CollectionType and
160+
forall(Type t | t = this.getAParamType() | t instanceof PrimitiveType)
161+
}
162+
}
163+
164+
private module CollectionFlowStepsInput implements UniversalFlow::UniversalFlowInput<Location> {
165+
import FlowStepsInput
166+
167+
/**
168+
* Holds if `n2` is a collection/array/constant whose value(s) are
169+
* determined completely from the range of `n1` nodes.
170+
*/
171+
predicate step(FlowNode n1, FlowNode n2) {
172+
// Exclude the regular input constraints for those nodes that are covered
173+
// completely by `collectionStep`.
174+
FlowStepsInput::step(n1, n2) and
175+
not collectionStep(_, n2)
176+
or
177+
// For collections with side-effects in the form of additions, we add the
178+
// sources of those additions as additional input that need to originate
179+
// from constants.
180+
exists(Variable v |
181+
nodeWithAddition(n2, v) and
182+
collectionAddition(v, _, n1.asExpr())
183+
)
184+
or
185+
// Include various forms of collection transformation.
186+
collectionStep(n1, n2)
187+
}
188+
189+
predicate isExcludedFromNullAnalysis = FlowStepsInput::isExcludedFromNullAnalysis/1;
190+
}
191+
192+
private module CollectionUniversalFlow = UniversalFlow::Make<Location, CollectionFlowStepsInput>;
193+
194+
private module ConstantCollectionProp implements CollectionUniversalFlow::NullaryPropertySig {
195+
/**
196+
* Holds if `n` forms the base case for finding collections of constants.
197+
* These are individual constants and empty collections.
198+
*/
199+
predicate hasPropertyBase(FlowNode n) {
200+
n.asExpr().isCompileTimeConstant() or
201+
n.asExpr().(ConstructorCall).getConstructor() instanceof EmptyCollectionConstructor
202+
}
203+
204+
predicate barrier = collectionWithPossibleMutation/1;
205+
}
206+
207+
/**
208+
* Holds if the given node is either a constant or a collection/array of
209+
* constants.
210+
*/
211+
private predicate constantCollection =
212+
CollectionUniversalFlow::FlowNullary<ConstantCollectionProp>::hasProperty/1;
213+
214+
/** Gets the result of a case normalization call of `arg`. */
215+
private MethodCall normalizeCaseCall(Expr arg) {
216+
exists(Method changecase | result.getMethod() = changecase |
217+
changecase.hasName(["toUpperCase", "toLowerCase"]) and
218+
changecase.getDeclaringType() instanceof TypeString and
219+
arg = result.getQualifier()
220+
or
221+
changecase
222+
.hasQualifiedName(["org.apache.commons.lang", "org.apache.commons.lang3"], "StringUtils",
223+
["lowerCase", "upperCase"]) and
224+
arg = result.getArgument(0)
225+
or
226+
changecase
227+
.hasQualifiedName("org.apache.hadoop.util", "StringUtils", ["toLowerCase", "toUpperCase"]) and
228+
arg = result.getArgument(0)
229+
)
230+
}
231+
232+
/**
233+
* Holds if the guard `g` ensures that the expression `e` is one of a set of
234+
* known constants upon evaluating to `branch`.
235+
*/
236+
private predicate constantCollectionContainsCheck(Guard g, Expr e, boolean branch) {
237+
exists(MethodCall mc, Method m, FlowNode n, Expr checked |
238+
g = mc and
239+
mc.getMethod().getSourceDeclaration().overridesOrInstantiates*(m) and
240+
m.hasQualifiedName("java.util", "Collection", "contains") and
241+
n.asExpr() = mc.getQualifier() and
242+
constantCollection(n) and
243+
checked = mc.getAnArgument() and
244+
branch = true
245+
|
246+
checked = e or
247+
checked = normalizeCaseCall(e)
248+
)
249+
}
250+
251+
/**
252+
* A comparison against a list of compile-time constants, sanitizing taint by
253+
* restricting to a set of known values.
254+
*/
255+
private class ListOfConstantsComparisonSanitizerGuard extends TaintTracking::DefaultTaintSanitizer {
256+
ListOfConstantsComparisonSanitizerGuard() {
257+
this = DataFlow::BarrierGuard<constantCollectionContainsCheck/3>::getABarrierNode()
258+
}
259+
}

0 commit comments

Comments
 (0)