Skip to content

Rust: Use type inference to insert implicit borrows and derefs #19419

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 1, 2025
Merged

Rust: Use type inference to insert implicit borrows and derefs #19419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f584d22
Rust: Use type inference to insert implicit borrows and derefs
paldepind Apr 30, 2025
c263d3f
Rust: Remove predicates unused after refactor
paldepind Apr 30, 2025
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
22 changes: 3 additions & 19 deletions rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ private import rust
private import SsaImpl as SsaImpl
private import codeql.rust.controlflow.internal.Scope as Scope
private import codeql.rust.internal.PathResolution
private import codeql.rust.internal.TypeInference as TypeInference
private import codeql.rust.controlflow.ControlFlowGraph
private import codeql.rust.controlflow.CfgNodes
private import codeql.rust.dataflow.Ssa
Expand Down Expand Up @@ -321,23 +322,6 @@ predicate lambdaCallExpr(CallExprCfgNode call, LambdaCallKind kind, ExprCfgNode
exists(kind)
}

/** Holds if `mc` implicitly borrows its receiver. */
private predicate implicitBorrow(MethodCallExpr mc) {
// Determining whether an implicit borrow happens depends on the type of the
// receiever as well as the target. As a heuristic we simply check if the
// target takes `self` as a borrow and limit the approximation to cases where
// the receiver is a simple variable.
mc.getReceiver() instanceof VariableAccess and
mc.getStaticTarget().getParamList().getSelfParam().isRef()
}

/** Holds if `mc` implicitly dereferences its receiver. */
private predicate implicitDeref(MethodCallExpr mc) {
// Similarly to `implicitBorrow` this is an approximation.
mc.getReceiver() instanceof VariableAccess and
not mc.getStaticTarget().getParamList().getSelfParam().isRef()
}

// Defines a set of aliases needed for the `RustDataFlow` module
private module Aliases {
class DataFlowCallableAlias = DataFlowCallable;
Expand Down Expand Up @@ -520,15 +504,15 @@ module RustDataFlow implements InputSig<Location> {

pragma[nomagic]
private predicate implicitDerefToReceiver(Node node1, ReceiverNode node2, ReferenceContent c) {
TypeInference::receiverHasImplicitDeref(node1.asExpr().getExpr()) and
node1.asExpr() = node2.getReceiver() and
implicitDeref(node2.getMethodCall().getMethodCallExpr()) and
exists(c)
}

pragma[nomagic]
private predicate implicitBorrowToReceiver(Node node1, ReceiverNode node2, ReferenceContent c) {
TypeInference::receiverHasImplicitBorrow(node1.asExpr().getExpr()) and
node1.asExpr() = node2.getReceiver() and
implicitBorrow(node2.getMethodCall().getMethodCallExpr()) and
exists(c)
}

Expand Down
44 changes: 24 additions & 20 deletions rust/ql/lib/codeql/rust/internal/TypeInference.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -662,15 +662,6 @@ private module CallExprBaseMatchingInput implements MatchingInputSig {
tAdj = t
)
}

pragma[nomagic]
additional Type inferReceiverType(AstNode n) {
exists(Access a, AccessPosition apos |
result = inferType(n) and
n = a.getNodeAt(apos) and
apos.isSelf()
)
}
}

private module CallExprBaseMatching = Matching<CallExprBaseMatchingInput>;
Expand All @@ -690,7 +681,7 @@ private Type inferCallExprBaseType(AstNode n, TypePath path) {
|
if apos.isSelf()
then
exists(Type receiverType | receiverType = CallExprBaseMatchingInput::inferReceiverType(n) |
exists(Type receiverType | receiverType = inferType(n) |
if receiverType = TRefType()
then
path = path0 and
Expand Down Expand Up @@ -813,15 +804,6 @@ private module FieldExprMatchingInput implements MatchingInputSig {
tAdj = t
)
}

pragma[nomagic]
additional Type inferReceiverType(AstNode n) {
exists(Access a, AccessPosition apos |
result = inferType(n) and
n = a.getNodeAt(apos) and
apos.isSelf()
)
}
}

private module FieldExprMatching = Matching<FieldExprMatchingInput>;
Expand All @@ -840,7 +822,7 @@ private Type inferFieldExprType(AstNode n, TypePath path) {
|
if apos.isSelf()
then
exists(Type receiverType | receiverType = FieldExprMatchingInput::inferReceiverType(n) |
exists(Type receiverType | receiverType = inferType(n) |
if receiverType = TRefType()
then
// adjust for implicit deref
Expand Down Expand Up @@ -895,6 +877,28 @@ cached
private module Cached {
private import codeql.rust.internal.CachedStages

/** Holds if `receiver` is the receiver of a method call with an implicit dereference. */
cached
predicate receiverHasImplicitDeref(AstNode receiver) {
exists(CallExprBaseMatchingInput::Access a, CallExprBaseMatchingInput::AccessPosition apos |
apos.isSelf() and
receiver = a.getNodeAt(apos) and
inferType(receiver) = TRefType() and
CallExprBaseMatching::inferAccessType(a, apos, TypePath::nil()) != TRefType()
)
}

/** Holds if `receiver` is the receiver of a method call with an implicit borrow. */
cached
predicate receiverHasImplicitBorrow(AstNode receiver) {
exists(CallExprBaseMatchingInput::Access a, CallExprBaseMatchingInput::AccessPosition apos |
apos.isSelf() and
receiver = a.getNodeAt(apos) and
CallExprBaseMatching::inferAccessType(a, apos, TypePath::nil()) = TRefType() and
inferType(receiver) != TRefType()
)
}

pragma[inline]
private Type getLookupType(AstNode n) {
exists(Type t |
Expand Down
35 changes: 0 additions & 35 deletions rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1931,33 +1931,16 @@ readStep
| main.rs:221:9:221:23 | ...::Some(...) | Some | main.rs:221:22:221:22 | n |
| main.rs:230:9:230:15 | Some(...) | Some | main.rs:230:14:230:14 | n |
| main.rs:234:9:234:15 | Some(...) | Some | main.rs:234:14:234:14 | n |
| main.rs:241:10:241:11 | s1 | &ref | main.rs:241:10:241:11 | receiver for s1 |
| main.rs:246:10:246:11 | s1 | &ref | main.rs:246:10:246:11 | receiver for s1 |
| main.rs:249:10:249:11 | s2 | &ref | main.rs:249:10:249:11 | receiver for s2 |
| main.rs:254:10:254:11 | s1 | &ref | main.rs:254:10:254:11 | receiver for s1 |
| main.rs:257:10:257:11 | s2 | &ref | main.rs:257:10:257:11 | receiver for s2 |
| main.rs:263:14:263:15 | s1 | Ok | main.rs:263:14:263:16 | TryExpr |
| main.rs:263:14:263:15 | s1 | Some | main.rs:263:14:263:16 | TryExpr |
| main.rs:265:10:265:11 | s2 | Ok | main.rs:265:10:265:12 | TryExpr |
| main.rs:265:10:265:11 | s2 | Some | main.rs:265:10:265:12 | TryExpr |
| main.rs:271:29:271:30 | r1 | &ref | main.rs:271:29:271:30 | receiver for r1 |
| main.rs:272:29:272:30 | r1 | &ref | main.rs:272:29:272:30 | receiver for r1 |
| main.rs:273:10:273:12 | o1a | &ref | main.rs:273:10:273:12 | receiver for o1a |
| main.rs:274:10:274:12 | o1b | &ref | main.rs:274:10:274:12 | receiver for o1b |
| main.rs:277:29:277:30 | r2 | &ref | main.rs:277:29:277:30 | receiver for r2 |
| main.rs:278:29:278:30 | r2 | &ref | main.rs:278:29:278:30 | receiver for r2 |
| main.rs:279:10:279:12 | o2a | &ref | main.rs:279:10:279:12 | receiver for o2a |
| main.rs:280:10:280:12 | o2b | &ref | main.rs:280:10:280:12 | receiver for o2b |
| main.rs:287:14:287:15 | s1 | Ok | main.rs:287:14:287:16 | TryExpr |
| main.rs:287:14:287:15 | s1 | Some | main.rs:287:14:287:16 | TryExpr |
| main.rs:288:14:288:15 | s2 | Ok | main.rs:288:14:288:16 | TryExpr |
| main.rs:288:14:288:15 | s2 | Some | main.rs:288:14:288:16 | TryExpr |
| main.rs:291:14:291:15 | s3 | Ok | main.rs:291:14:291:16 | TryExpr |
| main.rs:291:14:291:15 | s3 | Some | main.rs:291:14:291:16 | TryExpr |
| main.rs:298:10:298:11 | s1 | &ref | main.rs:298:10:298:11 | receiver for s1 |
| main.rs:299:10:299:11 | s1 | &ref | main.rs:299:10:299:11 | receiver for s1 |
| main.rs:302:10:302:11 | s2 | &ref | main.rs:302:10:302:11 | receiver for s2 |
| main.rs:303:10:303:11 | s2 | &ref | main.rs:303:10:303:11 | receiver for s2 |
| main.rs:315:9:315:25 | ...::A(...) | A | main.rs:315:24:315:24 | n |
| main.rs:316:9:316:25 | ...::B(...) | B | main.rs:316:24:316:24 | n |
| main.rs:319:9:319:25 | ...::A(...) | A | main.rs:319:24:319:24 | n |
Expand Down Expand Up @@ -1997,40 +1980,22 @@ readStep
| main.rs:442:9:442:20 | TuplePat | tuple.0 | main.rs:442:10:442:13 | cond |
| main.rs:442:9:442:20 | TuplePat | tuple.1 | main.rs:442:16:442:19 | name |
| main.rs:442:25:442:29 | names | element | main.rs:442:9:442:20 | TuplePat |
| main.rs:444:21:444:24 | name | &ref | main.rs:444:21:444:24 | receiver for name |
| main.rs:444:41:444:67 | [post] \|...\| ... | captured default_name | main.rs:444:41:444:67 | [post] default_name |
| main.rs:444:44:444:55 | default_name | &ref | main.rs:444:44:444:55 | receiver for default_name |
| main.rs:444:44:444:55 | this | captured default_name | main.rs:444:44:444:55 | default_name |
| main.rs:445:18:445:18 | n | &ref | main.rs:445:18:445:18 | receiver for n |
| main.rs:468:13:468:13 | a | &ref | main.rs:468:13:468:13 | receiver for a |
| main.rs:469:13:469:13 | b | &ref | main.rs:469:13:469:13 | receiver for b |
| main.rs:470:19:470:19 | b | &ref | main.rs:470:19:470:19 | receiver for b |
| main.rs:481:10:481:11 | vs | element | main.rs:481:10:481:14 | vs[0] |
| main.rs:482:11:482:12 | vs | &ref | main.rs:482:11:482:12 | receiver for vs |
| main.rs:482:11:482:35 | ... .unwrap() | &ref | main.rs:482:10:482:35 | * ... |
| main.rs:483:11:483:12 | vs | &ref | main.rs:483:11:483:12 | receiver for vs |
| main.rs:483:11:483:35 | ... .unwrap() | &ref | main.rs:483:10:483:35 | * ... |
| main.rs:485:14:485:15 | vs | element | main.rs:485:9:485:9 | v |
| main.rs:488:9:488:10 | &... | &ref | main.rs:488:10:488:10 | v |
| main.rs:488:15:488:16 | vs | &ref | main.rs:488:15:488:16 | receiver for vs |
| main.rs:488:15:488:23 | vs.iter() | element | main.rs:488:9:488:10 | &... |
| main.rs:492:27:492:28 | vs | &ref | main.rs:492:27:492:28 | receiver for vs |
| main.rs:493:9:493:10 | &... | &ref | main.rs:493:10:493:10 | v |
| main.rs:493:15:493:17 | vs2 | element | main.rs:493:9:493:10 | &... |
| main.rs:497:5:497:6 | vs | &ref | main.rs:497:5:497:6 | receiver for vs |
| main.rs:497:29:497:29 | x | &ref | main.rs:497:28:497:29 | * ... |
| main.rs:498:5:498:6 | vs | &ref | main.rs:498:5:498:6 | receiver for vs |
| main.rs:498:34:498:34 | x | &ref | main.rs:498:33:498:34 | * ... |
| main.rs:500:14:500:15 | vs | &ref | main.rs:500:14:500:15 | receiver for vs |
| main.rs:500:14:500:27 | vs.into_iter() | element | main.rs:500:9:500:9 | v |
| main.rs:506:10:506:15 | vs_mut | element | main.rs:506:10:506:18 | vs_mut[0] |
| main.rs:507:11:507:16 | vs_mut | &ref | main.rs:507:11:507:16 | receiver for vs_mut |
| main.rs:507:11:507:39 | ... .unwrap() | &ref | main.rs:507:10:507:39 | * ... |
| main.rs:508:11:508:16 | vs_mut | &ref | main.rs:508:11:508:16 | receiver for vs_mut |
| main.rs:508:11:508:39 | ... .unwrap() | &ref | main.rs:508:10:508:39 | * ... |
| main.rs:510:9:510:14 | &mut ... | &ref | main.rs:510:14:510:14 | v |
| main.rs:510:19:510:24 | vs_mut | &ref | main.rs:510:19:510:24 | receiver for vs_mut |
| main.rs:510:19:510:35 | vs_mut.iter_mut() | element | main.rs:510:9:510:14 | &mut ... |
| main.rs:524:11:524:15 | c_ref | &ref | main.rs:524:10:524:15 | * ... |
| main.rs:531:10:531:10 | a | &ref | main.rs:531:10:531:10 | receiver for a |
| main.rs:537:10:537:10 | b | &ref | main.rs:537:10:537:10 | receiver for b |
12 changes: 0 additions & 12 deletions rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,11 @@ edges
| sqlx.rs:52:32:52:87 | ...::must_use(...) | sqlx.rs:52:9:52:20 | safe_query_3 | provenance | |
| sqlx.rs:52:32:52:87 | MacroExpr | sqlx.rs:52:32:52:87 | ...::format(...) | provenance | MaD:4 |
| sqlx.rs:52:32:52:87 | { ... } | sqlx.rs:52:32:52:87 | ...::must_use(...) | provenance | MaD:9 |
| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:63:26:63:39 | unsafe_query_1 [&ref] | provenance | |
| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:63:26:63:48 | unsafe_query_1.as_str() | provenance | MaD:3 |
| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:74:25:74:38 | unsafe_query_1 [&ref] | provenance | |
| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:74:25:74:47 | unsafe_query_1.as_str() | provenance | MaD:3 |
| sqlx.rs:53:26:53:36 | &arg_string [&ref] | sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | provenance | |
| sqlx.rs:53:27:53:36 | arg_string | sqlx.rs:53:26:53:36 | &arg_string [&ref] | provenance | |
| sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | sqlx.rs:65:30:65:43 | unsafe_query_2 [&ref] | provenance | |
| sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | sqlx.rs:65:30:65:52 | unsafe_query_2.as_str() | provenance | MaD:3 |
| sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | sqlx.rs:76:29:76:42 | unsafe_query_2 [&ref] | provenance | |
| sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | sqlx.rs:76:29:76:51 | unsafe_query_2.as_str() | provenance | MaD:3 |
| sqlx.rs:54:26:54:39 | &remote_string [&ref] | sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | provenance | |
| sqlx.rs:54:27:54:39 | remote_string | sqlx.rs:54:26:54:39 | &remote_string [&ref] | provenance | |
Expand All @@ -50,10 +46,6 @@ edges
| sqlx.rs:56:34:56:89 | ...::must_use(...) | sqlx.rs:56:9:56:22 | unsafe_query_4 | provenance | |
| sqlx.rs:56:34:56:89 | MacroExpr | sqlx.rs:56:34:56:89 | ...::format(...) | provenance | MaD:4 |
| sqlx.rs:56:34:56:89 | { ... } | sqlx.rs:56:34:56:89 | ...::must_use(...) | provenance | MaD:9 |
| sqlx.rs:63:26:63:39 | unsafe_query_1 [&ref] | sqlx.rs:63:26:63:48 | unsafe_query_1.as_str() | provenance | MaD:3 |
| sqlx.rs:65:30:65:43 | unsafe_query_2 [&ref] | sqlx.rs:65:30:65:52 | unsafe_query_2.as_str() | provenance | MaD:3 |
| sqlx.rs:74:25:74:38 | unsafe_query_1 [&ref] | sqlx.rs:74:25:74:47 | unsafe_query_1.as_str() | provenance | MaD:3 |
| sqlx.rs:76:29:76:42 | unsafe_query_2 [&ref] | sqlx.rs:76:29:76:51 | unsafe_query_2.as_str() | provenance | MaD:3 |
models
| 1 | Source: lang:std; crate::env::args; commandargs; ReturnValue.Element |
| 2 | Source: repo:https://github.com/seanmonstar/reqwest:reqwest; crate::blocking::get; remote; ReturnValue.Field[crate::result::Result::Ok(0)] |
Expand Down Expand Up @@ -100,15 +92,11 @@ nodes
| sqlx.rs:56:34:56:89 | MacroExpr | semmle.label | MacroExpr |
| sqlx.rs:56:34:56:89 | { ... } | semmle.label | { ... } |
| sqlx.rs:62:26:62:46 | safe_query_3.as_str() | semmle.label | safe_query_3.as_str() |
| sqlx.rs:63:26:63:39 | unsafe_query_1 [&ref] | semmle.label | unsafe_query_1 [&ref] |
| sqlx.rs:63:26:63:48 | unsafe_query_1.as_str() | semmle.label | unsafe_query_1.as_str() |
| sqlx.rs:65:30:65:43 | unsafe_query_2 [&ref] | semmle.label | unsafe_query_2 [&ref] |
| sqlx.rs:65:30:65:52 | unsafe_query_2.as_str() | semmle.label | unsafe_query_2.as_str() |
| sqlx.rs:67:30:67:52 | unsafe_query_4.as_str() | semmle.label | unsafe_query_4.as_str() |
| sqlx.rs:73:25:73:45 | safe_query_3.as_str() | semmle.label | safe_query_3.as_str() |
| sqlx.rs:74:25:74:38 | unsafe_query_1 [&ref] | semmle.label | unsafe_query_1 [&ref] |
| sqlx.rs:74:25:74:47 | unsafe_query_1.as_str() | semmle.label | unsafe_query_1.as_str() |
| sqlx.rs:76:29:76:42 | unsafe_query_2 [&ref] | semmle.label | unsafe_query_2 [&ref] |
| sqlx.rs:76:29:76:51 | unsafe_query_2.as_str() | semmle.label | unsafe_query_2.as_str() |
| sqlx.rs:78:29:78:51 | unsafe_query_4.as_str() | semmle.label | unsafe_query_4.as_str() |
subpaths
8 changes: 0 additions & 8 deletions rust/ql/test/utils-tests/modelgenerator/CaptureSummaryModels.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,4 @@
unexpectedModel
| Unexpected summary found: repo::test;<crate::option::MyOption as crate::clone::Clone>::clone;Argument[self].Field[crate::option::MyOption::MySome(0)].Reference;ReturnValue.Field[crate::option::MyOption::MySome(0)];value;dfc-generated |
| Unexpected summary found: repo::test;<crate::option::MyOption as crate::convert::From>::from;Argument[0].Field[crate::option::MyOption::MySome(0)];ReturnValue.Field[crate::option::MyOption::MySome(0)].Reference;value;dfc-generated |
| Unexpected summary found: repo::test;<crate::option::MyOption>::cloned;Argument[self].Field[crate::option::MyOption::MySome(0)].Reference;ReturnValue.Field[crate::option::MyOption::MySome(0)];value;dfc-generated |
| Unexpected summary found: repo::test;<crate::option::MyOption>::get_or_insert;Argument[0];Argument[self].Field[crate::option::MyOption::MySome(0)];value;dfc-generated |
| Unexpected summary found: repo::test;<crate::option::MyOption>::get_or_insert;Argument[self].Field[crate::option::MyOption::MySome(0)];ReturnValue.Reference;value;dfc-generated |
| Unexpected summary found: repo::test;<crate::option::MyOption>::get_or_insert_default;Argument[self].Field[crate::option::MyOption::MySome(0)];ReturnValue.Reference;value;dfc-generated |
| Unexpected summary found: repo::test;<crate::option::MyOption>::get_or_insert_with;Argument[self].Field[crate::option::MyOption::MySome(0)];ReturnValue.Reference;value;dfc-generated |
| Unexpected summary found: repo::test;<crate::option::MyOption>::insert;Argument[self].Field[crate::option::MyOption::MySome(0)];ReturnValue.Reference;value;dfc-generated |
expectedModel
| Expected summary missing: repo::test;<crate::option::MyOption>::take_if;Argument[self].Reference.Field[crate::option::MyOption::MySome(0)];Argument[0].Parameter[0].Reference;value;dfc-generated |
| Expected summary missing: repo::test;<crate::option::MyOption>::take_if;Argument[self].Reference;ReturnValue;value;dfc-generated |