Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C#: Make the flow summary filtering in the adapter. #16520

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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, _)
}