From dd138b04294e7681b6b41b4bf951ff1df2baf67b Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Tue, 19 Oct 2021 15:33:25 +0200 Subject: [PATCH] Address review comments --- .../cpp/dataflow/internal/DataFlowImpl.qll | 57 ++++++++++--------- .../cpp/dataflow/internal/DataFlowImpl2.qll | 57 ++++++++++--------- .../cpp/dataflow/internal/DataFlowImpl3.qll | 57 ++++++++++--------- .../cpp/dataflow/internal/DataFlowImpl4.qll | 57 ++++++++++--------- .../dataflow/internal/DataFlowImplCommon.qll | 2 +- .../dataflow/internal/DataFlowImplLocal.qll | 57 ++++++++++--------- .../cpp/dataflow/internal/DataFlowPrivate.qll | 7 ++- .../cpp/ir/dataflow/internal/DataFlowImpl.qll | 57 ++++++++++--------- .../ir/dataflow/internal/DataFlowImpl2.qll | 57 ++++++++++--------- .../ir/dataflow/internal/DataFlowImpl3.qll | 57 ++++++++++--------- .../ir/dataflow/internal/DataFlowImpl4.qll | 57 ++++++++++--------- .../dataflow/internal/DataFlowImplCommon.qll | 2 +- .../ir/dataflow/internal/DataFlowPrivate.qll | 7 ++- .../csharp/dataflow/internal/DataFlowImpl.qll | 57 ++++++++++--------- .../dataflow/internal/DataFlowImpl2.qll | 57 ++++++++++--------- .../dataflow/internal/DataFlowImpl3.qll | 57 ++++++++++--------- .../dataflow/internal/DataFlowImpl4.qll | 57 ++++++++++--------- .../dataflow/internal/DataFlowImpl5.qll | 57 ++++++++++--------- .../dataflow/internal/DataFlowImplCommon.qll | 2 +- .../dataflow/internal/DataFlowPrivate.qll | 8 +-- .../dataflow/internal/FlowSummaryImpl.qll | 11 ++-- .../java/dataflow/internal/DataFlowImpl.qll | 57 ++++++++++--------- .../java/dataflow/internal/DataFlowImpl2.qll | 57 ++++++++++--------- .../java/dataflow/internal/DataFlowImpl3.qll | 57 ++++++++++--------- .../java/dataflow/internal/DataFlowImpl4.qll | 57 ++++++++++--------- .../java/dataflow/internal/DataFlowImpl5.qll | 57 ++++++++++--------- .../java/dataflow/internal/DataFlowImpl6.qll | 57 ++++++++++--------- .../dataflow/internal/DataFlowImplCommon.qll | 2 +- .../DataFlowImplForSerializability.qll | 57 ++++++++++--------- .../dataflow/internal/DataFlowPrivate.qll | 8 +-- .../dataflow/internal/FlowSummaryImpl.qll | 11 ++-- .../dataflow/new/internal/DataFlowImpl.qll | 57 ++++++++++--------- .../dataflow/new/internal/DataFlowImpl2.qll | 57 ++++++++++--------- .../dataflow/new/internal/DataFlowImpl3.qll | 57 ++++++++++--------- .../dataflow/new/internal/DataFlowImpl4.qll | 57 ++++++++++--------- .../new/internal/DataFlowImplCommon.qll | 2 +- .../dataflow/new/internal/DataFlowPrivate.qll | 7 ++- 37 files changed, 767 insertions(+), 727 deletions(-) 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 457cb21c361..b3d03ea4e26 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 13a8b3180a7..e11244c42b0 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll @@ -802,7 +802,7 @@ private module Cached { } cached - predicate allowFlowThroughParameterCached(Node ret) { allowFlowThroughParameter(ret) } + predicate allowParameterReturnInSelfCached(ParamNode p) { allowParameterReturnInSelf(p) } cached newtype TCallContext = 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 457cb21c361..b3d03ea4e26 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 f68c00a72a5..d970e31e129 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll @@ -288,5 +288,8 @@ 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() } +/** + * Holds if flow is allowed to pass from parameter `p`, to a return + * node, and back out to `p`. + */ +predicate allowParameterReturnInSelf(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 457cb21c361..b3d03ea4e26 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 @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 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 @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 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 @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 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 @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 13a8b3180a7..e11244c42b0 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 @@ -802,7 +802,7 @@ private module Cached { } cached - predicate allowFlowThroughParameterCached(Node ret) { allowFlowThroughParameter(ret) } + predicate allowParameterReturnInSelfCached(ParamNode p) { allowParameterReturnInSelf(p) } cached newtype TCallContext = 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 b7ecbaad6c6..6f6f5407b14 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 @@ -508,5 +508,8 @@ 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() } +/** + * Holds if flow is allowed to pass from parameter `p`, to a return + * node, and back out to `p`. + */ +predicate allowParameterReturnInSelf(ParameterNode p) { none() } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll index 457cb21c361..b3d03ea4e26 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll index 13a8b3180a7..e11244c42b0 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll @@ -802,7 +802,7 @@ private module Cached { } cached - predicate allowFlowThroughParameterCached(Node ret) { allowFlowThroughParameter(ret) } + predicate allowParameterReturnInSelfCached(ParamNode p) { allowParameterReturnInSelf(p) } cached newtype TCallContext = diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll index 3f50a203f22..ad190c127db 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -2015,9 +2015,9 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves } /** - * Holds if flow is allowed to pass from a parameter `p`, to return - * node `ret`, and back out to `p`. + * Holds if flow is allowed to pass from parameter `p`, to a return + * node, and back out to `p`. */ -predicate allowFlowThroughParameter(ReturnNodeExt ret) { - FlowSummaryImpl::Private::summaryAllowFlowThroughParameter(ret) +predicate allowParameterReturnInSelf(ParameterNode p) { + FlowSummaryImpl::Private::summaryAllowParameterReturnInSelf(p) } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll index 8cc49ace88b..5955285bd6f 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll @@ -504,11 +504,14 @@ module Private { } /** - * Holds if flow is allowed to pass from a parameter `p`, to return - * node `ret`, and back out to `p`. + * Holds if flow is allowed to pass from parameter `p`, to a return + * node, and back out to `p`. */ - predicate summaryAllowFlowThroughParameter(ReturnNodeExt ret) { - ret = summaryNode(_, TSummaryNodeClearsContentState(_, true)) + predicate summaryAllowParameterReturnInSelf(ParamNode p) { + exists(SummarizedCallable c, int i | + c.clearsContent(i, _) and + p.isParameterOf(c, i) + ) } /** Provides a compilation of flow summaries to atomic data-flow steps. */ 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 457cb21c361..b3d03ea4e26 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl2.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl2.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl3.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl3.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl4.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl4.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl5.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl5.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl6.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImpl6.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 13a8b3180a7..e11244c42b0 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll @@ -802,7 +802,7 @@ private module Cached { } cached - predicate allowFlowThroughParameterCached(Node ret) { allowFlowThroughParameter(ret) } + predicate allowParameterReturnInSelfCached(ParamNode p) { allowParameterReturnInSelf(p) } cached newtype TCallContext = 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 457cb21c361..b3d03ea4e26 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplForSerializability.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplForSerializability.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 387cab61704..8cac4d44443 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll @@ -367,9 +367,9 @@ predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) { 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`. + * Holds if flow is allowed to pass from parameter `p`, to a return + * node, and back out to `p`. */ -predicate allowFlowThroughParameter(ReturnNodeExt ret) { - FlowSummaryImpl::Private::summaryAllowFlowThroughParameter(ret) +predicate allowParameterReturnInSelf(ParameterNode p) { + FlowSummaryImpl::Private::summaryAllowParameterReturnInSelf(p) } 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 8cc49ace88b..5955285bd6f 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll @@ -504,11 +504,14 @@ module Private { } /** - * Holds if flow is allowed to pass from a parameter `p`, to return - * node `ret`, and back out to `p`. + * Holds if flow is allowed to pass from parameter `p`, to a return + * node, and back out to `p`. */ - predicate summaryAllowFlowThroughParameter(ReturnNodeExt ret) { - ret = summaryNode(_, TSummaryNodeClearsContentState(_, true)) + predicate summaryAllowParameterReturnInSelf(ParamNode p) { + exists(SummarizedCallable c, int i | + c.clearsContent(i, _) and + p.isParameterOf(c, i) + ) } /** Provides a compilation of flow summaries to atomic data-flow steps. */ 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 457cb21c361..b3d03ea4e26 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 457cb21c361..b3d03ea4e26 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll @@ -244,6 +244,8 @@ private class ParamNodeEx extends NodeEx { } int getPosition() { this.isParameterOf(_, result) } + + predicate allowParameterReturnInSelf() { allowParameterReturnInSelfCached(this.asNode()) } } private class RetNodeEx extends NodeEx { @@ -252,8 +254,6 @@ 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) { @@ -727,16 +727,12 @@ private module Stage1 { /** Holds if flow may return from `callable`. */ pragma[nomagic] private predicate returnFlowCallableNodeCand( - DataFlowCallable callable, ReturnKindExt kind, boolean allowFlowThroughParameter, - Configuration config + DataFlowCallable callable, ReturnKindExt kind, Configuration config ) { exists(RetNodeEx ret | throughFlowNodeCand(ret, config) and callable = ret.getEnclosingCallable() and - kind = ret.getKind() and - if ret.allowFlowThroughParameter() - then allowFlowThroughParameter = true - else allowFlowThroughParameter = false + kind = ret.getKind() ) } @@ -745,16 +741,17 @@ private module Stage1 { * candidate for the origin of a summary. */ predicate parameterMayFlowThrough(ParamNodeEx p, DataFlowCallable c, Ap ap, Configuration config) { - exists(ReturnKindExt kind, boolean allowFlowThroughParameter | + exists(ReturnKindExt kind | throughFlowNodeCand(p, config) and - returnFlowCallableNodeCand(c, kind, allowFlowThroughParameter, config) and + returnFlowCallableNodeCand(c, kind, config) and p.getEnclosingCallable() = c and + exists(ap) and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then allowFlowThroughParameter = true - else any() - ) and - exists(ap) + not kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() + or + p.allowParameterReturnInSelf() + ) ) } @@ -1403,10 +1400,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2095,10 +2093,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2858,10 +2857,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, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = p.getPosition() - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + p.allowParameterReturnInSelf() ) ) } @@ -2935,7 +2935,7 @@ private class SummaryCtxSome extends SummaryCtx, TSummaryCtxSome { int getParameterPos() { p.isParameterOf(_, result) } - ParameterNode getParameterNode() { result = p.asNode() } + ParamNodeEx getParamNode() { result = p } override string toString() { result = p + ": " + ap } @@ -3637,10 +3637,11 @@ private predicate paramFlowsThrough( ap = mid.getAp() and apa = ap.getApprox() and pos = sc.getParameterPos() and + // we don't expect a parameter to return stored in itself, unless explicitly allowed ( - if kind.(ParamUpdateReturnKind).getPosition() = pos - then ret.allowFlowThroughParameter() - else any() + not kind.(ParamUpdateReturnKind).getPosition() = pos + or + sc.getParamNode().allowParameterReturnInSelf() ) ) } 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 13a8b3180a7..e11244c42b0 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplCommon.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplCommon.qll @@ -802,7 +802,7 @@ private module Cached { } cached - predicate allowFlowThroughParameterCached(Node ret) { allowFlowThroughParameter(ret) } + predicate allowParameterReturnInSelfCached(ParamNode p) { allowParameterReturnInSelf(p) } cached newtype TCallContext = 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 34c8cbc4287..76edf182968 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll @@ -1665,5 +1665,8 @@ 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() } +/** + * Holds if flow is allowed to pass from parameter `p`, to a return + * node, and back out to `p`. + */ +predicate allowParameterReturnInSelf(ParameterNode p) { none() }