Skip to content

Commit 4f9da19

Browse files
committed
data flow wip
1 parent 2c9fb6c commit 4f9da19

File tree

9 files changed

+157
-63
lines changed

9 files changed

+157
-63
lines changed

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

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -145,13 +145,9 @@ final class ArgumentPosition extends ParameterPosition {
145145
* as the synthetic `ReceiverNode` is the argument for the `self` parameter.
146146
*/
147147
predicate isArgumentForCall(ExprCfgNode arg, CallCfgNode call, ParameterPosition pos) {
148-
// TODO: Handle index expressions as calls in data flow.
149-
not call.getCall() instanceof IndexExpr and
150-
(
151-
call.getPositionalArgument(pos.getPosition()) = arg
152-
or
153-
call.getReceiver() = arg and pos.isSelf() and not call.getCall().receiverImplicitlyBorrowed()
154-
)
148+
call.getPositionalArgument(pos.getPosition()) = arg
149+
or
150+
call.getReceiver() = arg and pos.isSelf() and not call.getCall().receiverImplicitlyBorrowed()
155151
}
156152

157153
/** Provides logic related to SSA. */
@@ -557,12 +553,6 @@ module RustDataFlow implements InputSig<Location> {
557553
access = c.(FieldContent).getAnAccess()
558554
)
559555
or
560-
exists(IndexExprCfgNode arr |
561-
c instanceof ElementContent and
562-
node1.asExpr() = arr.getBase() and
563-
node2.asExpr() = arr
564-
)
565-
or
566556
exists(ForExprCfgNode for |
567557
c instanceof ElementContent and
568558
node1.asExpr() = for.getIterable() and
@@ -582,13 +572,6 @@ module RustDataFlow implements InputSig<Location> {
582572
.isVariantField([any(OptionEnum o).getSome(), any(ResultEnum r).getOk()], 0)
583573
)
584574
or
585-
exists(PrefixExprCfgNode deref |
586-
c instanceof ReferenceContent and
587-
deref.getOperatorName() = "*" and
588-
node1.asExpr() = deref.getExpr() and
589-
node2.asExpr() = deref
590-
)
591-
or
592575
// Read from function return
593576
exists(DataFlowCall call |
594577
lambdaCall(call, _, node1) and
@@ -700,6 +683,7 @@ module RustDataFlow implements InputSig<Location> {
700683
or
701684
referenceAssignment(node1, node2.(PostUpdateNode).getPreUpdateNode(), c)
702685
or
686+
// todo: rely on flow summary
703687
exists(AssignmentExprCfgNode assignment, IndexExprCfgNode index |
704688
c instanceof ElementContent and
705689
assignment.getLhs() = index and
@@ -991,11 +975,7 @@ private module Cached {
991975

992976
cached
993977
newtype TDataFlowCall =
994-
TCall(CallCfgNode c) {
995-
Stages::DataFlowStage::ref() and
996-
// TODO: Handle index expressions as calls in data flow.
997-
not c.getCall() instanceof IndexExpr
998-
} or
978+
TCall(CallCfgNode c) { Stages::DataFlowStage::ref() } or
999979
TSummaryCall(
1000980
FlowSummaryImpl::Public::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNode receiver
1001981
) {

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -467,15 +467,11 @@ newtype TNode =
467467
any(TryExprCfgNode try).getExpr(), //
468468
any(PrefixExprCfgNode pe | pe.getOperatorName() = "*").getExpr(), //
469469
any(AwaitExprCfgNode a).getExpr(), //
470-
any(MethodCallExprCfgNode mc).getReceiver(), //
470+
any(CallCfgNode call | call.getCall().receiverImplicitlyBorrowed()).getReceiver(), //
471471
getPostUpdateReverseStep(any(PostUpdateNode n).getPreUpdateNode().asExpr(), _)
472472
]
473473
} or
474-
TReceiverNode(CallCfgNode mc, Boolean isPost) {
475-
mc.getCall().receiverImplicitlyBorrowed() and
476-
// TODO: Handle index expressions as calls in data flow.
477-
not mc.getCall() instanceof IndexExpr
478-
} or
474+
TReceiverNode(CallCfgNode mc, Boolean isPost) { mc.getCall().receiverImplicitlyBorrowed() } or
479475
TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or
480476
TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) or
481477
TClosureSelfReferenceNode(CfgScope c) { lambdaCreationExpr(c, _) } or

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

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,14 @@ module RustTaintTracking implements InputSig<Location, RustDataFlow> {
3535
or
3636
pred.asExpr() = succ.asExpr().(CastExprCfgNode).getExpr()
3737
or
38-
exists(IndexExprCfgNode index |
39-
index.getIndex() instanceof RangeExprCfgNode and
40-
pred.asExpr() = index.getBase() and
41-
succ.asExpr() = index
42-
)
43-
or
44-
// Although data flow through collections is modeled using stores/reads,
45-
// we also allow taint to flow out of a tainted collection. This is
46-
// needed in order to support taint-tracking configurations where the
47-
// source is a collection.
48-
exists(SingletonContentSet cs |
49-
RustDataFlow::readStep(pred, cs, succ) and
38+
// Although data flow through collections and references is modeled using
39+
// stores/reads, we also allow taint to flow out of a tainted collection.
40+
// This is needed in order to support taint-tracking configurations where
41+
// the source is a collection or reference.
42+
exists(SingletonContentSet cs | RustDataFlow::readStep(pred, cs, succ) |
5043
cs.getContent() instanceof ElementContent
44+
or
45+
cs.getContent() instanceof ReferenceContent
5146
)
5247
or
5348
exists(FormatArgsExprCfgNode format | succ.asExpr() = format |

rust/ql/lib/codeql/rust/frameworks/stdlib/Stdlib.qll

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ private import codeql.rust.Concepts
77
private import codeql.rust.controlflow.ControlFlowGraph as Cfg
88
private import codeql.rust.controlflow.CfgNodes as CfgNodes
99
private import codeql.rust.dataflow.DataFlow
10+
private import codeql.rust.dataflow.FlowSummary
1011
private import codeql.rust.internal.PathResolution
12+
private import codeql.rust.internal.Type
13+
private import codeql.rust.internal.TypeInference
14+
private import codeql.rust.internal.TypeMention
1115

1216
/**
1317
* A call to the `starts_with` method on a `Path`.
@@ -213,3 +217,111 @@ class StringStruct extends Struct {
213217
pragma[nomagic]
214218
StringStruct() { this.getCanonicalPath() = "alloc::string::String" }
215219
}
220+
221+
/**
222+
* The [`Deref` trait][1].
223+
*
224+
* [1]: https://doc.rust-lang.org/core/ops/trait.Deref.html
225+
*/
226+
class DerefTrait extends Trait {
227+
pragma[nomagic]
228+
DerefTrait() { this.getCanonicalPath() = "core::ops::deref::Deref" }
229+
230+
/** Gets the `Target` associated type. */
231+
pragma[nomagic]
232+
TypeAlias getTargetType() {
233+
result = this.getAssocItemList().getAnAssocItem() and
234+
result.getName().getText() = "Target"
235+
}
236+
}
237+
238+
/**
239+
* One of the two special
240+
*
241+
* ```rust
242+
* impl<T: ?Sized> const Deref for &T
243+
* impl<T: ?Sized> const Deref for &mut T
244+
* ```
245+
*
246+
* implementations.
247+
*/
248+
private class CoreDerefImpl extends ImplItemNode {
249+
pragma[nomagic]
250+
CoreDerefImpl() {
251+
this.resolveTraitTy() instanceof DerefTrait and
252+
this.(Impl)
253+
.getSelfTy()
254+
.(TypeMention)
255+
.resolveTypeAt(TypePath::singleton(TRefTypeParameter()))
256+
.(TypeParamTypeParameter)
257+
.getTypeParam() = this.getTypeParam(_)
258+
}
259+
260+
Function getDeref() { result = this.getASuccessor("deref") }
261+
}
262+
263+
// TODO: Use MaD when `&(mut) T` is assigned an appropriate canonical path
264+
private class CoreDerefSummarizedCallable extends SummarizedCallable::Range {
265+
CoreDerefSummarizedCallable() { this = any(CoreDerefImpl i).getDeref() }
266+
267+
override predicate propagatesFlow(string input, string output, boolean preservesValue) {
268+
input = "Argument[self].Reference" and
269+
output = "ReturnValue" and
270+
preservesValue = true
271+
}
272+
}
273+
274+
/**
275+
* The [`Index` trait][1].
276+
*
277+
* [1]: https://doc.rust-lang.org/std/ops/trait.Index.html
278+
*/
279+
class IndexTrait extends Trait {
280+
pragma[nomagic]
281+
IndexTrait() { this.getCanonicalPath() = "core::ops::index::Index" }
282+
283+
/** Gets the `Output` associated type. */
284+
pragma[nomagic]
285+
TypeAlias getOutputType() {
286+
result = this.getAssocItemList().getAnAssocItem() and
287+
result.getName().getText() = "Output"
288+
}
289+
}
290+
291+
/**
292+
* One of the two special
293+
*
294+
* ```rust
295+
* impl<T, I, const N: usize> Index<I> for [T; N]
296+
* impl<T, I> ops::Index<I> for [T]
297+
* ```
298+
*
299+
* implementations.
300+
*/
301+
private class CoreIndexImpl extends ImplItemNode {
302+
pragma[nomagic]
303+
CoreIndexImpl() {
304+
this.resolveTraitTy() instanceof IndexTrait and
305+
exists(TypeParameter tp | tp = TArrayTypeParameter() or tp = TSliceTypeParameter() |
306+
this.(Impl)
307+
.getSelfTy()
308+
.(TypeMention)
309+
.resolveTypeAt(TypePath::singleton(tp))
310+
.(TypeParamTypeParameter)
311+
.getTypeParam() = this.getTypeParam(_)
312+
)
313+
}
314+
315+
Function getIndex() { result = this.getASuccessor("index") }
316+
}
317+
318+
// TODO: Use MaD when `[T(; N)]` is assigned an appropriate canonical path
319+
private class CoreIndexSummarizedCallable extends SummarizedCallable::Range {
320+
CoreIndexSummarizedCallable() { this = any(CoreIndexImpl i).getIndex() }
321+
322+
override predicate propagatesFlow(string input, string output, boolean preservesValue) {
323+
input = "Argument[self].Reference.Element" and
324+
output = "ReturnValue" and
325+
preservesValue = true
326+
}
327+
}

rust/ql/lib/codeql/rust/frameworks/stdlib/lang-alloc.model.yml

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,10 @@ extensions:
3535
- ["<alloc::boxed::Box>::pin", "Argument[0]", "ReturnValue.Reference", "value", "manual"]
3636
- ["<alloc::boxed::Box>::new", "Argument[0]", "ReturnValue.Reference", "value", "manual"]
3737
- ["<alloc::boxed::Box>::into_pin", "Argument[0]", "ReturnValue", "value", "manual"]
38+
- ["<alloc::boxed::Box as core::ops::deref::Deref>::deref", "Argument[self].Reference", "ReturnValue", "value", "manual"]
3839
# Fmt
3940
- ["alloc::fmt::format", "Argument[0]", "ReturnValue", "taint", "manual"]
4041
# String
41-
- ["<core::str>::as_str", "Argument[self]", "ReturnValue", "value", "manual"]
42-
- ["<core::str>::as_bytes", "Argument[self]", "ReturnValue", "value", "manual"]
43-
- ["<alloc::string::String>::as_str", "Argument[self]", "ReturnValue", "value", "manual"]
44-
- ["<alloc::string::String>::as_bytes", "Argument[self]", "ReturnValue", "value", "manual"]
45-
- ["<alloc::str as alloc::string::ToString>::to_string", "Argument[self]", "ReturnValue", "taint", "manual"]
46-
- ["<alloc::string::String as alloc::string::ToString>::to_string", "Argument[self]", "ReturnValue", "taint", "manual"]
47-
- ["<core::str>::parse", "Argument[self]", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
48-
- ["<core::str>::trim", "Argument[self]", "ReturnValue.Reference", "taint", "manual"]
49-
- ["<alloc::string::String as core::convert::From>::from", "Argument[0]", "ReturnValue", "value", "manual"]
42+
- ["<alloc::string::String>::as_str", "Argument[self].Reference", "ReturnValue.Reference", "taint", "manual"]
43+
- ["<alloc::string::String>::as_bytes", "Argument[self].Reference", "ReturnValue.Reference", "taint", "manual"]
44+
- ["<alloc::string::String as core::convert::From>::from", "Argument[0].Reference", "ReturnValue", "taint", "manual"]

rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ extensions:
2626
- ["<core::slice::iter::Iter as core::iter::traits::iterator::Iterator>::collect", "Argument[self].Element", "ReturnValue.Element", "value", "manual"]
2727
- ["<core::slice::iter::Iter as core::iter::traits::iterator::Iterator>::map", "Argument[self].Element", "Argument[0].Parameter[0]", "value", "manual"]
2828
- ["<_ as core::iter::traits::iterator::Iterator>::for_each", "Argument[self].Element", "Argument[0].Parameter[0]", "value", "manual"]
29+
# Index
30+
- ["<alloc::vec::Vec as core::ops::index::Index>::index", "Argument[self].Reference.Element", "ReturnValue", "value", "manual"]
31+
- ["<alloc::string::String as core::ops::index::Index>::index", "Argument[self].Reference", "ReturnValue", "taint", "manual"]
2932
# Layout
3033
- ["<core::alloc::layout::Layout>::from_size_align", "Argument[0]", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
3134
- ["<core::alloc::layout::Layout>::from_size_align_unchecked", "Argument[0]", "ReturnValue", "taint", "manual"]
@@ -48,6 +51,7 @@ extensions:
4851
- ["<core::pin::Pin>::into_inner", "Argument[0]", "ReturnValue", "value", "manual"]
4952
- ["<core::pin::Pin>::into_inner_unchecked", "Argument[0]", "ReturnValue", "value", "manual"]
5053
- ["<core::pin::Pin>::set", "Argument[0]", "Argument[self]", "value", "manual"]
54+
- ["<core::pin::Pin as core::ops::deref::Deref>::deref", "Argument[self].Reference", "ReturnValue", "value", "manual"]
5155
# Ptr
5256
- ["core::ptr::read", "Argument[0].Reference", "ReturnValue", "value", "manual"]
5357
- ["core::ptr::read_unaligned", "Argument[0].Reference", "ReturnValue", "value", "manual"]
@@ -56,13 +60,10 @@ extensions:
5660
- ["core::ptr::write_unaligned", "Argument[1]", "Argument[0].Reference", "value", "manual"]
5761
- ["core::ptr::write_volatile", "Argument[1]", "Argument[0].Reference", "value", "manual"]
5862
# Str
59-
- ["<core::str>::as_str", "Argument[self]", "ReturnValue", "taint", "value"]
60-
- ["<alloc::string::String>::as_str", "Argument[self]", "ReturnValue", "taint", "value"]
61-
- ["<core::str>::as_bytes", "Argument[self]", "ReturnValue", "taint", "value"]
62-
- ["<alloc::string::String>::as_bytes", "Argument[self]", "ReturnValue", "taint", "value"]
63-
- ["<core::str>::to_string", "Argument[self]", "ReturnValue", "taint", "manual"]
64-
- ["<core::str>::parse", "Argument[self]", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
65-
- ["<core::str>::trim", "Argument[self]", "ReturnValue.Reference", "taint", "manual"]
63+
- ["<core::str>::as_str", "Argument[self].Reference", "ReturnValue.Reference", "taint", "value"]
64+
- ["<core::str>::as_bytes", "Argument[self].Reference", "ReturnValue.Reference", "taint", "value"]
65+
- ["<core::str>::parse", "Argument[self].Reference", "ReturnValue.Field[core::result::Result::Ok(0)]", "taint", "manual"]
66+
- ["<core::str>::trim", "Argument[self].Reference", "ReturnValue.Reference", "taint", "manual"]
6667
- addsTo:
6768
pack: codeql/rust-all
6869
extensible: sourceModel

rust/ql/lib/codeql/rust/internal/TypeInference.qll

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3264,7 +3264,15 @@ private module Cached {
32643264
/** Holds if `receiver` is the receiver of a method call with an implicit dereference. */
32653265
cached
32663266
predicate receiverHasImplicitDeref(AstNode receiver) {
3267-
none() // todo
3267+
exists(MethodCall mc |
3268+
exists(resolveMethodCallTarget(MkMethodCallDerefChainRef(mc, ".ref;"))) and
3269+
receiver = mc.getArgument(CallImpl::TSelfArgumentPosition())
3270+
)
3271+
// or
3272+
// exists(Op op |
3273+
// op.(Call).implicitBorrowAt(CallImpl::TSelfArgumentPosition(), true) and
3274+
// receiver = op.getOperand(0)
3275+
// )
32683276
// exists(MethodCallExprMatchingInput::Access a, MethodCallExprMatchingInput::AccessPosition apos |
32693277
// apos.getArgumentPosition().isSelf() and
32703278
// apos.isBorrowed(_) and
@@ -3277,7 +3285,15 @@ private module Cached {
32773285
/** Holds if `receiver` is the receiver of a method call with an implicit borrow. */
32783286
cached
32793287
predicate receiverHasImplicitBorrow(AstNode receiver) {
3280-
none() // todo
3288+
exists(MethodCall mc |
3289+
exists(resolveMethodCallTarget(MkMethodCallDerefChainRef(mc, ";ref"))) and
3290+
receiver = mc.getArgument(CallImpl::TSelfArgumentPosition())
3291+
)
3292+
or
3293+
exists(Op op |
3294+
op.(Call).implicitBorrowAt(CallImpl::TSelfArgumentPosition(), true) and
3295+
receiver = op.getOperand(0)
3296+
)
32813297
// exists(MethodCallExprMatchingInput::Access a, MethodCallExprMatchingInput::AccessPosition apos |
32823298
// apos.getArgumentPosition().isSelf() and
32833299
// apos.isBorrowed(_) and

rust/ql/test/library-tests/dataflow/global/inline-flow.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ subpaths
400400
| main.rs:301:50:301:50 | a [MyInt] | main.rs:289:18:289:21 | SelfParam [MyInt] | main.rs:289:48:291:5 | { ... } [MyInt] | main.rs:301:30:301:54 | ...::take_self(...) [MyInt] |
401401
| main.rs:306:55:306:55 | b [MyInt] | main.rs:293:26:293:37 | ...: MyInt [MyInt] | main.rs:293:49:295:5 | { ... } [MyInt] | main.rs:306:30:306:56 | ...::take_second(...) [MyInt] |
402402
testFailures
403+
| main.rs:277:14:277:58 | //... | Missing result: hasTaintFlow=28 |
403404
#select
404405
| main.rs:18:10:18:10 | a | main.rs:13:5:13:13 | source(...) | main.rs:18:10:18:10 | a | $@ | main.rs:13:5:13:13 | source(...) | source(...) |
405406
| main.rs:39:10:39:21 | a.get_data() | main.rs:38:23:38:31 | source(...) | main.rs:39:10:39:21 | a.get_data() | $@ | main.rs:38:23:38:31 | source(...) | source(...) |

rust/ql/test/library-tests/dataflow/global/viableCallable.expected

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,6 @@
5959
| main.rs:212:13:212:34 | ...::new(...) | main.rs:205:5:208:5 | fn new |
6060
| main.rs:212:24:212:33 | source(...) | main.rs:1:1:3:1 | fn source |
6161
| main.rs:214:5:214:11 | sink(...) | main.rs:5:1:7:1 | fn sink |
62-
| main.rs:228:10:228:14 | * ... | main.rs:235:5:237:5 | fn deref |
63-
| main.rs:236:11:236:15 | * ... | main.rs:235:5:237:5 | fn deref |
6462
| main.rs:242:28:242:36 | source(...) | main.rs:1:1:3:1 | fn source |
6563
| main.rs:244:13:244:17 | ... + ... | main.rs:220:5:223:5 | fn add |
6664
| main.rs:245:5:245:17 | sink(...) | main.rs:5:1:7:1 | fn sink |

0 commit comments

Comments
 (0)