diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowNodes.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowNodes.qll index 552849c2c34..c47efa287ad 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowNodes.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowNodes.qll @@ -4,71 +4,48 @@ private import semmle.code.java.dataflow.FlowSummary private import semmle.code.java.dataflow.TypeFlow private import DataFlowPrivate private import FlowSummaryImpl as FlowSummaryImpl +private import DataFlowImplCommon as DataFlowImplCommon cached -private module Cached { - cached - newtype TNode = - TExprNode(Expr e) { - not e.getType() instanceof VoidType and - not e.getParent*() instanceof Annotation - } or - TExplicitParameterNode(Parameter p) { - exists(p.getCallable().getBody()) or p.getCallable() instanceof SummarizedCallable - } or - TImplicitVarargsArray(Call c) { - c.getCallee().isVarargs() and - not exists(Argument arg | arg.getCall() = c and arg.isExplicitVarargsArray()) - } or - TInstanceParameterNode(Callable c) { - (exists(c.getBody()) or c instanceof SummarizedCallable) and - not c.isStatic() - } or - TImplicitInstanceAccess(InstanceAccessExt ia) { not ia.isExplicit(_) } or - TMallocNode(ClassInstanceExpr cie) or - TExplicitExprPostUpdate(Expr e) { - explicitInstanceArgument(_, e) - or - e instanceof Argument and not e.getType() instanceof ImmutableType - or - exists(FieldAccess fa | fa.getField() instanceof InstanceField and e = fa.getQualifier()) - or - exists(ArrayAccess aa | e = aa.getArray()) - } or - TImplicitExprPostUpdate(InstanceAccessExt ia) { - implicitInstanceArgument(_, ia) - or - exists(FieldAccess fa | - fa.getField() instanceof InstanceField and ia.isImplicitFieldQualifier(fa) - ) - } or - TSummaryInternalNode(SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) { - FlowSummaryImpl::Private::summaryNodeRange(c, state) - } - - cached - predicate summaryOutNodeCached(DataFlowCall c, Node out) { - FlowSummaryImpl::Private::summaryOutNode(c, out, _) +newtype TNode = + TExprNode(Expr e) { + DataFlowImplCommon::forceCachingInSameStage() and + not e.getType() instanceof VoidType and + not e.getParent*() instanceof Annotation + } or + TExplicitParameterNode(Parameter p) { + exists(p.getCallable().getBody()) or p.getCallable() instanceof SummarizedCallable + } or + TImplicitVarargsArray(Call c) { + c.getCallee().isVarargs() and + not exists(Argument arg | arg.getCall() = c and arg.isExplicitVarargsArray()) + } or + TInstanceParameterNode(Callable c) { + (exists(c.getBody()) or c instanceof SummarizedCallable) and + not c.isStatic() + } or + TImplicitInstanceAccess(InstanceAccessExt ia) { not ia.isExplicit(_) } or + TMallocNode(ClassInstanceExpr cie) or + TExplicitExprPostUpdate(Expr e) { + explicitInstanceArgument(_, e) + or + e instanceof Argument and not e.getType() instanceof ImmutableType + or + exists(FieldAccess fa | fa.getField() instanceof InstanceField and e = fa.getQualifier()) + or + exists(ArrayAccess aa | e = aa.getArray()) + } or + TImplicitExprPostUpdate(InstanceAccessExt ia) { + implicitInstanceArgument(_, ia) + or + exists(FieldAccess fa | + fa.getField() instanceof InstanceField and ia.isImplicitFieldQualifier(fa) + ) + } or + TSummaryInternalNode(SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) { + FlowSummaryImpl::Private::summaryNodeRange(c, state) } - cached - predicate summaryArgumentNodeCached(DataFlowCall c, Node arg, int i) { - FlowSummaryImpl::Private::summaryArgumentNode(c, arg, i) - } - - cached - predicate summaryPostUpdateNodeCached(Node post, ParameterNode pre) { - FlowSummaryImpl::Private::summaryPostUpdateNode(post, pre) - } - - cached - predicate summaryReturnNodeCached(Node ret) { - FlowSummaryImpl::Private::summaryReturnNode(ret, _) - } -} - -private import Cached - private predicate explicitInstanceArgument(Call call, Expr instarg) { call instanceof MethodAccess and instarg = call.getQualifier() and @@ -404,13 +381,15 @@ module Private { override string toString() { result = "[summary] " + state + " in " + c } /** Holds if this summary node is the `i`th argument of `call`. */ - predicate isArgumentOf(DataFlowCall call, int i) { summaryArgumentNodeCached(call, this, i) } + predicate isArgumentOf(DataFlowCall call, int i) { + FlowSummaryImpl::Private::summaryArgumentNode(call, this, i) + } /** Holds if this summary node is a return node. */ - predicate isReturn() { summaryReturnNodeCached(this) } + predicate isReturn() { FlowSummaryImpl::Private::summaryReturnNode(this, _) } /** Holds if this summary node is an out node for `call`. */ - predicate isOut(DataFlowCall call) { summaryOutNodeCached(call, this) } + predicate isOut(DataFlowCall call) { FlowSummaryImpl::Private::summaryOutNode(call, this, _) } } SummaryNode getSummaryNode(SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) { @@ -439,7 +418,7 @@ private class MallocNode extends Node, TMallocNode { private class SummaryPostUpdateNode extends SummaryNode, PostUpdateNode { private Node pre; - SummaryPostUpdateNode() { summaryPostUpdateNodeCached(this, pre) } + SummaryPostUpdateNode() { FlowSummaryImpl::Private::summaryPostUpdateNode(this, pre) } override Node getPreUpdateNode() { result = pre } } diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowPrivate.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowPrivate.qll index 23d84c10c9f..5756c56ba6d 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowPrivate.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowPrivate.qll @@ -284,7 +284,6 @@ private class ConstantBooleanArgumentNode extends ArgumentNode, ExprNode { /** * Holds if the node `n` is unreachable when the call context is `call`. */ -cached predicate isUnreachableInCall(Node n, DataFlowCall call) { exists( ExplicitParameterNode paramNode, ConstantBooleanArgumentNode arg, SsaImplicitInit param, diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll index 2ad09574015..e25ab24cfbb 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll @@ -11,6 +11,7 @@ private import semmle.code.java.dataflow.FlowSteps private import semmle.code.java.dataflow.FlowSummary import semmle.code.java.dataflow.InstanceAccess private import FlowSummaryImpl as FlowSummaryImpl +private import TaintTrackingUtil as TaintTrackingUtil import DataFlowNodes::Public /** Holds if `n` is an access to an unqualified `this` at `cfgnode`. */ @@ -112,6 +113,7 @@ predicate localFlowStep(Node node1, Node node2) { */ cached predicate simpleLocalFlowStep(Node node1, Node node2) { + TaintTrackingUtil::forceCachingInSameStage() and // Variable flow steps through adjacent def-use and use-use pairs. exists(SsaExplicitUpdate upd | upd.getDefiningExpr().(VariableAssign).getSource() = node1.asExpr() or diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll index f9278ab815e..8c514e357d2 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll @@ -29,55 +29,69 @@ predicate localExprTaint(Expr src, Expr sink) { localTaint(DataFlow::exprNode(src), DataFlow::exprNode(sink)) } -/** - * Holds if taint can flow in one local step from `src` to `sink`. - */ -predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) { - DataFlow::localFlowStep(src, sink) or - localAdditionalTaintStep(src, sink) or - // Simple flow through library code is included in the exposed local - // step relation, even though flow is technically inter-procedural - FlowSummaryImpl::Private::Steps::summaryThroughStep(src, sink, false) +cached +private module Cached { + private import DataFlowImplCommon as DataFlowImplCommon + + cached + predicate forceCachingInSameStage() { DataFlowImplCommon::forceCachingInSameStage() } + + /** + * Holds if taint can flow in one local step from `src` to `sink`. + */ + cached + predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) { + DataFlow::localFlowStep(src, sink) or + localAdditionalTaintStep(src, sink) or + // Simple flow through library code is included in the exposed local + // step relation, even though flow is technically inter-procedural + FlowSummaryImpl::Private::Steps::summaryThroughStep(src, sink, false) + } + + /** + * Holds if taint can flow in one local step from `src` to `sink` excluding + * local data flow steps. That is, `src` and `sink` are likely to represent + * different objects. + */ + cached + predicate localAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { + localAdditionalTaintExprStep(src.asExpr(), sink.asExpr()) + or + localAdditionalTaintUpdateStep(src.asExpr(), + sink.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr()) + or + exists(Argument arg | + src.asExpr() = arg and + arg.isVararg() and + sink.(DataFlow::ImplicitVarargsArray).getCall() = arg.getCall() + ) + or + FlowSummaryImpl::Private::Steps::summaryLocalStep(src, sink, false) + } + + /** + * Holds if the additional step from `src` to `sink` should be included in all + * global taint flow configurations. + */ + cached + predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { + localAdditionalTaintStep(src, sink) or + any(AdditionalTaintStep a).step(src, sink) + } + + /** + * Holds if `node` should be a sanitizer in all global taint flow configurations + * but not in local taint. + */ + cached + predicate defaultTaintSanitizer(DataFlow::Node node) { + // Ignore paths through test code. + node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass or + node.asExpr() instanceof ValidatedVariableAccess + } } -/** - * Holds if taint can flow in one local step from `src` to `sink` excluding - * local data flow steps. That is, `src` and `sink` are likely to represent - * different objects. - */ -predicate localAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { - localAdditionalTaintExprStep(src.asExpr(), sink.asExpr()) - or - localAdditionalTaintUpdateStep(src.asExpr(), - sink.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr()) - or - exists(Argument arg | - src.asExpr() = arg and - arg.isVararg() and - sink.(DataFlow::ImplicitVarargsArray).getCall() = arg.getCall() - ) - or - FlowSummaryImpl::Private::Steps::summaryLocalStep(src, sink, false) -} - -/** - * Holds if the additional step from `src` to `sink` should be included in all - * global taint flow configurations. - */ -predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { - localAdditionalTaintStep(src, sink) or - any(AdditionalTaintStep a).step(src, sink) -} - -/** - * Holds if `node` should be a sanitizer in all global taint flow configurations - * but not in local taint. - */ -predicate defaultTaintSanitizer(DataFlow::Node node) { - // Ignore paths through test code. - node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass or - node.asExpr() instanceof ValidatedVariableAccess -} +import Cached /** * Holds if taint can flow in one local step from `src` to `sink` excluding