Skip to content

Commit df2e2e5

Browse files
authored
Merge pull request #17901 from aschackmull/java/allowlist-sanitizer
Java: Add a default taint sanitizer for contains-checks on lists of constants
2 parents c580046 + 5ef496d commit df2e2e5

15 files changed

+1141
-14
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Calling `coll.contains(x)` is now a taint sanitizer (for any query) for the value `x`, where `coll` is a collection of constants.

Diff for: java/ql/lib/semmle/code/java/dataflow/FlowSteps.qll

+6
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 { }

Diff for: java/ql/lib/semmle/code/java/dataflow/TypeFlow.qll

+31-14
Original file line numberDiff line numberDiff line change
@@ -13,46 +13,55 @@ private import semmle.code.java.dispatch.VirtualDispatch
1313
private import semmle.code.java.dataflow.internal.BaseSSA
1414
private import semmle.code.java.controlflow.Guards
1515
private import codeql.typeflow.TypeFlow
16+
private import codeql.typeflow.UniversalFlow as UniversalFlow
1617

17-
private module Input implements TypeFlowInput<Location> {
18-
private newtype TTypeFlowNode =
18+
/** Gets `t` if it is a `RefType` or the boxed type if `t` is a primitive type. */
19+
private RefType boxIfNeeded(J::Type t) {
20+
t.(PrimitiveType).getBoxedType() = result or
21+
result = t
22+
}
23+
24+
/** Provides the input types and predicates for instantiation of `UniversalFlow`. */
25+
module FlowStepsInput implements UniversalFlow::UniversalFlowInput<Location> {
26+
private newtype TFlowNode =
1927
TField(Field f) { not f.getType() instanceof PrimitiveType } or
2028
TSsa(BaseSsaVariable ssa) { not ssa.getSourceVariable().getType() instanceof PrimitiveType } or
2129
TExpr(Expr e) or
2230
TMethod(Method m) { not m.getReturnType() instanceof PrimitiveType }
2331

24-
/** Gets `t` if it is a `RefType` or the boxed type if `t` is a primitive type. */
25-
private RefType boxIfNeeded(J::Type t) {
26-
t.(PrimitiveType).getBoxedType() = result or
27-
result = t
28-
}
29-
3032
/**
3133
* A `Field`, `BaseSsaVariable`, `Expr`, or `Method`.
3234
*/
33-
class TypeFlowNode extends TTypeFlowNode {
35+
class FlowNode extends TFlowNode {
36+
/** Gets a textual representation of this element. */
3437
string toString() {
3538
result = this.asField().toString() or
3639
result = this.asSsa().toString() or
3740
result = this.asExpr().toString() or
3841
result = this.asMethod().toString()
3942
}
4043

44+
/** Gets the source location for this element. */
4145
Location getLocation() {
4246
result = this.asField().getLocation() or
4347
result = this.asSsa().getLocation() or
4448
result = this.asExpr().getLocation() or
4549
result = this.asMethod().getLocation()
4650
}
4751

52+
/** Gets the field corresponding to this node, if any. */
4853
Field asField() { this = TField(result) }
4954

55+
/** Gets the SSA variable corresponding to this node, if any. */
5056
BaseSsaVariable asSsa() { this = TSsa(result) }
5157

58+
/** Gets the expression corresponding to this node, if any. */
5259
Expr asExpr() { this = TExpr(result) }
5360

61+
/** Gets the method corresponding to this node, if any. */
5462
Method asMethod() { this = TMethod(result) }
5563

64+
/** Gets the type of this node. */
5665
RefType getType() {
5766
result = this.asField().getType() or
5867
result = this.asSsa().getSourceVariable().getType() or
@@ -61,8 +70,6 @@ private module Input implements TypeFlowInput<Location> {
6170
}
6271
}
6372

64-
class Type = RefType;
65-
6673
private SrcCallable viableCallable_v1(Call c) {
6774
result = viableImpl_v1(c)
6875
or
@@ -88,7 +95,7 @@ private module Input implements TypeFlowInput<Location> {
8895
*
8996
* For a given `n2`, this predicate must include all possible `n1` that can flow to `n2`.
9097
*/
91-
predicate step(TypeFlowNode n1, TypeFlowNode n2) {
98+
predicate step(FlowNode n1, FlowNode n2) {
9299
n2.asExpr().(ChooseExpr).getAResultExpr() = n1.asExpr()
93100
or
94101
exists(Field f, Expr e |
@@ -134,7 +141,7 @@ private module Input implements TypeFlowInput<Location> {
134141
/**
135142
* Holds if `null` is the only value that flows to `n`.
136143
*/
137-
predicate isNullValue(TypeFlowNode n) {
144+
predicate isNullValue(FlowNode n) {
138145
n.asExpr() instanceof NullLiteral
139146
or
140147
exists(LocalVariableDeclExpr decl |
@@ -144,11 +151,21 @@ private module Input implements TypeFlowInput<Location> {
144151
)
145152
}
146153

147-
predicate isExcludedFromNullAnalysis(TypeFlowNode n) {
154+
predicate isExcludedFromNullAnalysis(FlowNode n) {
148155
// Fields that are never assigned a non-null value are probably set by
149156
// reflection and are thus not always null.
150157
exists(n.asField())
151158
}
159+
}
160+
161+
private module Input implements TypeFlowInput<Location> {
162+
import FlowStepsInput
163+
164+
class TypeFlowNode = FlowNode;
165+
166+
predicate isExcludedFromNullAnalysis = FlowStepsInput::isExcludedFromNullAnalysis/1;
167+
168+
class Type = RefType;
152169

153170
predicate exactTypeBase(TypeFlowNode n, RefType t) {
154171
exists(ClassInstanceExpr e |

Diff for: java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll

+4
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ class BaseSsaSourceVariable extends TBaseSsaSourceVariable {
3838
/** Gets the `Callable` in which this `BaseSsaSourceVariable` is defined. */
3939
Callable getEnclosingCallable() { this = TLocalVar(result, _) }
4040

41+
/** Gets a textual representation of this element. */
4142
string toString() {
4243
exists(LocalScopeVariable v, Callable c | this = TLocalVar(c, v) |
4344
if c = v.getCallable()
@@ -46,6 +47,7 @@ class BaseSsaSourceVariable extends TBaseSsaSourceVariable {
4647
)
4748
}
4849

50+
/** Gets the source location for this element. */
4951
Location getLocation() {
5052
exists(LocalScopeVariable v | this = TLocalVar(_, v) and result = v.getLocation())
5153
}
@@ -482,8 +484,10 @@ class BaseSsaVariable extends TBaseSsaVariable {
482484
this = TSsaEntryDef(_, result)
483485
}
484486

487+
/** Gets a textual representation of this element. */
485488
string toString() { none() }
486489

490+
/** Gets the source location for this element. */
487491
Location getLocation() { result = this.getCfgNode().getLocation() }
488492

489493
/** Gets the `BasicBlock` in which this SSA variable is defined. */

Diff for: java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll

+1
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

0 commit comments

Comments
 (0)