diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll index 626584efcbc..457cb21c361 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll index 626584efcbc..457cb21c361 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll index 626584efcbc..457cb21c361 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll index 626584efcbc..457cb21c361 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll index 494780d2e1b..13a8b3180a7 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll @@ -801,6 +801,9 @@ private module Cached { exists(Node n | getNodeEnclosingCallable(n) = callable | isUnreachableInCallCached(n, call)) } + cached + predicate allowFlowThroughParameterCached(Node ret) { allowFlowThroughParameter(ret) } + cached newtype TCallContext = TAnyCallContext() or diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll index a55e65a81f6..dd64fc70039 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll @@ -175,6 +175,7 @@ module Consistency { query predicate postWithInFlow(Node n, string msg) { isPostUpdateNode(n) and + not clearsContent(n, _) and simpleLocalFlowStep(_, n) and msg = "PostUpdateNode should not be the target of local flow." } diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll index 626584efcbc..457cb21c361 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll index 7767b644cbd..f68c00a72a5 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll @@ -287,3 +287,6 @@ predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) { no /** Extra data-flow steps needed for lambda flow analysis. */ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preservesValue) { none() } + +/** Holds if flow is allowed to pass into `p` and back out again. */ +predicate allowFlowThroughParameter(ParameterNode p) { none() } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll index 626584efcbc..457cb21c361 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll index 626584efcbc..457cb21c361 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll index 626584efcbc..457cb21c361 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll index 626584efcbc..457cb21c361 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll index 494780d2e1b..13a8b3180a7 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll @@ -801,6 +801,9 @@ private module Cached { exists(Node n | getNodeEnclosingCallable(n) = callable | isUnreachableInCallCached(n, call)) } + cached + predicate allowFlowThroughParameterCached(Node ret) { allowFlowThroughParameter(ret) } + cached newtype TCallContext = TAnyCallContext() or diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll index a55e65a81f6..dd64fc70039 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll @@ -175,6 +175,7 @@ module Consistency { query predicate postWithInFlow(Node n, string msg) { isPostUpdateNode(n) and + not clearsContent(n, _) and simpleLocalFlowStep(_, n) and msg = "PostUpdateNode should not be the target of local flow." } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index 00996a6ebfc..b7ecbaad6c6 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -507,3 +507,6 @@ predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) { no /** Extra data-flow steps needed for lambda flow analysis. */ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preservesValue) { none() } + +/** Holds if flow is allowed to pass into `p` and back out again. */ +predicate allowFlowThroughParameter(ParameterNode p) { none() } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll index 626584efcbc..457cb21c361 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll index 626584efcbc..457cb21c361 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll index 626584efcbc..457cb21c361 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll index 626584efcbc..457cb21c361 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl.qll index 626584efcbc..457cb21c361 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl2.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl2.qll index 626584efcbc..457cb21c361 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl2.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl2.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl3.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl3.qll index 626584efcbc..457cb21c361 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl3.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl3.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl4.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl4.qll index 626584efcbc..457cb21c361 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl4.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl4.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl5.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl5.qll index 626584efcbc..457cb21c361 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl5.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl5.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl6.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl6.qll index 626584efcbc..457cb21c361 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl6.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl6.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll index 494780d2e1b..13a8b3180a7 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll @@ -801,6 +801,9 @@ private module Cached { exists(Node n | getNodeEnclosingCallable(n) = callable | isUnreachableInCallCached(n, call)) } + cached + predicate allowFlowThroughParameterCached(Node ret) { allowFlowThroughParameter(ret) } + cached newtype TCallContext = TAnyCallContext() or diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll index a55e65a81f6..dd64fc70039 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll @@ -175,6 +175,7 @@ module Consistency { query predicate postWithInFlow(Node n, string msg) { isPostUpdateNode(n) and + not clearsContent(n, _) and simpleLocalFlowStep(_, n) and msg = "PostUpdateNode should not be the target of local flow." } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplForSerializability.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplForSerializability.qll index 626584efcbc..457cb21c361 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplForSerializability.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplForSerializability.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll index d35610ecf4a..89f81149640 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll @@ -369,3 +369,11 @@ predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) { /** Extra data-flow steps needed for lambda flow analysis. */ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preservesValue) { none() } + +/** + * Holds if flow is allowed to pass from a parameter `p`, to return + * node `ret`, and back out to `p`. + */ +predicate allowFlowThroughParameter(ReturnNodeExt ret) { + FlowSummaryImpl::Private::summaryAllowFlowThroughParameter(ret) +} diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll index 83076558ec4..8cc49ace88b 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll @@ -261,7 +261,10 @@ module Private { private newtype TSummaryNodeState = TSummaryNodeInputState(SummaryComponentStack s) { inputState(_, s) } or - TSummaryNodeOutputState(SummaryComponentStack s) { outputState(_, s) } + TSummaryNodeOutputState(SummaryComponentStack s) { outputState(_, s) } or + TSummaryNodeClearsContentState(int i, boolean post) { + any(SummarizedCallable sc).clearsContent(i, _) and post in [false, true] + } /** * A state used to break up (complex) flow summaries into atomic flow steps. @@ -308,6 +311,12 @@ module Private { this = TSummaryNodeOutputState(s) and result = "to write: " + s ) + or + exists(int i, boolean post, string postStr | + this = TSummaryNodeClearsContentState(i, post) and + (if post = true then postStr = " (post)" else postStr = "") and + result = "clear: " + i + postStr + ) } } @@ -329,6 +338,11 @@ module Private { not parameterReadState(c, state, _) or state.isOutputState(c, _) + or + exists(int i | + c.clearsContent(i, _) and + state = TSummaryNodeClearsContentState(i, _) + ) } pragma[noinline] @@ -364,6 +378,8 @@ module Private { parameterReadState(c, _, i) or isParameterPostUpdate(_, c, i) + or + c.clearsContent(i, _) } private predicate callbackOutput( @@ -436,6 +452,12 @@ module Private { ) ) ) + or + exists(SummarizedCallable c, int i, ParamNode p | + n = summaryNode(c, TSummaryNodeClearsContentState(i, false)) and + p.isParameterOf(c, i) and + result = getNodeType(p) + ) } /** Holds if summary node `out` contains output of kind `rk` from call `c`. */ @@ -461,6 +483,9 @@ module Private { exists(SummarizedCallable c, int i | isParameterPostUpdate(post, c, i) and pre.(ParamNode).isParameterOf(c, i) + or + pre = summaryNode(c, TSummaryNodeClearsContentState(i, false)) and + post = summaryNode(c, TSummaryNodeClearsContentState(i, true)) ) or exists(SummarizedCallable callable, SummaryComponentStack s | @@ -478,6 +503,14 @@ module Private { ) } + /** + * Holds if flow is allowed to pass from a parameter `p`, to return + * node `ret`, and back out to `p`. + */ + predicate summaryAllowFlowThroughParameter(ReturnNodeExt ret) { + ret = summaryNode(_, TSummaryNodeClearsContentState(_, true)) + } + /** Provides a compilation of flow summaries to atomic data-flow steps. */ module Steps { /** @@ -504,11 +537,21 @@ module Private { // for `StringBuilder.append(x)` with a specified value flow from qualifier to // return value and taint flow from argument 0 to the qualifier, then this // allows us to infer taint flow from argument 0 to the return value. - succ instanceof ParamNode and summaryPostUpdateNode(pred, succ) and preservesValue = true + succ instanceof ParamNode and + summaryPostUpdateNode(pred, succ) and + preservesValue = true or // Similarly we would like to chain together summaries where values get passed // into callbacks along the way. - pred instanceof ArgNode and summaryPostUpdateNode(succ, pred) and preservesValue = true + pred instanceof ArgNode and + summaryPostUpdateNode(succ, pred) and + preservesValue = true + or + exists(SummarizedCallable c, int i | + pred.(ParamNode).isParameterOf(c, i) and + succ = summaryNode(c, TSummaryNodeClearsContentState(i, _)) and + preservesValue = true + ) } /** @@ -536,10 +579,39 @@ module Private { } /** - * Holds if values stored inside content `c` are cleared when passed as - * input of type `input` in `call`. + * Holds if values stored inside content `c` are cleared at `n`. `n` is a + * synthesized summary node, so in order for values to be cleared at calls + * to the relevant method, it is important that flow does not pass over + * the argument, either via use-use flow or def-use flow. + * + * Example: + * + * ``` + * a.b = taint; + * a.clearB(); // assume we have a flow summary for `clearB` that clears `b` on the qualifier + * sink(a.b); + * ``` + * + * In the above, flow should not pass from `a` on the first line (or the second + * line) to `a` on the third line. Instead, there will be synthesized flow from + * `a` on line 2 to the post-update node for `a` on that line (via an intermediate + * node where field `b` is cleared). */ - predicate summaryClearsContent(ArgNode arg, Content c) { + predicate summaryClearsContent(Node n, Content c) { + exists(SummarizedCallable sc, int i | + n = summaryNode(sc, TSummaryNodeClearsContentState(i, true)) and + sc.clearsContent(i, c) + ) + } + + /** + * Holds if values stored inside content `c` are cleared inside a + * callable to which `arg` is an argument. + * + * In such cases, it is important to prevent use-use flow out of + * `arg` (see comment for `summaryClearsContent`). + */ + predicate summaryClearsContentArg(ArgNode arg, Content c) { exists(DataFlowCall call, int i | viableCallable(call).(SummarizedCallable).clearsContent(i, c) and arg.argumentOf(call, i) @@ -599,25 +671,6 @@ module Private { ret.getKind() = rk ) } - - /** - * Holds if data is written into content `c` of argument `arg` using a flow summary. - * - * Depending on the type of `c`, this predicate may be relevant to include in the - * definition of `clearsContent()`. - */ - predicate summaryStoresIntoArg(Content c, Node arg) { - exists(ParamUpdateReturnKind rk, ReturnNodeExt ret, PostUpdateNode out | - exists(DataFlowCall call, SummarizedCallable callable | - getNodeEnclosingCallable(ret) = callable and - viableCallable(call) = callable and - summaryStoreStep(_, c, ret) and - ret.getKind() = pragma[only_bind_into](rk) and - out = rk.getAnOutNode(call) and - arg = out.getPreUpdateNode() - ) - ) - } } /** diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl.qll index 626584efcbc..457cb21c361 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll index 626584efcbc..457cb21c361 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll index 626584efcbc..457cb21c361 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll index 626584efcbc..457cb21c361 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll @@ -252,6 +252,8 @@ private class RetNodeEx extends NodeEx { ReturnPosition getReturnPosition() { result = getReturnPosition(this.asNode()) } ReturnKindExt getKind() { result = this.asNode().(ReturnNodeExt).getKind() } + + predicate allowFlowThroughParameter() { allowFlowThroughParameterCached(this.asNode()) } } private predicate inBarrier(NodeEx node, Configuration config) { @@ -725,12 +727,16 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, Configuration config + DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, + Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() + kind = ret.getKind() and + if ret.allowFlowThroughParameter() + then allowFlowThroughParameter = true + else allowFlowThroughParameter = false ) } @@ -739,13 +745,16 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind | + exists(ReturnKindExt kind, boolean allowFlowThroughParameter | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, config) and + returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and p.getEnclosingCallable() = c and - exists(ap) and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then allowFlowThroughParameter = true + else any() + ) and + exists(ap) ) } @@ -1394,8 +1403,11 @@ private module Stage2 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2083,8 +2095,11 @@ private module Stage3 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2843,8 +2858,11 @@ private module Stage4 { fwdFlow(ret, any(CcCall ccc), apSome(ap), ap0, config) and kind = ret.getKind() and p.getPosition() = pos and - // we don't expect a parameter to return stored in itself - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + then ret.allowFlowThroughParameter() + else any() + ) ) } @@ -2917,6 +2935,8 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } + ParameterNode getParameterNode() { result = p.asNode() } + override string toString() { result = p + ": " + ap } predicate hasLocationInfo( @@ -3617,7 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and - not kind.(ParamUpdateReturnKind).getPosition() = pos + ( + if kind.(ParamUpdateReturnKind).getPosition() = pos + then ret.allowFlowThroughParameter() + else any() + ) ) } diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplCommon.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplCommon.qll index 494780d2e1b..13a8b3180a7 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplCommon.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplCommon.qll @@ -801,6 +801,9 @@ private module Cached { exists(Node n | getNodeEnclosingCallable(n) = callable | isUnreachableInCallCached(n, call)) } + cached + predicate allowFlowThroughParameterCached(Node ret) { allowFlowThroughParameter(ret) } + cached newtype TCallContext = TAnyCallContext() or diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll index a55e65a81f6..dd64fc70039 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll @@ -175,6 +175,7 @@ module Consistency { query predicate postWithInFlow(Node n, string msg) { isPostUpdateNode(n) and + not clearsContent(n, _) and simpleLocalFlowStep(_, n) and msg = "PostUpdateNode should not be the target of local flow." } diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll index b41edd28898..34c8cbc4287 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll @@ -1664,3 +1664,6 @@ predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) { no /** Extra data-flow steps needed for lambda flow analysis. */ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preservesValue) { none() } + +/** Holds if flow is allowed to pass into `p` and back out again. */ +predicate allowFlowThroughParameter(ParameterNode p) { none() }