diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll index aa3fad909c5d7..8c15a3279e278 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll @@ -8,6 +8,7 @@ private import semmle.code.csharp.dispatch.Dispatch private import semmle.code.csharp.dispatch.RuntimeCallable private import semmle.code.csharp.frameworks.system.Collections private import semmle.code.csharp.frameworks.system.collections.Generic +private import semmle.code.csharp.internal.Location /** * Gets a source declaration of callable `c` that has a body and is @@ -24,6 +25,21 @@ newtype TReturnKind = TOutReturnKind(int i) { i = any(Parameter p | p.isOut()).getPosition() } or TRefReturnKind(int i) { i = any(Parameter p | p.isRef()).getPosition() } +private predicate hasMultipleSourceLocations(Callable c) { strictcount(getASourceLocation(c)) > 1 } + +private module NearestBodyLocationInput implements NearestLocationInputSig { + class C = ControlFlowElement; + + predicate relevantLocations(ControlFlowElement body, Location l1, Location l2) { + exists(Callable c | + hasMultipleSourceLocations(c) and + l1 = getASourceLocation(c) and + body = c.getBody() and + l2 = body.getLocation() + ) + } +} + cached private module Cached { /** @@ -33,7 +49,18 @@ private module Cached { */ cached newtype TDataFlowCallable = - TCallable(Callable c) { c.isUnboundDeclaration() } or + TCallable(Callable c, Location l) { + c.isUnboundDeclaration() and + l = [c.getLocation(), getASourceLocation(c)] and + ( + not hasMultipleSourceLocations(c) + or + // when `c` has multiple source locations, only use those with a body; + // for example, `partial` methods may have multiple source locations but + // we are only interested in the one with a body + NearestLocation::nearestLocation(_, l, _) + ) + } or TSummarizedCallable(FlowSummary::SummarizedCallable sc) or TFieldOrPropertyCallable(FieldOrProperty f) or TCapturedVariableCallable(LocalScopeVariable v) { v.isCaptured() } @@ -82,17 +109,23 @@ private module DispatchImpl { */ predicate mayBenefitFromCallContext(DataFlowCall call) { mayBenefitFromCallContext(call, _) } + bindingset[dc, result] + pragma[inline_late] + private Callable viableImplInCallContext0(DispatchCall dc, NonDelegateDataFlowCall ctx) { + result = dc.getADynamicTargetInCallContext(ctx.getDispatchCall()).getUnboundDeclaration() + } + /** * Gets a viable dispatch target of `call` in the context `ctx`. This is * restricted to those `call`s for which a context might make a difference. */ DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) { - exists(DispatchCall dc | dc = call.(NonDelegateDataFlowCall).getDispatchCall() | - result.getUnderlyingCallable() = - getCallableForDataFlow(dc.getADynamicTargetInCallContext(ctx.(NonDelegateDataFlowCall) - .getDispatchCall()).getUnboundDeclaration()) + exists(DispatchCall dc, Callable c | dc = call.(NonDelegateDataFlowCall).getDispatchCall() | + result = call.getARuntimeTarget() and + getCallableForDataFlow(c) = result.asCallable(_) and + c = viableImplInCallContext0(dc, ctx) or - exists(Callable c, DataFlowCallable encl | + exists(DataFlowCallable encl | result.asSummarizedCallable() = c and mayBenefitFromCallContext(call, encl) and encl = ctx.getARuntimeTarget() and @@ -159,7 +192,51 @@ class RefReturnKind extends OutRefReturnKind, TRefReturnKind { /** A callable used for data flow. */ class DataFlowCallable extends TDataFlowCallable { /** Gets the underlying source code callable, if any. */ - Callable asCallable() { this = TCallable(result) } + Callable asCallable(Location l) { this = TCallable(result, l) } + + /** Holds if this callable is multi-bodied. */ + pragma[nomagic] + predicate isMultiBodied() { hasMultipleSourceLocations(this.asCallable(_)) } + + private ControlFlow::Nodes::ElementNode getAMultiBodyEntryNode() { + exists(ControlFlowElement body, Location l | + body = this.asCallable(l).getBody() and + NearestLocation::nearestLocation(body, l, _) and + result = body.getAControlFlowEntryNode() + ) + } + + pragma[nomagic] + private ControlFlow::Nodes::ElementNode getAMultiBodyControlFlowNodePred() { + result = this.getAMultiBodyEntryNode().getAPredecessor() + or + result = this.getAMultiBodyControlFlowNodePred().getAPredecessor() + } + + pragma[nomagic] + private ControlFlow::Nodes::ElementNode getAMultiBodyControlFlowNodeSucc() { + result = this.getAMultiBodyEntryNode().getASuccessor() + or + result = this.getAMultiBodyControlFlowNodeSucc().getASuccessor() + } + + pragma[nomagic] + private ControlFlow::Nodes::ElementNode getAMultiBodyControlFlowNode() { + result = + [ + this.getAMultiBodyEntryNode(), this.getAMultiBodyControlFlowNodePred(), + this.getAMultiBodyControlFlowNodeSucc() + ] + } + + /** Gets a control flow node belonging to this callable. */ + pragma[inline] + ControlFlow::Node getAControlFlowNode() { + result = this.getAMultiBodyControlFlowNode() + or + not this.isMultiBodied() and + result.getEnclosingCallable() = this.asCallable(_) + } /** Gets the underlying summarized callable, if any. */ FlowSummary::SummarizedCallable asSummarizedCallable() { this = TSummarizedCallable(result) } @@ -171,7 +248,7 @@ class DataFlowCallable extends TDataFlowCallable { /** Gets the underlying callable. */ Callable getUnderlyingCallable() { - result = this.asCallable() or result = this.asSummarizedCallable() + result = this.asCallable(_) or result = this.asSummarizedCallable() } /** Gets a textual representation of this dataflow callable. */ @@ -185,7 +262,9 @@ class DataFlowCallable extends TDataFlowCallable { /** Get the location of this dataflow callable. */ Location getLocation() { - result = this.getUnderlyingCallable().getLocation() + this = TCallable(_, result) + or + result = this.asSummarizedCallable().getLocation() or result = this.asFieldOrProperty().getLocation() or @@ -256,6 +335,26 @@ abstract class DataFlowCall extends TDataFlowCall { } } +private predicate relevantFolder(Folder f) { + exists(NonDelegateDataFlowCall call, Location l | f = l.getFile().getParentContainer() | + l = call.getLocation() and + call.getARuntimeTargetCandidate(_, _).isMultiBodied() + or + call.getARuntimeTargetCandidate(l, _).isMultiBodied() + ) +} + +private predicate adjacentFolders(Folder f1, Folder f2) { + f1 = f2.getParentContainer() + or + f2 = f1.getParentContainer() +} + +bindingset[f1, f2] +pragma[inline_late] +private predicate folderDist(Folder f1, Folder f2, int i) = + shortestDistances(relevantFolder/1, adjacentFolders/2)(f1, f2, i) + /** A non-delegate C# call relevant for data flow. */ class NonDelegateDataFlowCall extends DataFlowCall, TNonDelegateCall { private ControlFlow::Nodes::ElementNode cfn; @@ -266,25 +365,63 @@ class NonDelegateDataFlowCall extends DataFlowCall, TNonDelegateCall { /** Gets the underlying call. */ DispatchCall getDispatchCall() { result = dc } - override DataFlowCallable getARuntimeTarget() { - result.asCallable() = getCallableForDataFlow(dc.getADynamicTarget()) - or + pragma[nomagic] + private predicate hasSourceTarget() { dc.getADynamicTarget().fromSource() } + + pragma[nomagic] + private FlowSummary::SummarizedCallable getASummarizedCallableTarget() { // Only use summarized callables with generated summaries in case // we are not able to dispatch to a source declaration. - exists(FlowSummary::SummarizedCallable sc, boolean static | - result.asSummarizedCallable() = sc and - sc = this.getATarget(static) and + exists(boolean static | + result = this.getATarget(static) and not ( - sc.applyGeneratedModel() and - dc.getADynamicTarget().getUnboundDeclaration().getFile().fromSource() + result.applyGeneratedModel() and + this.hasSourceTarget() ) | static = false or - static = true and not sc instanceof RuntimeCallable + static = true and not result instanceof RuntimeCallable ) } + pragma[nomagic] + DataFlowCallable getARuntimeTargetCandidate(Location l, Callable c) { + c = result.asCallable(l) and + c = getCallableForDataFlow(dc.getADynamicTarget()) + } + + pragma[nomagic] + private DataFlowCallable getAMultiBodiedRuntimeTargetCandidate(Callable c, int distance) { + result.isMultiBodied() and + exists(Location l | result = this.getARuntimeTargetCandidate(l, c) | + inSameFile(l, this.getLocation()) and + distance = -1 + or + folderDist(l.getFile().getParentContainer(), + this.getLocation().getFile().getParentContainer(), distance) + ) + } + + pragma[nomagic] + override DataFlowCallable getARuntimeTarget() { + // For calls to multi-bodied methods, we restrict the viable targets to those + // that are closest to the callsite, measured by file-system distance. + exists(Callable c | + result = + min(DataFlowCallable cand, int distance | + cand = this.getAMultiBodiedRuntimeTargetCandidate(c, distance) + | + cand order by distance + ) + ) + or + result = this.getARuntimeTargetCandidate(_, _) and + not result.isMultiBodied() + or + result.asSummarizedCallable() = this.getASummarizedCallableTarget() + } + /** Gets a static or dynamic target of this call. */ Callable getATarget(boolean static) { result = dc.getADynamicTarget().getUnboundDeclaration() and static = false @@ -296,9 +433,7 @@ class NonDelegateDataFlowCall extends DataFlowCall, TNonDelegateCall { override DataFlow::ExprNode getNode() { result.getControlFlowNode() = cfn } - override DataFlowCallable getEnclosingCallable() { - result.asCallable() = cfn.getEnclosingCallable() - } + override DataFlowCallable getEnclosingCallable() { result.getAControlFlowNode() = cfn } override string toString() { result = cfn.toString() } @@ -326,9 +461,7 @@ class ExplicitDelegateLikeDataFlowCall extends DelegateDataFlowCall, TExplicitDe override DataFlow::ExprNode getNode() { result.getControlFlowNode() = cfn } - override DataFlowCallable getEnclosingCallable() { - result.asCallable() = cfn.getEnclosingCallable() - } + override DataFlowCallable getEnclosingCallable() { result.getAControlFlowNode() = cfn } override string toString() { result = cfn.toString() } 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 b32fb5ffb38df..040ce64467fc8 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -17,6 +17,7 @@ private import semmle.code.csharp.frameworks.NHibernate private import semmle.code.csharp.frameworks.Razor private import semmle.code.csharp.frameworks.system.Collections private import semmle.code.csharp.frameworks.system.threading.Tasks +private import semmle.code.csharp.internal.Location private import codeql.util.Unit private import codeql.util.Boolean @@ -94,7 +95,7 @@ private DataFlowCallable getEnclosingStaticFieldOrProperty(Expr e) { private class ExprNodeImpl extends ExprNode, NodeImpl { override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = this.getControlFlowNodeImpl().getEnclosingCallable() + result.getAControlFlowNode() = this.getControlFlowNodeImpl() or result = getEnclosingStaticFieldOrProperty(this.asExpr()) } @@ -143,9 +144,7 @@ abstract private class LocalFunctionCreationNode extends NodeImpl, TLocalFunctio else inSameCallable = false } - override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cfn.getEnclosingCallable() - } + override DataFlowCallable getEnclosingCallableImpl() { result.getAControlFlowNode() = cfn } override Type getTypeImpl() { none() } @@ -173,8 +172,6 @@ private module ThisFlow { /** Holds if `n` is a `this` access at control flow node `cfn`. */ private predicate thisAccess(Node n, ControlFlow::Node cfn) { - n.(InstanceParameterNode).getCallable() = cfn.(ControlFlow::Nodes::EntryNode).getCallable() - or thisAccessExpr(n.asExprAtNode(cfn)) or cfn = n.(InstanceParameterAccessPreNode).getUnderlyingControlFlowNode() @@ -187,6 +184,20 @@ private module ThisFlow { bb.getCallable() = p.getCallable() and i = p.getPosition() + 1 ) + or + exists(DataFlowCallable c, ControlFlow::BasicBlocks::EntryBlock entry | + n.(InstanceParameterNode).isParameterOf(c, _) and + exists(ControlFlow::Node succ | + succ = c.getAControlFlowNode() and + succ = entry.getFirstNode().getASuccessor() and + // In case `c` has multiple bodies, we want each body to gets its own implicit + // entry definition. In case `c` doesn't have multiple bodies, the line below + // is simply the same as `bb = entry`, because `entry.getFirstNode().getASuccessor()` + // will be in the entry block. + bb = succ.getBasicBlock() and + i = -1 + ) + ) } private predicate thisRank(Node n, BasicBlock bb, int rankix) { @@ -339,7 +350,7 @@ module VariableCapture { private class CapturedThisParameter extends CapturedParameter, TCapturedThis { override InstanceParameterNode getParameterNode() { - result = TInstanceParameterNode(this.asThis()) + result = TInstanceParameterNode(this.asThis(), _) } } @@ -772,11 +783,12 @@ module LocalFlow { * Holds if the value of `node2` is given by `node1`. */ predicate localMustFlowStep(Node node1, Node node2) { - exists(Callable c, Expr e | - node1.(InstanceParameterNode).getCallable() = c and - node2.asExpr() = e and - (e instanceof ThisAccess or e instanceof BaseAccess) and - c = e.getEnclosingCallable() + exists(DataFlowCallable c, Expr e | + node1.(InstanceParameterNode).getEnclosingCallableImpl() = c and + node2.getControlFlowNode() = c.getAControlFlowNode() and + node2.asExpr() = e + | + e instanceof ThisAccess or e instanceof BaseAccess ) or hasNodePath(any(LocalExprStepConfiguration x), node1, node2) and @@ -1066,14 +1078,18 @@ private Gvn::GvnType getANonTypeParameterSubTypeRestricted(RelevantGvnType t) { /** A callable with an implicit `this` parameter. */ private class InstanceCallable extends Callable { + private Location l; + InstanceCallable() { - this = any(DataFlowCallable dfc).asCallable() and + this = any(DataFlowCallable dfc).asCallable(l) and not this.(Modifiable).isStatic() and // local functions and delegate capture `this` and should therefore // not have a `this` parameter not this instanceof LocalFunction and not this instanceof AnonymousFunctionExpr } + + Location getARelevantLocation() { result = l } } /** A collection of cached types and predicates to be evaluated in the same stage. */ @@ -1097,15 +1113,14 @@ private module Cached { TExprNode(ControlFlow::Nodes::ElementNode cfn) { cfn.getAstNode() instanceof Expr } or TSsaDefinitionExtNode(SsaImpl::DefinitionExt def) { // Handled by `TExplicitParameterNode` below - not def instanceof Ssa::ImplicitParameterDefinition + not def instanceof Ssa::ImplicitParameterDefinition and + def.getBasicBlock() = any(DataFlowCallable c).getAControlFlowNode().getBasicBlock() } or TAssignableDefinitionNode(AssignableDefinition def, ControlFlow::Node cfn) { cfn = def.getExpr().getAControlFlowNode() } or - TExplicitParameterNode(Parameter p) { - p = any(DataFlowCallable dfc).asCallable().getAParameter() - } or - TInstanceParameterNode(InstanceCallable c) or + TExplicitParameterNode(Parameter p, DataFlowCallable c) { p = c.asCallable(_).getAParameter() } or + TInstanceParameterNode(InstanceCallable c, Location l) { l = c.getARelevantLocation() } or TDelegateSelfReferenceNode(Callable c) { lambdaCreationExpr(_, c) } or TLocalFunctionCreationNode(ControlFlow::Nodes::ElementNode cfn, Boolean isPostUpdate) { cfn.getAstNode() instanceof LocalFunctionStmt @@ -1150,8 +1165,8 @@ private module Cached { TInstanceParameterAccessNode(ControlFlow::Node cfn, Boolean isPostUpdate) { cfn = getAPrimaryConstructorParameterCfn(_) } or - TPrimaryConstructorThisAccessNode(Parameter p, Boolean isPostUpdate) { - p.getCallable() instanceof PrimaryConstructor + TPrimaryConstructorThisAccessNode(Parameter p, Boolean isPostUpdate, DataFlowCallable c) { + p = c.asCallable(_).(PrimaryConstructor).getAParameter() } or TCaptureNode(VariableCapture::Flow::SynthesizedCaptureNode cn) @@ -1244,7 +1259,7 @@ predicate nodeIsHidden(Node n) { n = TInstanceParameterNode(any(Callable c | not c.fromSource() or c instanceof FlowSummary::SummarizedCallable - )) + ), _) or n instanceof YieldReturnNode or @@ -1281,7 +1296,7 @@ class SsaDefinitionExtNode extends NodeImpl, TSsaDefinitionExtNode { SsaImpl::DefinitionExt getDefinitionExt() { result = def } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = def.getEnclosingCallable() + result.getAControlFlowNode().getBasicBlock() = def.getBasicBlock() } override Type getTypeImpl() { result = def.getSourceVariable().getType() } @@ -1311,9 +1326,7 @@ class AssignableDefinitionNodeImpl extends NodeImpl, TAssignableDefinitionNode { cfn = cfn_ } - override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cfn_.getEnclosingCallable() - } + override DataFlowCallable getEnclosingCallableImpl() { result.getAControlFlowNode() = cfn_ } override Type getTypeImpl() { result = def.getTarget().getType() } @@ -1336,32 +1349,93 @@ abstract class ParameterNodeImpl extends NodeImpl { abstract predicate isParameterOf(DataFlowCallable c, ParameterPosition pos); } +private module NearestLocationInputParamAfterCallable implements NearestLocationInputSig { + class C = Parameter; + + predicate relevantLocations(Parameter p, Location l1, Location l2) { + exists(DataFlowCallable c | + c.asCallable(l1).getParameter(_) = p and + l2 = [p.getLocation(), getASourceLocation(p)] + ) + } +} + private module ParameterNodes { + pragma[nomagic] + private predicate ssaParamDef(Ssa::ImplicitParameterDefinition ssaDef, Parameter p, Location l) { + p = ssaDef.getParameter() and + l = ssaDef.getLocation() + } + + private module NearestLocationInputParamBeforeCallable implements NearestLocationInputSig { + class C = Parameter; + + predicate relevantLocations(Parameter p, Location l1, Location l2) { + exists(DataFlowCallable c | + c.asCallable(l2).getParameter(_) = p and + l1 = [p.getLocation(), getASourceLocation(p)] + ) + } + } + /** * The value of an explicit parameter at function entry, viewed as a node in a data * flow graph. */ class ExplicitParameterNode extends ParameterNodeImpl, TExplicitParameterNode { private Parameter parameter; + private DataFlowCallable callable; + + ExplicitParameterNode() { this = TExplicitParameterNode(parameter, callable) } - ExplicitParameterNode() { this = TExplicitParameterNode(parameter) } + Parameter getParameter() { result = parameter } + + pragma[nomagic] + private Location getNearestParameterLocation() { + exists(Location cloc | exists(callable.asCallable(cloc)) | + // typical scenario: parameter is syntactically after the callable + NearestLocation::nearestLocation(parameter, cloc, + result) + or + // atypical scenario: parameter is syntactically before the callable, for example + // `int this[int x] { get => x }`, where the parameter `x` is syntactically + // before the the callable `get_Item` + NearestLocation::nearestLocation(parameter, result, + cloc) + ) + } + + pragma[nomagic] + private Location getParameterLocation(Parameter p) { + p = parameter and + ( + result = this.getNearestParameterLocation() + or + not exists(this.getNearestParameterLocation()) and + result = parameter.getLocation() + ) + } /** Gets the SSA definition corresponding to this parameter, if any. */ - Ssa::ImplicitParameterDefinition getSsaDefinition() { result.getParameter() = parameter } + Ssa::ImplicitParameterDefinition getSsaDefinition() { + exists(Parameter p, Location l | + l = this.getParameterLocation(p) and + ssaParamDef(result, p, l) + ) + } override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { - c.asCallable().getParameter(pos.getPosition()) = parameter + c = callable and + c.asCallable(_).getParameter(pos.getPosition()) = parameter } - override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = parameter.getCallable() - } + override DataFlowCallable getEnclosingCallableImpl() { this.isParameterOf(result, _) } override Type getTypeImpl() { result = parameter.getType() } override ControlFlow::Node getControlFlowNodeImpl() { none() } - override Location getLocationImpl() { result = parameter.getLocation() } + override Location getLocationImpl() { result = this.getParameterLocation(_) } override string toStringImpl() { result = parameter.toString() } } @@ -1369,23 +1443,24 @@ private module ParameterNodes { /** An implicit instance (`this`) parameter. */ class InstanceParameterNode extends ParameterNodeImpl, TInstanceParameterNode { private Callable callable; + private Location location; - InstanceParameterNode() { this = TInstanceParameterNode(callable) } + InstanceParameterNode() { this = TInstanceParameterNode(callable, location) } /** Gets the callable containing this implicit instance parameter. */ - Callable getCallable() { result = callable } + Callable getCallable(Location loc) { result = callable and location = loc } override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { - callable = c.asCallable() and pos.isThisParameter() + callable = c.asCallable(location) and pos.isThisParameter() } - override DataFlowCallable getEnclosingCallableImpl() { result.asCallable() = callable } + override DataFlowCallable getEnclosingCallableImpl() { result.asCallable(location) = callable } override Type getTypeImpl() { result = callable.getDeclaringType() } override ControlFlow::Node getControlFlowNodeImpl() { none() } - override Location getLocationImpl() { result = callable.getLocation() } + override Location getLocationImpl() { result = location } override string toStringImpl() { result = "this" } } @@ -1405,12 +1480,12 @@ private module ParameterNodes { final Callable getCallable() { result = callable } override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { - callable = c.asCallable() and pos.isDelegateSelf() + callable = c.asCallable(_) and pos.isDelegateSelf() } override ControlFlow::Node getControlFlowNodeImpl() { none() } - override DataFlowCallable getEnclosingCallableImpl() { result.asCallable() = callable } + override DataFlowCallable getEnclosingCallableImpl() { result.asCallable(_) = callable } override Location getLocationImpl() { result = callable.getLocation() } @@ -1524,7 +1599,7 @@ private module ArgumentNodes { override ControlFlow::Node getControlFlowNodeImpl() { result = cfn } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cfn.getEnclosingCallable() + result.getAControlFlowNode() = cfn or result = getEnclosingStaticFieldOrProperty(cfn.getAstNode()) } @@ -1565,7 +1640,7 @@ private module ArgumentNodes { } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = callCfn.getEnclosingCallable() + result.getAControlFlowNode() = callCfn or result = getEnclosingStaticFieldOrProperty(callCfn.getAstNode()) } @@ -1652,9 +1727,7 @@ private module ReturnNodes { override NormalReturnKind getKind() { any() } - override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = yrs.getEnclosingCallable() - } + override DataFlowCallable getEnclosingCallableImpl() { result.getAControlFlowNode() = cfn } override Type getTypeImpl() { result = yrs.getEnclosingCallable().getReturnType() } @@ -1678,9 +1751,7 @@ private module ReturnNodes { override NormalReturnKind getKind() { any() } - override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = expr.getEnclosingCallable() - } + override DataFlowCallable getEnclosingCallableImpl() { result.getAControlFlowNode() = cfn } override Type getTypeImpl() { result = expr.getEnclosingCallable().getReturnType() } @@ -1865,9 +1936,7 @@ abstract private class InstanceParameterAccessNode extends NodeImpl, TInstancePa exists(ParameterAccess pa | cfn = getAPrimaryConstructorParameterCfn(pa) and pa.getTarget() = p) } - override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cfn.getEnclosingCallable() - } + override DataFlowCallable getEnclosingCallableImpl() { result.getAControlFlowNode() = cfn } override Type getTypeImpl() { result = cfn.getEnclosingCallable().getDeclaringType() } @@ -1910,16 +1979,22 @@ abstract private class PrimaryConstructorThisAccessNode extends NodeImpl, { Parameter p; boolean isPostUpdate; + DataFlowCallable callable; - PrimaryConstructorThisAccessNode() { this = TPrimaryConstructorThisAccessNode(p, isPostUpdate) } + PrimaryConstructorThisAccessNode() { + this = TPrimaryConstructorThisAccessNode(p, isPostUpdate, callable) + } - override DataFlowCallable getEnclosingCallableImpl() { result.asCallable() = p.getCallable() } + override DataFlowCallable getEnclosingCallableImpl() { result = callable } override Type getTypeImpl() { result = p.getCallable().getDeclaringType() } override ControlFlow::Nodes::ElementNode getControlFlowNodeImpl() { none() } - override Location getLocationImpl() { result = p.getLocation() } + override Location getLocationImpl() { + NearestLocation::nearestLocation(p, + callable.getLocation(), result) + } override string toStringImpl() { result = "this" } @@ -1952,7 +2027,7 @@ class CaptureNode extends NodeImpl, TCaptureNode { VariableCapture::Flow::SynthesizedCaptureNode getSynthesizedCaptureNode() { result = cn } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cn.getEnclosingCallable() + result.getAControlFlowNode().getBasicBlock() = cn.getBasicBlock() } override Type getTypeImpl() { @@ -2186,9 +2261,9 @@ predicate storeStep(Node node1, ContentSet c, Node node2) { or primaryConstructorParameterStore(node1, c, node2) or - exists(Parameter p | - node1 = TExplicitParameterNode(p) and - node2 = TPrimaryConstructorThisAccessNode(p, true) and + exists(Parameter p, DataFlowCallable callable | + node1 = TExplicitParameterNode(p, callable) and + node2 = TPrimaryConstructorThisAccessNode(p, true, callable) and if p.getCallable().getDeclaringType() instanceof RecordType then c.(PropertyContent).getProperty().getName() = p.getName() else c.(PrimaryConstructorParameterContent).getParameter() = p @@ -2603,7 +2678,7 @@ module PostUpdateNodes { } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cfn.getEnclosingCallable() + result.getAControlFlowNode() = cfn or result = getEnclosingStaticFieldOrProperty(oc) } @@ -2636,7 +2711,7 @@ module PostUpdateNodes { } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cfn.getEnclosingCallable() + result.getAControlFlowNode() = cfn or result = getEnclosingStaticFieldOrProperty(cfn.getAstNode()) } @@ -2679,7 +2754,7 @@ module PostUpdateNodes { PrimaryConstructorThisAccessPostUpdateNode() { isPostUpdate = true } override PrimaryConstructorThisAccessPreNode getPreUpdateNode() { - result = TPrimaryConstructorThisAccessNode(p, false) + result = TPrimaryConstructorThisAccessNode(p, false, callable) } override string toStringImpl() { result = "[post] this" } @@ -2774,7 +2849,7 @@ class LambdaCallKind = Unit; /** Holds if `creation` is an expression that creates a delegate for `c`. */ predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c) { - lambdaCreationExpr(creation.asExpr(), c.asCallable()) and + lambdaCreationExpr(creation.asExpr(), c.asCallable(_)) and exists(kind) } diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll index 20e3dd8eb7d00..4d1dc25f6e995 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll @@ -39,7 +39,7 @@ class Node extends TNode { /** Gets the enclosing callable of this node. */ final Callable getEnclosingCallable() { - result = this.(NodeImpl).getEnclosingCallableImpl().asCallable() + result = this.(NodeImpl).getEnclosingCallableImpl().asCallable(_) } /** Gets the control flow node corresponding to this node, if any. */ @@ -88,7 +88,7 @@ class ExprNode extends Node, TExprNode { pragma[nomagic] private predicate isParameterOf0(DataFlowCallable c, ParameterPosition ppos, Parameter p) { - p.getCallable() = c.asCallable() and + p.getCallable() = c.asCallable(_) and p.getPosition() = ppos.getPosition() } diff --git a/csharp/ql/lib/semmle/code/csharp/exprs/Call.qll b/csharp/ql/lib/semmle/code/csharp/exprs/Call.qll index 7a3cf0ea8d935..be4577d760eb1 100644 --- a/csharp/ql/lib/semmle/code/csharp/exprs/Call.qll +++ b/csharp/ql/lib/semmle/code/csharp/exprs/Call.qll @@ -528,7 +528,7 @@ class DelegateLikeCall extends Call, DelegateLikeCall_ { final override Callable getARuntimeTarget() { exists(ExplicitDelegateLikeDataFlowCall call | this = call.getCall() and - result = viableCallableLambda(call, _).asCallable() + result = viableCallableLambda(call, _).asCallable(_) ) } diff --git a/csharp/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll b/csharp/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll index 29df13ccbd79a..9d13666816658 100644 --- a/csharp/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll +++ b/csharp/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll @@ -238,7 +238,7 @@ string paramReturnNodeAsOutput(CS::Callable c, ParameterPosition pos) { * Gets the enclosing callable of `ret`. */ Callable returnNodeEnclosingCallable(DataFlow::Node ret) { - result = DataFlowImplCommon::getNodeEnclosingCallable(ret).asCallable() + result = DataFlowImplCommon::getNodeEnclosingCallable(ret).asCallable(_) } /** diff --git a/csharp/ql/test/library-tests/dataflow/global/DataFlowPath.expected b/csharp/ql/test/library-tests/dataflow/global/DataFlowPath.expected index 9b8f7aa390a82..096865964efa8 100644 --- a/csharp/ql/test/library-tests/dataflow/global/DataFlowPath.expected +++ b/csharp/ql/test/library-tests/dataflow/global/DataFlowPath.expected @@ -453,8 +453,8 @@ edges | GlobalDataFlow.cs:558:46:558:46 | access to local variable x : String | GlobalDataFlow.cs:558:44:558:47 | delegate call : String | provenance | | | MultiImplementationA.cs:5:28:5:41 | "taint source" : String | MultiImplementationA.cs:7:27:7:27 | x : String | provenance | | | MultiImplementationA.cs:7:27:7:27 | x : String | MultiImplementationA.cs:7:39:7:39 | access to parameter x | provenance | | -| MultiImplementationA.cs:7:27:7:27 | x : String | MultiImplementationB.cs:5:39:5:39 | access to parameter x | provenance | | -| MultiImplementationB.cs:3:28:3:41 | "taint source" : String | MultiImplementationA.cs:7:27:7:27 | x : String | provenance | | +| MultiImplementationB.cs:3:28:3:41 | "taint source" : String | MultiImplementationB.cs:5:27:5:27 | x : String | provenance | | +| MultiImplementationB.cs:5:27:5:27 | x : String | MultiImplementationB.cs:5:39:5:39 | access to parameter x | provenance | | | Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:8:24:8:30 | [b (line 3): false] access to parameter tainted : String | provenance | | | Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:8:24:8:30 | [b (line 3): true] access to parameter tainted : String | provenance | | | Splitting.cs:8:13:8:13 | access to local variable x : String | Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x | provenance | | @@ -912,6 +912,7 @@ nodes | MultiImplementationA.cs:7:27:7:27 | x : String | semmle.label | x : String | | MultiImplementationA.cs:7:39:7:39 | access to parameter x | semmle.label | access to parameter x | | MultiImplementationB.cs:3:28:3:41 | "taint source" : String | semmle.label | "taint source" : String | +| MultiImplementationB.cs:5:27:5:27 | x : String | semmle.label | x : String | | MultiImplementationB.cs:5:39:5:39 | access to parameter x | semmle.label | access to parameter x | | Splitting.cs:3:28:3:34 | tainted : String | semmle.label | tainted : String | | Splitting.cs:8:13:8:13 | access to local variable x : String | semmle.label | access to local variable x : String | @@ -1085,8 +1086,6 @@ subpaths | GlobalDataFlow.cs:323:15:323:24 | access to parameter sinkParam9 | GlobalDataFlow.cs:211:46:211:59 | "taint source" : String | GlobalDataFlow.cs:323:15:323:24 | access to parameter sinkParam9 | access to parameter sinkParam9 | | Capture.cs:273:30:273:30 | access to parameter x | Capture.cs:273:34:273:47 | "taint source" : String | Capture.cs:273:30:273:30 | access to parameter x | access to parameter x | | MultiImplementationA.cs:7:39:7:39 | access to parameter x | MultiImplementationA.cs:5:28:5:41 | "taint source" : String | MultiImplementationA.cs:7:39:7:39 | access to parameter x | access to parameter x | -| MultiImplementationA.cs:7:39:7:39 | access to parameter x | MultiImplementationB.cs:3:28:3:41 | "taint source" : String | MultiImplementationA.cs:7:39:7:39 | access to parameter x | access to parameter x | -| MultiImplementationB.cs:5:39:5:39 | access to parameter x | MultiImplementationA.cs:5:28:5:41 | "taint source" : String | MultiImplementationB.cs:5:39:5:39 | access to parameter x | access to parameter x | | MultiImplementationB.cs:5:39:5:39 | access to parameter x | MultiImplementationB.cs:3:28:3:41 | "taint source" : String | MultiImplementationB.cs:5:39:5:39 | access to parameter x | access to parameter x | | GlobalDataFlow.cs:27:15:27:32 | access to property SinkProperty0 | GlobalDataFlow.cs:18:27:18:40 | "taint source" : String | GlobalDataFlow.cs:27:15:27:32 | access to property SinkProperty0 | access to property SinkProperty0 | | Splitting.cs:21:21:21:33 | call to method Return | Splitting.cs:24:28:24:34 | tainted : String | Splitting.cs:21:21:21:33 | call to method Return | call to method Return | diff --git a/csharp/ql/test/library-tests/dataflow/global/TaintTrackingPath.expected b/csharp/ql/test/library-tests/dataflow/global/TaintTrackingPath.expected index 6a317ec46cf90..bb21ba40806b6 100644 --- a/csharp/ql/test/library-tests/dataflow/global/TaintTrackingPath.expected +++ b/csharp/ql/test/library-tests/dataflow/global/TaintTrackingPath.expected @@ -505,8 +505,8 @@ edges | GlobalDataFlowStringBuilder.cs:49:21:49:33 | call to method ToString : String | GlobalDataFlowStringBuilder.cs:49:13:49:17 | access to local variable sink3 : String | provenance | | | MultiImplementationA.cs:5:28:5:41 | "taint source" : String | MultiImplementationA.cs:7:27:7:27 | x : String | provenance | | | MultiImplementationA.cs:7:27:7:27 | x : String | MultiImplementationA.cs:7:39:7:39 | access to parameter x | provenance | | -| MultiImplementationA.cs:7:27:7:27 | x : String | MultiImplementationB.cs:5:39:5:39 | access to parameter x | provenance | | -| MultiImplementationB.cs:3:28:3:41 | "taint source" : String | MultiImplementationA.cs:7:27:7:27 | x : String | provenance | | +| MultiImplementationB.cs:3:28:3:41 | "taint source" : String | MultiImplementationB.cs:5:27:5:27 | x : String | provenance | | +| MultiImplementationB.cs:5:27:5:27 | x : String | MultiImplementationB.cs:5:39:5:39 | access to parameter x | provenance | | | Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:8:24:8:30 | [b (line 3): false] access to parameter tainted : String | provenance | | | Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:8:24:8:30 | [b (line 3): true] access to parameter tainted : String | provenance | | | Splitting.cs:8:13:8:13 | access to local variable x : String | Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x | provenance | | @@ -1019,6 +1019,7 @@ nodes | MultiImplementationA.cs:7:27:7:27 | x : String | semmle.label | x : String | | MultiImplementationA.cs:7:39:7:39 | access to parameter x | semmle.label | access to parameter x | | MultiImplementationB.cs:3:28:3:41 | "taint source" : String | semmle.label | "taint source" : String | +| MultiImplementationB.cs:5:27:5:27 | x : String | semmle.label | x : String | | MultiImplementationB.cs:5:39:5:39 | access to parameter x | semmle.label | access to parameter x | | Splitting.cs:3:28:3:34 | tainted : String | semmle.label | tainted : String | | Splitting.cs:8:13:8:13 | access to local variable x : String | semmle.label | access to local variable x : String | @@ -1196,8 +1197,6 @@ subpaths | GlobalDataFlowStringBuilder.cs:42:15:42:19 | access to local variable sink2 | GlobalDataFlowStringBuilder.cs:30:35:30:48 | "taint source" : String | GlobalDataFlowStringBuilder.cs:42:15:42:19 | access to local variable sink2 | access to local variable sink2 | | GlobalDataFlowStringBuilder.cs:50:15:50:19 | access to local variable sink3 | GlobalDataFlowStringBuilder.cs:48:47:48:60 | "taint source" : String | GlobalDataFlowStringBuilder.cs:50:15:50:19 | access to local variable sink3 | access to local variable sink3 | | MultiImplementationA.cs:7:39:7:39 | access to parameter x | MultiImplementationA.cs:5:28:5:41 | "taint source" : String | MultiImplementationA.cs:7:39:7:39 | access to parameter x | access to parameter x | -| MultiImplementationA.cs:7:39:7:39 | access to parameter x | MultiImplementationB.cs:3:28:3:41 | "taint source" : String | MultiImplementationA.cs:7:39:7:39 | access to parameter x | access to parameter x | -| MultiImplementationB.cs:5:39:5:39 | access to parameter x | MultiImplementationA.cs:5:28:5:41 | "taint source" : String | MultiImplementationB.cs:5:39:5:39 | access to parameter x | access to parameter x | | MultiImplementationB.cs:5:39:5:39 | access to parameter x | MultiImplementationB.cs:3:28:3:41 | "taint source" : String | MultiImplementationB.cs:5:39:5:39 | access to parameter x | access to parameter x | | Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x | Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x | [b (line 3): false] access to local variable x | | Splitting.cs:9:15:9:15 | [b (line 3): true] access to local variable x | Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:9:15:9:15 | [b (line 3): true] access to local variable x | [b (line 3): true] access to local variable x | diff --git a/shared/dataflow/codeql/dataflow/VariableCapture.qll b/shared/dataflow/codeql/dataflow/VariableCapture.qll index c48b46e8a7ba0..309809f5761ff 100644 --- a/shared/dataflow/codeql/dataflow/VariableCapture.qll +++ b/shared/dataflow/codeql/dataflow/VariableCapture.qll @@ -793,18 +793,18 @@ module Flow Input> implements OutputSig private class TSynthesizedCaptureNode = TSynthRead or TSynthThisQualifier or TSynthPhi; class SynthesizedCaptureNode extends ClosureNode, TSynthesizedCaptureNode { - Callable getEnclosingCallable() { - exists(BasicBlock bb | this = TSynthRead(_, bb, _, _) and result = bb.getEnclosingCallable()) + BasicBlock getBasicBlock() { + this = TSynthRead(_, result, _, _) or - exists(BasicBlock bb | - this = TSynthThisQualifier(bb, _, _) and result = bb.getEnclosingCallable() - ) + this = TSynthThisQualifier(result, _, _) or - exists(CaptureSsa::DefinitionExt phi, BasicBlock bb | - this = TSynthPhi(phi) and phi.definesAt(_, bb, _, _) and result = bb.getEnclosingCallable() + exists(CaptureSsa::DefinitionExt phi | + this = TSynthPhi(phi) and phi.definesAt(_, result, _, _) ) } + Callable getEnclosingCallable() { result = this.getBasicBlock().getEnclosingCallable() } + predicate isVariableAccess(CapturedVariable v) { this = TSynthRead(v, _, _, _) or