Skip to content

Commit a212d4f

Browse files
committed
v2
1 parent 0494b3d commit a212d4f

7 files changed

Lines changed: 538 additions & 329 deletions

File tree

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -179,14 +179,7 @@ Expr getPostUpdateReverseStep(Expr e, boolean preservesValue) {
179179

180180
module LocalFlow {
181181
predicate flowSummaryLocalStep(Node nodeFrom, Node nodeTo, string model) {
182-
exists(FlowSummaryImpl::Public::SummarizedCallable c |
183-
FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom,
184-
nodeTo.(FlowSummaryNode).getSummaryNode(), true, model) and
185-
c = nodeFrom.(FlowSummaryNode).getSummarizedCallable()
186-
)
187-
or
188-
FlowSummaryImpl::Private::Steps::sourceLocalStep(nodeFrom.(FlowSummaryNode).getSummaryNode(),
189-
nodeTo, model)
182+
FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom, nodeTo, true, model)
190183
or
191184
FlowSummaryImpl::Private::Steps::sinkLocalStep(nodeFrom,
192185
nodeTo.(FlowSummaryNode).getSummaryNode(), model)
@@ -650,10 +643,7 @@ module RustDataFlowGen<RustDataFlowInputSig Input> implements InputSig<Location>
650643
* variable.
651644
*/
652645
predicate jumpStep(Node node1, Node node2) {
653-
FlowSummaryImpl::Private::Steps::summaryJumpStep(node1.(FlowSummaryNode).getSummaryNode(),
654-
node2.(FlowSummaryNode).getSummaryNode())
655-
or
656-
FlowSummaryImpl::Private::Steps::sourceJumpStep(node1.(FlowSummaryNode).getSummaryNode(), node2)
646+
FlowSummaryImpl::Private::Steps::summaryJumpStep(node1, node2)
657647
or
658648
exists(Const c |
659649
node1.asExpr() = c.getBody() and
@@ -1302,7 +1292,7 @@ private module Cached {
13021292
FlowSummaryImpl::Private::SummaryComponentStack stack
13031293
|
13041294
FlowSummaryImpl::Private::barrierSpec(b, stack, kind, model) and
1305-
exists(FlowSummaryImpl::getEntryElement(b, stack, _, n.asExpr()))
1295+
n.asExpr() = FlowSummaryImpl::getEntryElement(b, stack.headOfSingleton())
13061296
)
13071297
or
13081298
ParameterizedBarrierGuard<TKindModelPair, barrierGuardChecks/4>::getABarrierNode(TMkPair(kind,

rust/ql/lib/codeql/rust/dataflow/internal/FlowSummaryImpl.qll

Lines changed: 33 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -148,46 +148,23 @@ private Callable getCallable(Expr expr) {
148148
)
149149
}
150150

151-
bindingset[f, s]
152-
bindingset[result, s]
153-
Input2::SourceSinkReportingElement getEntryElement(
154-
Function f, Impl::Private::SummaryComponentStack s, boolean isArg,
155-
Input2::SourceSinkReportingElement exit
156-
) {
157-
s.headOfSingleton() = Impl::Private::SummaryComponent::return(_) and
158-
result.(Call).getResolvedTarget() = f and
159-
isArg = false and
160-
exit = result
161-
or
162-
exists(RustDataFlow::ArgumentPosition pos |
163-
s.head() = Impl::Private::SummaryComponent::parameter(pos)
164-
|
165-
exists(Call call, Expr arg |
166-
call.getResolvedTarget() = f and
167-
arg = getArg(call, s.tail().headOfSingleton()) and
168-
exit = pos.getParameter(getCallable(arg)) and
169-
result = call and
170-
isArg = false
171-
)
151+
Input2::SourceSinkReportingElement getEntryElement(Function f, Impl::Private::SummaryComponent sc) {
152+
exists(Call call | call.getResolvedTarget() = f |
153+
sc = Impl::Private::SummaryComponent::return(_) and
154+
result = call
172155
or
156+
result = getArg(call, sc)
157+
)
158+
or
159+
exists(RustDataFlow::ArgumentPosition pos | sc = Impl::Private::SummaryComponent::parameter(pos) |
173160
exists(Function f0 |
174161
f0 = f
175162
or
176163
f0.implements(f)
177164
|
178-
s.length() = 1 and
179-
result = pos.getParameter(f0) and
180-
exit = result and
181-
isArg = false
165+
result = pos.getParameter(f0)
182166
)
183167
)
184-
or
185-
exists(Call call |
186-
call.getResolvedTarget() = f and
187-
result = getArg(call, s.headOfSingleton()) and
188-
exit = result and
189-
isArg = true
190-
)
191168
}
192169

193170
Input2::SourceSinkReportingElement getExitElement(Function f, Impl::Private::SummaryComponent sc) {
@@ -198,26 +175,39 @@ Input2::SourceSinkReportingElement getExitElement(Function f, Impl::Private::Sum
198175
}
199176

200177
private module Input2 implements Impl::Private::InputSig2 {
178+
private import codeql.rust.controlflow.CfgNodes
179+
201180
class SourceSinkReportingElement extends AstNode {
202181
DataFlowCallable getEnclosingCallable() { result.asCfgScope() = this.getEnclosingCfgScope() }
203182
}
204183

205-
bindingset[source, s]
206184
SourceSinkReportingElement getSourceEntryElement(
207-
Impl::Public::SourceElement source, Impl::Private::SummaryComponentStack s
185+
Impl::Public::SourceElement source, Impl::Private::SummaryComponent sc
208186
) {
209-
result = getEntryElement(source, s, _, _)
187+
result = getEntryElement(source, sc)
210188
}
211189

212-
bindingset[e, s]
213-
NodePublic getSourceExitNode(SourceSinkReportingElement e, Impl::Private::SummaryComponentStack s) {
214-
exists(boolean isArg, SourceSinkReportingElement exit | e = getEntryElement(_, s, isArg, exit) |
215-
isArg = false and
216-
result.(Node).getAstNode() = exit
217-
or
218-
isArg = true and
219-
result.(PostUpdateNode).getPreUpdateNode().(Node).getAstNode() = exit
190+
SourceSinkReportingElement getNextElement(
191+
SourceSinkReportingElement prev, Impl::Private::SummaryComponent sc
192+
) {
193+
exists(RustDataFlow::ArgumentPosition pos |
194+
sc = Impl::Private::SummaryComponent::parameter(pos) and
195+
result = pos.getParameter(getCallable(prev))
220196
)
197+
or
198+
exists(Callable c |
199+
sc = Impl::Private::SummaryComponent::return(_) and
200+
c = getCallable(prev) and
201+
result.getACfgNode().getASuccessor() instanceof AnnotatedExitCfgNode and
202+
result.getEnclosingCfgScope() = c
203+
)
204+
}
205+
206+
bindingset[e, sc]
207+
NodePublic getSourceExitNode(SourceSinkReportingElement e, Impl::Private::SummaryComponent sc) {
208+
if sc = Impl::Private::SummaryComponent::argument(_)
209+
then result.(PostUpdateNode).getPreUpdateNode().(Node).getAstNode() = e
210+
else result.(Node).getAstNode() = e
221211
}
222212

223213
SourceSinkReportingElement getSinkExitElement(

rust/ql/lib/codeql/rust/dataflow/internal/TaintTrackingImpl.qll

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,7 @@ module RustTaintTrackingGen<DataFlowImpl::RustDataFlowInputSig I> implements
8383
pred.(Node::PostUpdateNode).getPreUpdateNode().asExpr(), _, succ, _)
8484
)
8585
or
86-
FlowSummaryImpl::Private::Steps::summaryLocalStep(pred,
87-
succ.(Node::FlowSummaryNode).getSummaryNode(), false, model)
86+
FlowSummaryImpl::Private::Steps::summaryLocalStep(pred, succ, false, model)
8887
}
8988

9089
/**

rust/ql/lib/utils/test/InlineFlowTest.qll

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,21 @@ private module FlowTestImpl implements InputSig<Location, RustDataFlow> {
2929
any(CallExpr call | callTargetName(call, "sink")).getASyntacticArgument() = sink.asExpr()
3030
}
3131

32+
private string getSummaryNodeArgString(AstNode n) {
33+
exists(Call call |
34+
n = call
35+
or
36+
not n instanceof Call and
37+
n = call.getASyntacticArgument() // `n` is a callback
38+
|
39+
result = call.getPositionalArgument(0).toString()
40+
)
41+
or
42+
result = n.(Call).getPositionalArgument(0).toString()
43+
or
44+
result = n.(Param).getPat().(IdentPat).getName().getText()
45+
}
46+
3247
private string getSourceArgString(DataFlow::Node src) {
3348
defaultSource(src) and
3449
exists(Expr arg | arg = src.asExpr().(Call).getPositionalArgument(0) |
@@ -42,12 +57,9 @@ private module FlowTestImpl implements InputSig<Location, RustDataFlow> {
4257
sourceNode(src, _) and
4358
n = src.(Node::FlowSummaryNode).getSummaryNode().getSourceSinkReportingElement()
4459
|
45-
result = n.(Call).getPositionalArgument(0).toString()
60+
result = getSummaryNodeArgString(n)
4661
or
47-
result = n.(Param).getPat().(IdentPat).getName().getText()
48-
or
49-
not n instanceof Call and
50-
not n instanceof Param and
62+
not exists(getSummaryNodeArgString(n)) and
5163
result = n.toString()
5264
) and
5365
// Don't use the result if it contains spaces

rust/ql/test/library-tests/dataflow/models/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ mod sink_out_of_function {
323323
fn test_sink_out_of_function() {
324324
let s = source(5);
325325
let a = |a| source(1);
326-
pass_sink(a); // $ MISSING: hasValueFlow=1
326+
pass_sink(a); // $ hasValueFlow=1
327327
}
328328
}
329329

0 commit comments

Comments
 (0)