Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java: Add a default taint sanitizer for contains-checks on lists of constants #17901

Merged
merged 11 commits into from
Nov 27, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Calling `coll.contains(x)` is now a taint sanitizer (for any query) for the value `x`, where `coll` is a collection of constants.
6 changes: 6 additions & 0 deletions java/ql/lib/semmle/code/java/dataflow/FlowSteps.qll
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ private module Frameworks {
private import semmle.code.java.frameworks.ThreadLocal
private import semmle.code.java.frameworks.ratpack.RatpackExec
private import semmle.code.java.frameworks.stapler.Stapler
private import semmle.code.java.security.ListOfConstantsSanitizer
}

/**
Expand Down Expand Up @@ -189,3 +190,8 @@ private class NumberTaintPreservingCallable extends TaintPreservingCallable {
* map-key and map-value content, so that e.g. a tainted `Map` is assumed to have tainted keys and values.
*/
abstract class TaintInheritingContent extends DataFlow::Content { }

/**
* A sanitizer in all global taint flow configurations but not in local taint.
*/
abstract class DefaultTaintSanitizer extends DataFlow::Node { }
45 changes: 31 additions & 14 deletions java/ql/lib/semmle/code/java/dataflow/TypeFlow.qll
Original file line number Diff line number Diff line change
Expand Up @@ -13,46 +13,55 @@ private import semmle.code.java.dispatch.VirtualDispatch
private import semmle.code.java.dataflow.internal.BaseSSA
private import semmle.code.java.controlflow.Guards
private import codeql.typeflow.TypeFlow
private import codeql.typeflow.UniversalFlow as UniversalFlow

private module Input implements TypeFlowInput<Location> {
private newtype TTypeFlowNode =
/** Gets `t` if it is a `RefType` or the boxed type if `t` is a primitive type. */
private RefType boxIfNeeded(J::Type t) {
t.(PrimitiveType).getBoxedType() = result or
result = t
}

/** Provides the input types and predicates for instantiation of `UniversalFlow`. */
module FlowStepsInput implements UniversalFlow::UniversalFlowInput<Location> {
private newtype TFlowNode =
TField(Field f) { not f.getType() instanceof PrimitiveType } or
TSsa(BaseSsaVariable ssa) { not ssa.getSourceVariable().getType() instanceof PrimitiveType } or
TExpr(Expr e) or
TMethod(Method m) { not m.getReturnType() instanceof PrimitiveType }

/** Gets `t` if it is a `RefType` or the boxed type if `t` is a primitive type. */
private RefType boxIfNeeded(J::Type t) {
t.(PrimitiveType).getBoxedType() = result or
result = t
}

/**
* A `Field`, `BaseSsaVariable`, `Expr`, or `Method`.
*/
class TypeFlowNode extends TTypeFlowNode {
class FlowNode extends TFlowNode {
/** Gets a textual representation of this element. */
string toString() {
result = this.asField().toString() or
result = this.asSsa().toString() or
result = this.asExpr().toString() or
result = this.asMethod().toString()
}

/** Gets the source location for this element. */
Location getLocation() {
result = this.asField().getLocation() or
result = this.asSsa().getLocation() or
result = this.asExpr().getLocation() or
result = this.asMethod().getLocation()
}

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

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

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

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

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

class Type = RefType;

private SrcCallable viableCallable_v1(Call c) {
result = viableImpl_v1(c)
or
Expand All @@ -88,7 +95,7 @@ private module Input implements TypeFlowInput<Location> {
*
* For a given `n2`, this predicate must include all possible `n1` that can flow to `n2`.
*/
predicate step(TypeFlowNode n1, TypeFlowNode n2) {
predicate step(FlowNode n1, FlowNode n2) {
n2.asExpr().(ChooseExpr).getAResultExpr() = n1.asExpr()
or
exists(Field f, Expr e |
Expand Down Expand Up @@ -134,7 +141,7 @@ private module Input implements TypeFlowInput<Location> {
/**
* Holds if `null` is the only value that flows to `n`.
*/
predicate isNullValue(TypeFlowNode n) {
predicate isNullValue(FlowNode n) {
n.asExpr() instanceof NullLiteral
or
exists(LocalVariableDeclExpr decl |
Expand All @@ -144,11 +151,21 @@ private module Input implements TypeFlowInput<Location> {
)
}

predicate isExcludedFromNullAnalysis(TypeFlowNode n) {
predicate isExcludedFromNullAnalysis(FlowNode n) {
// Fields that are never assigned a non-null value are probably set by
// reflection and are thus not always null.
exists(n.asField())
}
}

private module Input implements TypeFlowInput<Location> {
import FlowStepsInput

class TypeFlowNode = FlowNode;

predicate isExcludedFromNullAnalysis = FlowStepsInput::isExcludedFromNullAnalysis/1;

class Type = RefType;

predicate exactTypeBase(TypeFlowNode n, RefType t) {
exists(ClassInstanceExpr e |
Expand Down
4 changes: 4 additions & 0 deletions java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class BaseSsaSourceVariable extends TBaseSsaSourceVariable {
/** Gets the `Callable` in which this `BaseSsaSourceVariable` is defined. */
Callable getEnclosingCallable() { this = TLocalVar(result, _) }

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

/** Gets the source location for this element. */
Location getLocation() {
exists(LocalScopeVariable v | this = TLocalVar(_, v) and result = v.getLocation())
}
Expand Down Expand Up @@ -482,8 +484,10 @@ class BaseSsaVariable extends TBaseSsaVariable {
this = TSsaEntryDef(_, result)
}

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

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

/** Gets the `BasicBlock` in which this SSA variable is defined. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ private module Cached {
*/
cached
predicate defaultTaintSanitizer(DataFlow::Node node) {
node instanceof DefaultTaintSanitizer or
// Ignore paths through test code.
node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass or
node.asExpr() instanceof ValidatedVariableAccess
Expand Down
Loading