Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,16 @@ newtype TReturnKind =
}

/**
* Holds if the summary for `c` should be used for dataflow analysis.
* A summarized callable where the summary should be used for dataflow analysis.
*/
predicate useFlowSummary(FlowSummary::SummarizedCallable c) {
not c.fromSource()
or
c.fromSource() and not c.isAutoGenerated()
class DataFlowSummarizedCallable instanceof FlowSummary::SummarizedCallable {
DataFlowSummarizedCallable() {
not this.fromSource()
or
this.fromSource() and not this.isAutoGenerated()
}

string toString() { result = super.toString() }
}

private module Cached {
Expand All @@ -101,8 +105,10 @@ private module Cached {
*/
cached
newtype TDataFlowCallable =
TDotNetCallable(DotNet::Callable c) { c.isUnboundDeclaration() and not useFlowSummary(c) } or
TSummarizedCallable(FlowSummary::SummarizedCallable c) { useFlowSummary(c) }
TDotNetCallable(DotNet::Callable c) {
c.isUnboundDeclaration() and not c instanceof DataFlowSummarizedCallable
} or
TSummarizedCallable(DataFlowSummarizedCallable sc)

cached
newtype TDataFlowCall =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,8 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
or
exists(Ssa::Definition def |
LocalFlow::localSsaFlowStepUseUse(def, nodeFrom, nodeTo) and
not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom) and
not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom,
any(DataFlowSummarizedCallable sc)) and
not LocalFlow::usesInstanceField(def)
)
or
Expand Down Expand Up @@ -739,15 +740,10 @@ private module Cached {
)
)
} or
TSummaryNode(
FlowSummaryImpl::Public::SummarizedCallable c,
FlowSummaryImpl::Private::SummaryNodeState state
) {
useFlowSummary(c) and
TSummaryNode(DataFlowSummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) {
FlowSummaryImpl::Private::summaryNodeRange(c, state)
} or
TSummaryParameterNode(FlowSummaryImpl::Public::SummarizedCallable c, ParameterPosition pos) {
useFlowSummary(c) and
TSummaryParameterNode(DataFlowSummarizedCallable c, ParameterPosition pos) {
FlowSummaryImpl::Private::summaryParameterNodeRange(c, pos)
} or
TParamsArgumentNode(ControlFlow::Node callCfn) {
Expand All @@ -771,7 +767,8 @@ private module Cached {
or
// Simple flow through library code is included in the exposed local
// step relation, even though flow is technically inter-procedural
FlowSummaryImpl::Private::Steps::summaryThroughStepValue(nodeFrom, nodeTo)
FlowSummaryImpl::Private::Steps::summaryThroughStepValue(nodeFrom, nodeTo,
any(DataFlowSummarizedCallable sc))
}

cached
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -759,8 +759,8 @@ module Private {
}

pragma[nomagic]
private ParamNode summaryArgParam0(DataFlowCall call, ArgNode arg) {
exists(ParameterPosition ppos, SummarizedCallable sc |
private ParamNode summaryArgParam0(DataFlowCall call, ArgNode arg, SummarizedCallable sc) {
exists(ParameterPosition ppos |
argumentPositionMatch(call, arg, ppos) and
viableParam(call, sc, ppos, result)
)
Expand All @@ -774,9 +774,9 @@ module Private {
* or expects contents.
*/
pragma[nomagic]
predicate prohibitsUseUseFlow(ArgNode arg) {
predicate prohibitsUseUseFlow(ArgNode arg, SummarizedCallable sc) {
exists(ParamNode p, Node mid, ParameterPosition ppos, Node ret |
p = summaryArgParam0(_, arg) and
p = summaryArgParam0(_, arg, sc) and
p.isParameterOf(_, ppos) and
summaryLocalStep(p, mid, true) and
summaryLocalStep(mid, ret, true) and
Expand All @@ -788,9 +788,11 @@ module Private {
}

bindingset[ret]
private ParamNode summaryArgParam(ArgNode arg, ReturnNodeExt ret, OutNodeExt out) {
private ParamNode summaryArgParam(
ArgNode arg, ReturnNodeExt ret, OutNodeExt out, SummarizedCallable sc
) {
exists(DataFlowCall call, ReturnKindExt rk |
result = summaryArgParam0(call, arg) and
result = summaryArgParam0(call, arg, sc) and
ret.getKind() = pragma[only_bind_into](rk) and
out = pragma[only_bind_into](rk).getAnOutNode(call)
)
Expand All @@ -803,9 +805,9 @@ module Private {
* NOTE: This step should not be used in global data-flow/taint-tracking, but may
* be useful to include in the exposed local data-flow/taint-tracking relations.
*/
predicate summaryThroughStepValue(ArgNode arg, Node out) {
predicate summaryThroughStepValue(ArgNode arg, Node out, SummarizedCallable sc) {
exists(ReturnKind rk, ReturnNode ret, DataFlowCall call |
summaryLocalStep(summaryArgParam0(call, arg), ret, true) and
summaryLocalStep(summaryArgParam0(call, arg, sc), ret, true) and
ret.getKind() = pragma[only_bind_into](rk) and
out = getAnOutNode(call, pragma[only_bind_into](rk))
)
Expand All @@ -818,8 +820,8 @@ module Private {
* NOTE: This step should not be used in global data-flow/taint-tracking, but may
* be useful to include in the exposed local data-flow/taint-tracking relations.
*/
predicate summaryThroughStepTaint(ArgNode arg, Node out) {
exists(ReturnNodeExt ret | summaryLocalStep(summaryArgParam(arg, ret, out), ret, false))
predicate summaryThroughStepTaint(ArgNode arg, Node out, SummarizedCallable sc) {
exists(ReturnNodeExt ret | summaryLocalStep(summaryArgParam(arg, ret, out, sc), ret, false))
}

/**
Expand All @@ -829,9 +831,9 @@ module Private {
* NOTE: This step should not be used in global data-flow/taint-tracking, but may
* be useful to include in the exposed local data-flow/taint-tracking relations.
*/
predicate summaryGetterStep(ArgNode arg, ContentSet c, Node out) {
predicate summaryGetterStep(ArgNode arg, ContentSet c, Node out, SummarizedCallable sc) {
exists(Node mid, ReturnNodeExt ret |
summaryReadStep(summaryArgParam(arg, ret, out), c, mid) and
summaryReadStep(summaryArgParam(arg, ret, out, sc), c, mid) and
summaryLocalStep(mid, ret, _)
)
}
Expand All @@ -843,9 +845,9 @@ module Private {
* NOTE: This step should not be used in global data-flow/taint-tracking, but may
* be useful to include in the exposed local data-flow/taint-tracking relations.
*/
predicate summarySetterStep(ArgNode arg, ContentSet c, Node out) {
predicate summarySetterStep(ArgNode arg, ContentSet c, Node out, SummarizedCallable sc) {
exists(Node mid, ReturnNodeExt ret |
summaryLocalStep(summaryArgParam(arg, ret, out), mid, _) and
summaryLocalStep(summaryArgParam(arg, ret, out, sc), mid, _) and
summaryStoreStep(mid, c, ret)
)
}
Expand Down Expand Up @@ -929,14 +931,20 @@ module Private {
private class SummarizedCallableExternal extends SummarizedCallable {
SummarizedCallableExternal() { summaryElement(this, _, _, _, _) }

private predicate relevantSummaryElement(AccessPath inSpec, AccessPath outSpec, string kind) {
summaryElement(this, inSpec, outSpec, kind, false)
or
private predicate relevantSummaryElementGenerated(
AccessPath inSpec, AccessPath outSpec, string kind
) {
summaryElement(this, inSpec, outSpec, kind, true) and
not summaryElement(this, _, _, _, false) and
not this.clearsContent(_, _)
}

private predicate relevantSummaryElement(AccessPath inSpec, AccessPath outSpec, string kind) {
summaryElement(this, inSpec, outSpec, kind, false)
or
this.relevantSummaryElementGenerated(inSpec, outSpec, kind)
}

override predicate propagatesFlow(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
Expand All @@ -951,7 +959,7 @@ module Private {
)
}

override predicate isAutoGenerated() { summaryElement(this, _, _, _, true) }
override predicate isAutoGenerated() { this.relevantSummaryElementGenerated(_, _, _) }
}

/** Holds if component `c` of specification `spec` cannot be parsed. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ private import csharp
private import TaintTrackingPublic
private import FlowSummaryImpl as FlowSummaryImpl
private import semmle.code.csharp.Caching
private import semmle.code.csharp.dataflow.internal.DataFlowDispatch
private import semmle.code.csharp.dataflow.internal.DataFlowPrivate
private import semmle.code.csharp.dataflow.internal.ControlFlowReachability
private import semmle.code.csharp.dispatch.Dispatch
Expand Down Expand Up @@ -117,19 +118,22 @@ private module Cached {
(
// Simple flow through library code is included in the exposed local
// step relation, even though flow is technically inter-procedural
FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(nodeFrom, nodeTo)
FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(nodeFrom, nodeTo,
any(DataFlowSummarizedCallable sc))
or
// Taint collection by adding a tainted element
exists(DataFlow::ElementContent c |
storeStep(nodeFrom, c, nodeTo)
or
FlowSummaryImpl::Private::Steps::summarySetterStep(nodeFrom, c, nodeTo)
FlowSummaryImpl::Private::Steps::summarySetterStep(nodeFrom, c, nodeTo,
any(DataFlowSummarizedCallable sc))
)
or
exists(DataFlow::Content c |
readStep(nodeFrom, c, nodeTo)
or
FlowSummaryImpl::Private::Steps::summaryGetterStep(nodeFrom, c, nodeTo)
FlowSummaryImpl::Private::Steps::summaryGetterStep(nodeFrom, c, nodeTo,
any(DataFlowSummarizedCallable sc))
|
// Taint members
c = any(TaintedMember m).(FieldOrProperty).getContent()
Expand Down
4 changes: 3 additions & 1 deletion csharp/ql/src/Language Abuse/ForeachCapture.ql
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

import csharp
import semmle.code.csharp.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
import semmle.code.csharp.dataflow.internal.DataFlowDispatch as DataFlowDispatch
import semmle.code.csharp.dataflow.internal.DataFlowPrivate as DataFlowPrivate
import semmle.code.csharp.frameworks.system.Collections
import semmle.code.csharp.frameworks.system.collections.Generic
Expand Down Expand Up @@ -76,7 +77,8 @@ Element getAssignmentTarget(Expr e) {
Element getCollectionAssignmentTarget(Expr e) {
// Store into collection via method
exists(DataFlowPrivate::PostUpdateNode postNode |
FlowSummaryImpl::Private::Steps::summarySetterStep(DataFlow::exprNode(e), _, postNode) and
FlowSummaryImpl::Private::Steps::summarySetterStep(DataFlow::exprNode(e), _, postNode,
any(DataFlowDispatch::DataFlowSummarizedCallable sc)) and
result.(Variable).getAnAccess() = postNode.getPreUpdateNode().asExpr()
)
or
Expand Down
15 changes: 11 additions & 4 deletions csharp/ql/test/library-tests/dataflow/external-models/steps.ql
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import csharp
import DataFlow
import semmle.code.csharp.dataflow.ExternalFlow
import semmle.code.csharp.dataflow.FlowSummary
import semmle.code.csharp.dataflow.internal.DataFlowDispatch as DataFlowDispatch
import semmle.code.csharp.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
import CsvValidation

Expand Down Expand Up @@ -43,17 +44,23 @@ private class SummarizedCallableClear extends SummarizedCallable {
query predicate summaryThroughStep(
DataFlow::Node node1, DataFlow::Node node2, boolean preservesValue
) {
FlowSummaryImpl::Private::Steps::summaryThroughStepValue(node1, node2) and preservesValue = true
FlowSummaryImpl::Private::Steps::summaryThroughStepValue(node1, node2,
any(DataFlowDispatch::DataFlowSummarizedCallable sc)) and
preservesValue = true
or
FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(node1, node2) and preservesValue = false
FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(node1, node2,
any(DataFlowDispatch::DataFlowSummarizedCallable sc)) and
preservesValue = false
}

query predicate summaryGetterStep(DataFlow::Node arg, DataFlow::Node out, Content c) {
FlowSummaryImpl::Private::Steps::summaryGetterStep(arg, c, out)
FlowSummaryImpl::Private::Steps::summaryGetterStep(arg, c, out,
any(DataFlowDispatch::DataFlowSummarizedCallable sc))
}

query predicate summarySetterStep(DataFlow::Node arg, DataFlow::Node out, Content c) {
FlowSummaryImpl::Private::Steps::summarySetterStep(arg, c, out)
FlowSummaryImpl::Private::Steps::summarySetterStep(arg, c, out,
any(DataFlowDispatch::DataFlowSummarizedCallable sc))
}

query predicate clearsContent(SummarizedCallable c, DataFlow::Content k, ParameterPosition pos) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ private module Cached {
or
// Simple flow through library code is included in the exposed local
// step relation, even though flow is technically inter-procedural
FlowSummaryImpl::Private::Steps::summaryThroughStepValue(node1, node2)
FlowSummaryImpl::Private::Steps::summaryThroughStepValue(node1, node2, _)
}

/**
Expand Down Expand Up @@ -154,7 +154,7 @@ private predicate simpleLocalFlowStep0(Node node1, Node node2) {
not exists(FieldRead fr |
hasNonlocalValue(fr) and fr.getField().isStatic() and fr = node1.asExpr()
) and
not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(node1)
not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(node1, _)
or
ThisFlow::adjacentThisRefs(node1, node2)
or
Expand Down
Loading