Skip to content

Commit

Permalink
Merge pull request #16520 from michaelnebel/csharp/fixsummarizedcalla…
Browse files Browse the repository at this point in the history
…bledataflow

C#: Make the flow summary filtering in the adapter.
  • Loading branch information
michaelnebel authored May 24, 2024
2 parents 0dbce3d + 2449074 commit 95473c0
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,6 @@ newtype TReturnKind =
TOutReturnKind(int i) { i = any(Parameter p | p.isOut()).getPosition() } or
TRefReturnKind(int i) { i = any(Parameter p | p.isRef()).getPosition() }

/**
* A summarized callable where the summary should be used for dataflow analysis.
*/
class DataFlowSummarizedCallable instanceof FlowSummary::SummarizedCallable {
DataFlowSummarizedCallable() {
not this.hasBody()
or
this.hasBody() and not this.applyGeneratedModel()
}

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

cached
private module Cached {
/**
Expand All @@ -47,7 +34,7 @@ private module Cached {
cached
newtype TDataFlowCallable =
TCallable(Callable c) { c.isUnboundDeclaration() } or
TSummarizedCallable(DataFlowSummarizedCallable sc) or
TSummarizedCallable(FlowSummary::SummarizedCallable sc) or
TFieldOrPropertyCallable(FieldOrProperty f) or
TCapturedVariableCallable(LocalScopeVariable v) { v.isCaptured() }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1181,8 +1181,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(nodeFrom, nodeTo,
any(DataFlowSummarizedCallable sc))
FlowSummaryImpl::Private::Steps::summaryThroughStepValue(nodeFrom, nodeTo, _)
}

cached
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,13 @@ private predicate interpretNeutral(UnboundCallable c, string kind, string proven

// adapter class for converting Mad summaries to `SummarizedCallable`s
private class SummarizedCallableAdapter extends SummarizedCallable {
SummarizedCallableAdapter() { interpretSummary(this, _, _, _, _, _) }
SummarizedCallableAdapter() {
exists(Provenance provenance | interpretSummary(this, _, _, _, provenance, _) |
not this.hasBody()
or
this.hasBody() and provenance.isManual()
)
}

private predicate relevantSummaryElementManual(
string input, string output, string kind, string model
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,22 +119,19 @@ 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,
any(DataFlowSummarizedCallable sc))
FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(nodeFrom, nodeTo, _)
or
// Taint collection by adding a tainted element
exists(DataFlow::ElementContent c |
storeStep(nodeFrom, c, nodeTo)
or
FlowSummaryImpl::Private::Steps::summarySetterStep(nodeFrom, c, nodeTo,
any(DataFlowSummarizedCallable sc))
FlowSummaryImpl::Private::Steps::summarySetterStep(nodeFrom, c, nodeTo, _)
)
or
exists(DataFlow::Content c |
readStep(nodeFrom, c, nodeTo)
or
FlowSummaryImpl::Private::Steps::summaryGetterStep(nodeFrom, c, nodeTo,
any(DataFlowSummarizedCallable sc))
FlowSummaryImpl::Private::Steps::summaryGetterStep(nodeFrom, c, nodeTo, _)
|
// Taint members
c = any(TaintedMember m).(FieldOrProperty).getContent()
Expand Down
3 changes: 1 addition & 2 deletions csharp/ql/src/Language Abuse/ForeachCapture.ql
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ 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,
any(DataFlowDispatch::DataFlowSummarizedCallable sc)) and
FlowSummaryImpl::Private::Steps::summarySetterStep(DataFlow::exprNode(e), _, postNode, _) and
result.(Variable).getAnAccess() = postNode.getPreUpdateNode().asExpr()
)
or
Expand Down
12 changes: 4 additions & 8 deletions csharp/ql/test/library-tests/dataflow/external-models/steps.ql
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,17 @@ private class StepArgQualGenerated extends Method {
query predicate summaryThroughStep(
DataFlow::Node node1, DataFlow::Node node2, boolean preservesValue
) {
FlowSummaryImpl::Private::Steps::summaryThroughStepValue(node1, node2,
any(DataFlowDispatch::DataFlowSummarizedCallable sc)) and
FlowSummaryImpl::Private::Steps::summaryThroughStepValue(node1, node2, _) and
preservesValue = true
or
FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(node1, node2,
any(DataFlowDispatch::DataFlowSummarizedCallable sc)) and
FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(node1, node2, _) and
preservesValue = false
}

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

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

0 comments on commit 95473c0

Please sign in to comment.