From 187a1bba023a0428170256d3f8bbca27a75a856f Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Tue, 25 Jun 2024 10:25:12 +0200 Subject: [PATCH] wip --- .../lib/semmle/code/csharp/dataflow/SSA.qll | 24 +++- .../dataflow/internal/DataFlowDispatch.qll | 104 +++++++++++--- .../dataflow/internal/DataFlowPrivate.qll | 134 +++++++++++------- .../dataflow/internal/DataFlowPublic.qll | 4 +- .../ql/lib/semmle/code/csharp/exprs/Call.qll | 2 +- .../CWE-112/MissingXMLValidation.ql | 17 +++ .../dataflow/global/DataFlowPath.expected | 7 +- .../global/TaintTrackingPath.expected | 7 +- .../codeql/dataflow/VariableCapture.qll | 14 +- 9 files changed, 221 insertions(+), 92 deletions(-) diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll index 3dd8c89e73a16..d9a0e84a0421a 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll @@ -10,6 +10,21 @@ import csharp module Ssa { private import internal.SsaImpl as SsaImpl + pragma[nomagic] + private predicate assignableDefinitionLocalVariable( + AssignableDefinition ad, LocalScopeVariable v, Callable c + ) { + ad.getTarget() = v and + ad.getEnclosingCallable() = c + } + + private predicate localSourceVariable( + SourceVariables::LocalScopeSourceVariable sv, LocalScopeVariable v, Callable c + ) { + sv.getAssignable() = v and + sv.getEnclosingCallable() = c + } + /** * A variable that can be SSA converted. * @@ -34,11 +49,10 @@ module Ssa { or // Local variable declaration without initializer not exists(result.getTargetAccess()) and - this = - any(SourceVariables::LocalScopeSourceVariable v | - result.getTarget() = v.getAssignable() and - result.getEnclosingCallable() = v.getEnclosingCallable() - ) + exists(LocalScopeVariable v, Callable c | + assignableDefinitionLocalVariable(result, v, c) and + localSourceVariable(this, v, c) + ) } /** 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..687cd9d8c87c8 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll @@ -33,7 +33,10 @@ private module Cached { */ cached newtype TDataFlowCallable = - TCallable(Callable c) { c.isUnboundDeclaration() } or + TCallable(Callable c, Location l) { + c.isUnboundDeclaration() and + l = [c.getLocation(), c.getALocation().(SourceLocation)] + } or TSummarizedCallable(FlowSummary::SummarizedCallable sc) or TFieldOrPropertyCallable(FieldOrProperty f) or TCapturedVariableCallable(LocalScopeVariable v) { v.isCaptured() } @@ -88,7 +91,8 @@ private module DispatchImpl { */ DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) { exists(DispatchCall dc | dc = call.(NonDelegateDataFlowCall).getDispatchCall() | - result.getUnderlyingCallable() = + result.asCallable(_) = + // todo getCallableForDataFlow(dc.getADynamicTargetInCallContext(ctx.(NonDelegateDataFlowCall) .getDispatchCall()).getUnboundDeclaration()) or @@ -156,10 +160,21 @@ class RefReturnKind extends OutRefReturnKind, TRefReturnKind { override string toString() { result = "ref parameter " + pos } } +bindingset[e1, e2] +pragma[inline_late] +private predicate inSameFile(Location e1, Location e2) { e1.getFile() = e2.getFile() } + /** 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) } + + ControlFlow::BasicBlock getABasicBlock() { + exists(Location l | + result.getCallable() = this.asCallable(l) and + inSameFile(result.getLocation(), l) + ) + } /** Gets the underlying summarized callable, if any. */ FlowSummary::SummarizedCallable asSummarizedCallable() { this = TSummarizedCallable(result) } @@ -171,7 +186,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 +200,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 +273,25 @@ abstract class DataFlowCall extends TDataFlowCall { } } +private predicate relevantFolder(Folder f) { + exists(NonDelegateDataFlowCall call, Location l | f = l.getFile().getParentContainer() | + l = call.getLocation() + or + exists(call.getARuntimeTargetCandidate(l, _)) + ) +} + +private predicate parent(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, parent/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 +302,57 @@ class NonDelegateDataFlowCall extends DataFlowCall, TNonDelegateCall { /** Gets the underlying call. */ DispatchCall getDispatchCall() { result = dc } - override DataFlowCallable getARuntimeTarget() { - result.asCallable() = getCallableForDataFlow(dc.getADynamicTarget()) - or + pragma[nomagic] + DataFlowCallable getARuntimeTargetCandidate(Location l, Callable c) { + c = result.asCallable(l) and + c = getCallableForDataFlow(dc.getADynamicTarget()) + } + + pragma[nomagic] + private DataFlowCallable getARuntimeTargetCandidateAtDist(int dist, Callable c) { + exists(Location l | result = this.getARuntimeTargetCandidate(l, c) | + inSameFile(l, this.getLocation()) and + dist = -1 + or + folderDist(l.getFile().getParentContainer(), + this.getLocation().getFile().getParentContainer(), dist) + ) + } + + 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] + override DataFlowCallable getARuntimeTarget() { + exists(Callable c | + result = + min(DataFlowCallable cand, int dist | + cand = this.getARuntimeTargetCandidateAtDist(dist, c) + | + cand order by dist + ) + ) + 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 +364,7 @@ class NonDelegateDataFlowCall extends DataFlowCall, TNonDelegateCall { override DataFlow::ExprNode getNode() { result.getControlFlowNode() = cfn } - override DataFlowCallable getEnclosingCallable() { - result.asCallable() = cfn.getEnclosingCallable() - } + override DataFlowCallable getEnclosingCallable() { result.getABasicBlock() = cfn.getBasicBlock() } override string toString() { result = cfn.toString() } @@ -326,9 +392,7 @@ class ExplicitDelegateLikeDataFlowCall extends DelegateDataFlowCall, TExplicitDe override DataFlow::ExprNode getNode() { result.getControlFlowNode() = cfn } - override DataFlowCallable getEnclosingCallable() { - result.asCallable() = cfn.getEnclosingCallable() - } + override DataFlowCallable getEnclosingCallable() { result.getABasicBlock() = cfn.getBasicBlock() } 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 672e30b5b577a..ccfed1fea71d7 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -94,7 +94,7 @@ private DataFlowCallable getEnclosingStaticFieldOrProperty(Expr e) { private class ExprNodeImpl extends ExprNode, NodeImpl { override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = this.getControlFlowNodeImpl().getEnclosingCallable() + result.getABasicBlock() = this.getControlFlowNodeImpl().getBasicBlock() or result = getEnclosingStaticFieldOrProperty(this.asExpr()) } @@ -144,7 +144,7 @@ abstract private class LocalFunctionCreationNode extends NodeImpl, TLocalFunctio } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cfn.getEnclosingCallable() + result.getABasicBlock() = cfn.getBasicBlock() } override Type getTypeImpl() { none() } @@ -173,8 +173,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 +185,16 @@ private module ThisFlow { bb.getCallable() = p.getCallable() and i = p.getPosition() + 1 ) + or + exists(Location l, ControlFlow::BasicBlocks::EntryBlock entry | + n.(InstanceParameterNode).getCallable(l) = entry.getCallable() 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`. + bb = entry.getFirstNode().getASuccessor().getBasicBlock() and + inSameFile(l, bb.getLocation()) and + i = -1 + ) } private predicate thisRank(Node n, BasicBlock bb, int rankix) { @@ -339,7 +347,7 @@ module VariableCapture { private class CapturedThisParameter extends CapturedParameter, TCapturedThis { override InstanceParameterNode getParameterNode() { - result = TInstanceParameterNode(this.asThis()) + result = TInstanceParameterNode(this.asThis(), _) } } @@ -772,11 +780,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().getBasicBlock() = c.getABasicBlock() and + node2.asExpr() = e + | + e instanceof ThisAccess or e instanceof BaseAccess ) or hasNodePath(any(LocalExprStepConfiguration x), node1, node2) and @@ -1064,16 +1073,24 @@ private Gvn::GvnType getANonTypeParameterSubTypeRestricted(RelevantGvnType t) { result = getANonTypeParameterSubType(t) } +bindingset[e1, e2] +pragma[inline_late] +private predicate inSameFile(Location e1, Location e2) { e1.getFile() = e2.getFile() } + /** 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. */ @@ -1102,10 +1119,14 @@ private module Cached { TAssignableDefinitionNode(AssignableDefinition def, ControlFlow::Node cfn) { cfn = def.getExpr().getAControlFlowNode() } or - TExplicitParameterNode(Parameter p) { - p = any(DataFlowCallable dfc).asCallable().getAParameter() + TExplicitParameterNode(Parameter p, Location l) { + exists(Location cloc | + p = any(DataFlowCallable dfc).asCallable(cloc).getAParameter() and + l = p.getALocation() and + inSameFile(l, cloc) + ) } or - TInstanceParameterNode(InstanceCallable c) 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 +1171,9 @@ 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, Location l) { + p.getCallable() instanceof PrimaryConstructor and + l = [p.getLocation(), p.getALocation().(SourceLocation)] } or TCaptureNode(VariableCapture::Flow::SynthesizedCaptureNode cn) @@ -1244,7 +1266,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 +1303,7 @@ class SsaDefinitionExtNode extends NodeImpl, TSsaDefinitionExtNode { SsaImpl::DefinitionExt getDefinitionExt() { result = def } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = def.getEnclosingCallable() + result.getABasicBlock() = def.getBasicBlock() } override Type getTypeImpl() { result = def.getSourceVariable().getType() } @@ -1312,7 +1334,7 @@ class AssignableDefinitionNodeImpl extends NodeImpl, TAssignableDefinitionNode { } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cfn_.getEnclosingCallable() + result.getABasicBlock() = cfn_.getBasicBlock() } override Type getTypeImpl() { result = def.getTarget().getType() } @@ -1343,25 +1365,30 @@ private module ParameterNodes { */ class ExplicitParameterNode extends ParameterNodeImpl, TExplicitParameterNode { private Parameter parameter; + private Location location; - ExplicitParameterNode() { this = TExplicitParameterNode(parameter) } + ExplicitParameterNode() { this = TExplicitParameterNode(parameter, location) } /** Gets the SSA definition corresponding to this parameter, if any. */ - Ssa::ImplicitParameterDefinition getSsaDefinition() { result.getParameter() = parameter } + Ssa::ImplicitParameterDefinition getSsaDefinition() { + result.getSourceVariable().getAssignable() = parameter and + inSameFile(result.getLocation(), location) + } override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { - c.asCallable().getParameter(pos.getPosition()) = parameter + exists(Location cloc | + c.asCallable(cloc).getParameter(pos.getPosition()) = parameter and + inSameFile(location, cloc) + ) } - 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() { this = TExplicitParameterNode(_, result) } override string toStringImpl() { result = parameter.toString() } } @@ -1369,23 +1396,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 +1433,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 +1552,7 @@ private module ArgumentNodes { override ControlFlow::Node getControlFlowNodeImpl() { result = cfn } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cfn.getEnclosingCallable() + result.getABasicBlock() = cfn.getBasicBlock() or result = getEnclosingStaticFieldOrProperty(cfn.getAstNode()) } @@ -1565,7 +1593,7 @@ private module ArgumentNodes { } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = callCfn.getEnclosingCallable() + result.getABasicBlock() = callCfn.getBasicBlock() or result = getEnclosingStaticFieldOrProperty(callCfn.getAstNode()) } @@ -1653,7 +1681,7 @@ private module ReturnNodes { override NormalReturnKind getKind() { any() } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = yrs.getEnclosingCallable() + result.getABasicBlock() = cfn.getBasicBlock() } override Type getTypeImpl() { result = yrs.getEnclosingCallable().getReturnType() } @@ -1679,7 +1707,7 @@ private module ReturnNodes { override NormalReturnKind getKind() { any() } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = expr.getEnclosingCallable() + result.getABasicBlock() = cfn.getBasicBlock() } override Type getTypeImpl() { result = expr.getEnclosingCallable().getReturnType() } @@ -1866,7 +1894,7 @@ abstract private class InstanceParameterAccessNode extends NodeImpl, TInstancePa } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cfn.getEnclosingCallable() + result.getABasicBlock() = cfn.getBasicBlock() } override Type getTypeImpl() { result = cfn.getEnclosingCallable().getDeclaringType() } @@ -1910,16 +1938,24 @@ abstract private class PrimaryConstructorThisAccessNode extends NodeImpl, { Parameter p; boolean isPostUpdate; + Location location; - PrimaryConstructorThisAccessNode() { this = TPrimaryConstructorThisAccessNode(p, isPostUpdate) } + PrimaryConstructorThisAccessNode() { + this = TPrimaryConstructorThisAccessNode(p, isPostUpdate, location) + } - override DataFlowCallable getEnclosingCallableImpl() { result.asCallable() = p.getCallable() } + override DataFlowCallable getEnclosingCallableImpl() { + exists(Location cloc | + result.asCallable(cloc) = p.getCallable() and + inSameFile(cloc, location) + ) + } override Type getTypeImpl() { result = p.getCallable().getDeclaringType() } override ControlFlow::Nodes::ElementNode getControlFlowNodeImpl() { none() } - override Location getLocationImpl() { result = p.getLocation() } + override Location getLocationImpl() { result = location } override string toStringImpl() { result = "this" } @@ -1952,7 +1988,7 @@ class CaptureNode extends NodeImpl, TCaptureNode { VariableCapture::Flow::SynthesizedCaptureNode getSynthesizedCaptureNode() { result = cn } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cn.getEnclosingCallable() + result.getABasicBlock() = cn.getBasicBlock() } override Type getTypeImpl() { @@ -2186,9 +2222,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, Location l | + node1 = TExplicitParameterNode(p, l) and + node2 = TPrimaryConstructorThisAccessNode(p, true, l) and if p.getCallable().getDeclaringType() instanceof RecordType then c.(PropertyContent).getProperty().getName() = p.getName() else c.(PrimaryConstructorParameterContent).getParameter() = p @@ -2607,7 +2643,7 @@ module PostUpdateNodes { } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cfn.getEnclosingCallable() + result.getABasicBlock() = cfn.getBasicBlock() or result = getEnclosingStaticFieldOrProperty(oc) } @@ -2640,7 +2676,7 @@ module PostUpdateNodes { } override DataFlowCallable getEnclosingCallableImpl() { - result.asCallable() = cfn.getEnclosingCallable() + result.getABasicBlock() = cfn.getBasicBlock() or result = getEnclosingStaticFieldOrProperty(cfn.getAstNode()) } @@ -2683,7 +2719,7 @@ module PostUpdateNodes { PrimaryConstructorThisAccessPostUpdateNode() { isPostUpdate = true } override PrimaryConstructorThisAccessPreNode getPreUpdateNode() { - result = TPrimaryConstructorThisAccessNode(p, false) + result = TPrimaryConstructorThisAccessNode(p, false, location) } override string toStringImpl() { result = "[post] this" } @@ -2778,7 +2814,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/Security Features/CWE-112/MissingXMLValidation.ql b/csharp/ql/src/Security Features/CWE-112/MissingXMLValidation.ql index 42e9aa4c1f333..6758dd39b3ee6 100644 --- a/csharp/ql/src/Security Features/CWE-112/MissingXMLValidation.ql +++ b/csharp/ql/src/Security Features/CWE-112/MissingXMLValidation.ql @@ -15,6 +15,23 @@ import csharp import semmle.code.csharp.security.dataflow.MissingXMLValidationQuery import MissingXmlValidation::PathGraph +predicate stageStats = MissingXmlValidation::stageStats/10; + +predicate results(string s, int i) { + s = "edges" and + i = + strictcount(MissingXmlValidation::PathNode n1, MissingXmlValidation::PathNode n2 | + edges(n1, n2, _, _) + ) + or + s = "subpaths" and + i = + strictcount(MissingXmlValidation::PathNode n1, MissingXmlValidation::PathNode n2, + MissingXmlValidation::PathNode n3, MissingXmlValidation::PathNode n4 | + subpaths(n1, n2, n3, n4) + ) +} + from MissingXmlValidation::PathNode source, MissingXmlValidation::PathNode sink where MissingXmlValidation::flowPath(source, sink) select sink.getNode(), source, sink, 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 9fd385d445872..597af10665d76 100644 --- a/shared/dataflow/codeql/dataflow/VariableCapture.qll +++ b/shared/dataflow/codeql/dataflow/VariableCapture.qll @@ -791,18 +791,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