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 c1c851d18fd..b77bb173135 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -546,7 +546,11 @@ module LocalFlow { ) or hasNodePath(any(LocalExprStepConfiguration x), node1, node2) and - (node2 instanceof SsaDefinitionExtNode or node2.asExpr() instanceof Cast) + ( + node2 instanceof SsaDefinitionExtNode or + node2.asExpr() instanceof Cast or + node2.asExpr() instanceof AssignExpr + ) or exists(SsaImpl::Definition def | def = getSsaDefinitionExt(node1) and diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll index 7d9ef3e2404..3ea0df26059 100644 --- a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll @@ -1316,7 +1316,7 @@ module MakeImpl { ) or // flow into a callable - fwdFlowIn(_, node, state, _, cc, _, _, _, t, ap, apa) and + fwdFlowIn(_, _, node, state, _, cc, _, _, _, t, ap, apa, _) and if PrevStage::parameterMayFlowThrough(node, apa) then ( summaryCtx = TParamNodeSome(node.asNode()) and @@ -1476,15 +1476,14 @@ module MakeImpl { pragma[nomagic] private predicate fwdFlowIn( - DataFlowCall call, ParamNodeEx p, FlowState state, Cc outercc, CcCall innercc, - ParamNodeOption summaryCtx, TypOption argT, ApOption argAp, Typ t, Ap ap, ApApprox apa + DataFlowCall call, DataFlowCallable inner, ParamNodeEx p, FlowState state, Cc outercc, + CcCall innercc, ParamNodeOption summaryCtx, TypOption argT, ApOption argAp, Typ t, Ap ap, + ApApprox apa, boolean cc ) { - exists(DataFlowCallable inner, boolean cc | - fwdFlowInCand(call, inner, p, state, outercc, summaryCtx, argT, argAp, t, ap, apa) and - FwdTypeFlow::typeFlowValidEdgeIn(call, inner, cc) and - innercc = getCallContextCall(call, inner) and - if outercc instanceof CcCall then cc = true else cc = false - ) + fwdFlowInCand(call, inner, p, state, outercc, summaryCtx, argT, argAp, t, ap, apa) and + FwdTypeFlow::typeFlowValidEdgeIn(call, inner, cc) and + innercc = getCallContextCall(call, inner) and + if outercc instanceof CcCall then cc = true else cc = false } bindingset[ctx, result] @@ -1535,6 +1534,7 @@ module MakeImpl { ) } + pragma[nomagic] private predicate fwdFlowOut( DataFlowCall call, DataFlowCallable inner, NodeEx out, FlowState state, CcNoCall outercc, ParamNodeOption summaryCtx, TypOption argT, ApOption argAp, Typ t, Ap ap, ApApprox apa @@ -1557,11 +1557,9 @@ module MakeImpl { pragma[nomagic] predicate dataFlowTakenCallEdgeIn(DataFlowCall call, DataFlowCallable c, boolean cc) { - exists(ParamNodeEx p, Cc outercc, FlowState state, Cc innercc, Typ t, Ap ap | - fwdFlowIn(call, p, state, outercc, innercc, _, _, _, t, ap, _) and - fwdFlow1(p, state, innercc, _, _, _, t, _, ap, _) and - c = p.getEnclosingCallable() and - if outercc instanceof CcCall then cc = true else cc = false + exists(ParamNodeEx p, FlowState state, Cc innercc, Typ t, Ap ap | + fwdFlowIn(call, c, p, state, _, innercc, _, _, _, t, ap, _, cc) and + fwdFlow1(p, state, innercc, _, _, _, t, _, ap, _) ) } @@ -1647,8 +1645,8 @@ module MakeImpl { ApOption argAp, ParamNodeEx p, Typ t, Ap ap ) { exists(ApApprox apa | - fwdFlowIn(call, pragma[only_bind_into](p), _, cc, innerCc, summaryCtx, argT, argAp, t, - ap, pragma[only_bind_into](apa)) and + fwdFlowIn(call, _, pragma[only_bind_into](p), _, cc, innerCc, summaryCtx, argT, argAp, + t, ap, pragma[only_bind_into](apa), _) and PrevStage::parameterMayFlowThrough(p, apa) and PrevStage::callMayFlowThroughRev(call) ) @@ -1802,7 +1800,7 @@ module MakeImpl { or // flow out of a callable exists(ReturnPosition pos | - revFlowOut(_, node, pos, state, _, _, ap) and + revFlowOut(_, node, pos, state, _, _, _, ap) and if returnFlowsThrough(node, pos, state, _, _, _, _, ap) then ( returnCtx = TReturnCtxMaybeFlowThrough(pos) and @@ -1867,10 +1865,9 @@ module MakeImpl { pragma[nomagic] predicate dataFlowTakenCallEdgeIn(DataFlowCall call, DataFlowCallable c, boolean cc) { - exists(RetNodeEx ret, ReturnCtx returnCtx | - revFlowOut(call, ret, _, _, returnCtx, _, _) and - c = ret.getEnclosingCallable() and - if returnCtx instanceof TReturnCtxNone then cc = false else cc = true + exists(RetNodeEx ret | + revFlowOut(call, ret, _, _, _, cc, _, _) and + c = ret.getEnclosingCallable() ) } @@ -1925,9 +1922,9 @@ module MakeImpl { pragma[nomagic] private predicate revFlowOut( DataFlowCall call, RetNodeEx ret, ReturnPosition pos, FlowState state, - ReturnCtx returnCtx, ApOption returnAp, Ap ap + ReturnCtx returnCtx, boolean cc, ApOption returnAp, Ap ap ) { - exists(NodeEx out, boolean cc | + exists(NodeEx out | revFlow(out, state, returnCtx, returnAp, ap) and flowOutOfCallApValid(call, ret, pos, out, ap, cc) and if returnCtx instanceof TReturnCtxNone then cc = false else cc = true @@ -1963,7 +1960,7 @@ module MakeImpl { DataFlowCall call, ReturnCtx returnCtx, ApOption returnAp, ReturnPosition pos, Ap ap ) { exists(RetNodeEx ret, FlowState state, CcCall ccc | - revFlowOut(call, ret, pos, state, returnCtx, returnAp, ap) and + revFlowOut(call, ret, pos, state, returnCtx, _, returnAp, ap) and returnFlowsThrough(ret, pos, state, ccc, _, _, _, ap) and matchesCall(ccc, call) ) diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll index 14508eb89ee..62cfc3051a4 100644 --- a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll @@ -1002,7 +1002,7 @@ module MakeImplCommon { * flow between parameter and argument nodes in the cases where it is possible * for a type to first be weakened and then strengthened again. When the * stronger types at the end-points of such a type flow path are incompatible, - * the call relevant call edges can be excluded as impossible. + * the relevant call edges can be excluded as impossible. * * The predicates `relevantCallEdgeIn` and `relevantCallEdgeOut` give the * graph to be explored prior to the recursion, and the other three predicates @@ -1021,7 +1021,7 @@ module MakeImplCommon { } /** - * Holds if a sequence calls may propagates the value of `p` to some + * Holds if a sequence of calls may propagate the value of `p` to some * argument-to-parameter call edge that strengthens the static type. */ pragma[nomagic] @@ -1033,7 +1033,7 @@ module MakeImplCommon { } /** - * Holds if a sequence calls may propagates the value of `arg` to some + * Holds if a sequence of calls may propagate the value of `arg` to some * argument-to-parameter call edge that strengthens the static type. */ pragma[nomagic] @@ -1048,6 +1048,9 @@ module MakeImplCommon { ) or exists(ParamNode p, DataFlowType at, DataFlowType pt | + // A call edge may implicitly strengthen a type by ensuring that a + // specific argument node was reached if the type of that argument was + // strengthened via a cast. at = getNodeType(arg) and pt = getNodeType(p) and paramMustFlow(p, arg) and @@ -1072,6 +1075,10 @@ module MakeImplCommon { DataFlowCall call1, DataFlowCallable c1, ArgNode argOut, DataFlowCall call2, DataFlowCallable c2, ArgNode argIn | + // Data flow may exit `call1` and enter `call2`. If a stronger type is + // known for `argOut`, `argIn` may reach a strengthening, and both are + // determined by the same parameter `p` so we know they're equal, then + // we should track those nodes. trackedParamTypeCand(p) and callEdge(call1, c1, argOut, _) and Input::relevantCallEdgeOut(call1, c1) and @@ -1152,6 +1159,15 @@ module MakeImplCommon { paramMustFlow(_, arg) } + /** + * Gets the strongest of the two types `t1` and `t2`. If neither type is + * stronger then compatibility is checked and `t1` is returned. + */ + bindingset[t1, t2] + DataFlowType getStrongestType(DataFlowType t1, DataFlowType t2) { + if typeStrongerThan(t2, t1) then result = t2 else (compatibleTypes(t1, t2) and result = t1) + } + /** * Holds if `t` is a possible type for an argument reaching the tracked * parameter `p` through an in-going edge in the current data flow stage. @@ -1179,7 +1195,7 @@ module MakeImplCommon { cc = true and typeFlowParamTypeCand(p, t1) and nodeDataFlowType(p, t2) and - if typeStrongerThan(t2, t1) then t = t2 else (compatibleTypes(t1, t2) and t = t1) + t = getStrongestType(t1, t2) ) or exists(ArgNode arg, DataFlowType t1, DataFlowType t2 | @@ -1187,7 +1203,7 @@ module MakeImplCommon { typeFlowArgTypeFromReturn(arg, t1) and paramMustFlow(p, arg) and nodeDataFlowType(p, t2) and - if typeStrongerThan(t2, t1) then t = t2 else (compatibleTypes(t1, t2) and t = t1) + t = getStrongestType(t1, t2) ) or exists(DataFlowCall call | @@ -1208,7 +1224,7 @@ module MakeImplCommon { dataFlowTakenCallEdgeOut(_, _, arg, p) and (if trackedParamType(p) then typeFlowParamType(p, t1, false) else nodeDataFlowType(p, t1)) and nodeDataFlowType(arg, t2) and - if typeStrongerThan(t2, t1) then t = t2 else (compatibleTypes(t1, t2) and t = t1) + t = getStrongestType(t1, t2) ) } @@ -1224,7 +1240,7 @@ module MakeImplCommon { paramMustFlow(p, arg) and typeFlowParamType(p, t1, cc) and nodeDataFlowType(arg, t2) and - if typeStrongerThan(t2, t1) then t = t2 else (compatibleTypes(t1, t2) and t = t1) + t = getStrongestType(t1, t2) ) or cc = [true, false] and