Skip to content

Commit

Permalink
C#: Add read and store steps for delegate calls.
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelnebel committed Oct 24, 2024
1 parent 226756e commit 6d0fb03
Showing 4 changed files with 181 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@ private import semmle.code.csharp.Unification
private import semmle.code.csharp.controlflow.Guards
private import semmle.code.csharp.dispatch.Dispatch
private import semmle.code.csharp.frameworks.EntityFramework
private import semmle.code.csharp.frameworks.system.linq.Expressions
private import semmle.code.csharp.frameworks.NHibernate
private import semmle.code.csharp.frameworks.Razor
private import semmle.code.csharp.frameworks.system.Collections
@@ -1146,7 +1147,18 @@ private module Cached {
TPrimaryConstructorParameterContent(Parameter p) {
p.getCallable() instanceof PrimaryConstructor
} or
TCapturedVariableContent(VariableCapture::CapturedVariable v)
TCapturedVariableContent(VariableCapture::CapturedVariable v) or
TDelegateCallArgumentContent(Parameter p, int i) {
i =
[0 .. p.getType()
.getUnboundDeclaration()
.(SystemLinqExpressions::DelegateExtType)
.getDelegateType()
.getNumberOfParameters() - 1]
} or
TDelegateCallReturnContent(Parameter p) {
p.getType().getUnboundDeclaration() instanceof SystemLinqExpressions::DelegateExtType
}

cached
newtype TContentSet =
@@ -1162,7 +1174,13 @@ private module Cached {
TPrimaryConstructorParameterApproxContent(string firstChar) {
firstChar = approximatePrimaryConstructorParameterContent(_)
} or
TCapturedVariableContentApprox(VariableCapture::CapturedVariable v)
TCapturedVariableContentApprox(VariableCapture::CapturedVariable v) or
TDelegateCallArgumentApproxContent(string firstChar) {
firstChar = approximateDelegateCallArgumentContent(_)
} or
TDelegateCallReturnApproxContent(string firstChar) {
firstChar = approximateDelegateCallReturnContent(_)
}

pragma[nomagic]
private predicate commonSubTypeGeneral(DataFlowTypeOrUnifiable t1, RelevantGvnType t2) {
@@ -2273,6 +2291,22 @@ private predicate recordProperty(RecordType t, ContentSet c, string name) {
)
}

/**
* Holds if data can flow from `node1` to `node2` via an assignment to
* the content set `c` of a delegate call.
*
* If there is a delegate call f(x), then we store "x" on "f"
* using a delegate parameter content set.
*/
private predicate storeStepDelegateCall(Node node1, ContentSet c, Node node2) {
exists(DelegateCall call, Parameter p, int i |
node1.asExpr() = call.getArgument(i) and
node2.(PostUpdateNode).getPreUpdateNode().asExpr() = call.getExpr() and
call.getExpr() = p.getAnAccess() and
c.isDelegateCallArgument(p, i)
)
}

/**
* Holds if data can flow from `node1` to `node2` via an assignment to
* content `c`.
@@ -2305,6 +2339,8 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
or
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), c,
node2.(FlowSummaryNode).getSummaryNode())
or
storeStepDelegateCall(node1, c, node2)
}

private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration {
@@ -2425,6 +2461,22 @@ private predicate readContentStep(Node node1, Content c, Node node2) {
VariableCapture::readStep(node1, c, node2)
}

/**
* Holds if data can flow from `node1` to `node2` via an assignment to
* the content set `c` of a delegate call.
*
* If there is a delegate call f(x), then we read the result of the delegate
* call.
*/
private predicate readStepDelegateCall(Node node1, ContentSet c, Node node2) {
exists(DelegateCall call, Parameter p |
node1.asExpr() = call.getExpr() and
node2.asExpr() = call and
call.getExpr() = p.getAnAccess() and
c.isDelegateCallReturn(p)
)
}

/**
* Holds if data can flow from `node1` to `node2` via a read of content `c`.
*/
@@ -2443,6 +2495,8 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
or
FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
node2.(FlowSummaryNode).getSummaryNode())
or
readStepDelegateCall(node1, c, node2)
}

private predicate clearsCont(Node n, Content c) {
@@ -3037,6 +3091,16 @@ class ContentApprox extends TContentApprox {
exists(VariableCapture::CapturedVariable v |
this = TCapturedVariableContentApprox(v) and result = "captured " + v
)
or
exists(string firstChar |
this = TDelegateCallArgumentApproxContent(firstChar) and
result = "approximated delegate call argument " + firstChar
)
or
exists(string firstChar |
this = TDelegateCallReturnApproxContent(firstChar) and
result = "approximated delegate call return " + firstChar
)
}
}

@@ -3058,6 +3122,22 @@ private string approximatePrimaryConstructorParameterContent(PrimaryConstructorP
result = pc.getParameter().getName().prefix(1)
}

private string getApproximateParameterName(Parameter p) {
exists(string name | name = p.getName() |
name = "" and result = ""
or
result = name.prefix(1)
)
}

private string approximateDelegateCallArgumentContent(DelegateCallArgumentContent dc) {
result = getApproximateParameterName(dc.getParameter())
}

private string approximateDelegateCallReturnContent(DelegateCallReturnContent dc) {
result = getApproximateParameterName(dc.getParameter())
}

/** Gets an approximated value for content `c`. */
pragma[nomagic]
ContentApprox getContentApprox(Content c) {
@@ -3073,6 +3153,10 @@ ContentApprox getContentApprox(Content c) {
TPrimaryConstructorParameterApproxContent(approximatePrimaryConstructorParameterContent(c))
or
result = TCapturedVariableContentApprox(VariableCapture::getCapturedVariableContent(c))
or
result = TDelegateCallArgumentApproxContent(approximateDelegateCallArgumentContent(c))
or
result = TDelegateCallReturnApproxContent(approximateDelegateCallReturnContent(c))
}

/**
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@ private import DataFlowDispatch
private import DataFlowPrivate
private import semmle.code.csharp.controlflow.Guards
private import semmle.code.csharp.Unification
private import semmle.code.csharp.frameworks.system.linq.Expressions

/**
* An element, viewed as a node in a data flow graph. Either an expression
@@ -238,6 +239,61 @@ class PropertyContent extends Content, TPropertyContent {
override Location getLocation() { result = p.getLocation() }
}

/**
* A reference to the index of an argument of a delegate call
* (where the delegate is a parameter)
*/
class DelegateCallArgumentContent extends Content, TDelegateCallArgumentContent {
private Parameter p;
private int i;

DelegateCallArgumentContent() { this = TDelegateCallArgumentContent(p, i) }

/** Gets the underlying parameter. */
Parameter getParameter() { result = p }

/**
* Gets the type of the `i`th parameter of the underlying parameter `p`
* (which is of delegate type).
*/
Type getType() {
result =
p.getType()
.(SystemLinqExpressions::DelegateExtType)
.getDelegateType()
.getParameter(i)
.getType()
}

override string toString() { result = "delegate parameter " + p.getName() + " at position " + i }

override Location getLocation() { result = p.getLocation() }
}

/**
* A reference to the return of a delegate call
* (where the delegate is a parameter)
*/
class DelegateCallReturnContent extends Content, TDelegateCallReturnContent {
private Parameter p;

DelegateCallReturnContent() { this = TDelegateCallReturnContent(p) }

/** Gets the underlying parameter. */
Parameter getParameter() { result = p }

/**
* Gets the return type of the underlying parameter `p` (which is of delegate type).
*/
Type getType() {
result = p.getType().(SystemLinqExpressions::DelegateExtType).getDelegateType().getReturnType()
}

override string toString() { result = "delegate parameter " + p.getName() + " result" }

override Location getLocation() { result = p.getLocation() }
}

/**
* A reference to a synthetic field corresponding to a
* primary constructor parameter.
@@ -299,6 +355,20 @@ class ContentSet extends TContentSet {
*/
predicate isProperty(Property p) { this = TPropertyContentSet(p) }

/**
* Holds if this content set represents the `i`th argument of
* the parameter `p` of delegate type in a delegate call.
*/
predicate isDelegateCallArgument(Parameter p, int i) {
this.isSingleton(TDelegateCallArgumentContent(p, i))
}

/**
* Holds if this content set represents the return of a delegate call
* of parameter `p` (which is of delegate type).
*/
predicate isDelegateCallReturn(Parameter p) { this.isSingleton(TDelegateCallReturnContent(p)) }

/** Holds if this content set represents the field `f`. */
predicate isField(Field f) { this.isSingleton(TFieldContent(f)) }

12 changes: 10 additions & 2 deletions csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll
Original file line number Diff line number Diff line change
@@ -101,7 +101,9 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
api = any(FlowSummaryImpl::Public::NeutralSinkCallable sc | sc.hasManualModel())
}

predicate isUninterestingForDataFlowModels(Callable api) { isHigherOrder(api) }
predicate isUninterestingForDataFlowModels(Callable api) { none() }

predicate isUninterestingForHeuristicDataFlowModels(Callable api) { isHigherOrder(api) }

class SourceOrSinkTargetApi extends Callable {
SourceOrSinkTargetApi() { relevant(this) }
@@ -175,7 +177,9 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
*/
private CS::Type getUnderlyingContType(DataFlow::Content c) {
result = c.(DataFlow::FieldContent).getField().getType() or
result = c.(DataFlow::SyntheticFieldContent).getField().getType()
result = c.(DataFlow::SyntheticFieldContent).getField().getType() or
result = c.(DataFlow::DelegateCallArgumentContent).getType() or
result = c.(DataFlow::DelegateCallReturnContent).getType()
}

Type getUnderlyingContentType(DataFlow::ContentSet c) {
@@ -342,6 +346,10 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
or
c.isElement() and
result = "Element"
or
exists(int i | c.isDelegateCallArgument(_, i) and result = "Parameter[" + i + "]")
or
c.isDelegateCallReturn(_) and result = "ReturnValue"
}

predicate partialModel = ExternalFlow::partialModel/6;
Original file line number Diff line number Diff line change
@@ -223,6 +223,15 @@ signature module ModelGeneratorInputSig<LocationSig Location, InputSig<Location>
*/
predicate isUninterestingForDataFlowModels(Callable api);

/**
* Holds if it is irrelevant to generate models for `api` based on the heuristic
* (non-content) flow analysis.
*
* This serves as an extra filter for the `relevant`
* and `isUninterestingForDataFlowModels` predicates.
*/
predicate isUninterestingForHeuristicDataFlowModels(Callable api);

/**
* Holds if `namespace`, `type`, `extensible`, `name` and `parameters` are string representations
* for the corresponding MaD columns for `api`.
@@ -300,7 +309,7 @@ module MakeModelGenerator<
}
}

string getOutput(ReturnNodeExt node) {
private string getOutput(ReturnNodeExt node) {
result = PrintReturnNodeExt<paramReturnNodeAsOutput/2>::getOutput(node)
}

@@ -432,7 +441,11 @@ module MakeModelGenerator<

predicate isSource(DataFlow::Node source, FlowState state) {
source instanceof DataFlow::ParameterNode and
source.(NodeExtended).getEnclosingCallable() instanceof DataFlowSummaryTargetApi and
exists(Callable c |
c = source.(NodeExtended).getEnclosingCallable() and
c instanceof DataFlowSummaryTargetApi and
not isUninterestingForHeuristicDataFlowModels(c)
) and
state.(TaintRead).getStep() = 0
}

0 comments on commit 6d0fb03

Please sign in to comment.