Skip to content

Commit 5dda9be

Browse files
committed
Rust: Model Deref trait in type inference
1 parent d102195 commit 5dda9be

File tree

14 files changed

+389
-186
lines changed

14 files changed

+389
-186
lines changed

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

Lines changed: 107 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1553,24 +1553,13 @@ private module MethodResolution {
15531553
* Same as `getACandidateReceiverTypeAt`, but without borrows.
15541554
*/
15551555
pragma[nomagic]
1556-
private Type getACandidateReceiverTypeAtNoBorrow(string derefChain, TypePath path) {
1556+
Type getACandidateReceiverTypeAtNoBorrow(string derefChain, TypePath path) {
15571557
result = this.getReceiverTypeAt(path) and
15581558
derefChain = ""
15591559
or
1560-
this.supportsAutoDerefAndBorrow() and
1561-
exists(TypePath path0, Type t0, string derefChain0 |
1562-
this.hasNoCompatibleTargetMutBorrow(derefChain0) and
1563-
t0 = this.getACandidateReceiverTypeAtNoBorrow(derefChain0, path0)
1564-
|
1565-
path0.isCons(getRefTypeParameter(), path) and
1566-
result = t0 and
1567-
derefChain = derefChain0 + ".ref"
1568-
or
1569-
path0.isEmpty() and
1570-
path = path0 and
1571-
t0 = getStringStruct() and
1572-
result = getStrStruct() and
1573-
derefChain = derefChain0 + ".str"
1560+
exists(ImplicitDeref::DerefImplItemNode impl, string derefChain0 |
1561+
result = ImplicitDeref::getDereferencedCandidateReceiverType(this, impl, derefChain0, path) and
1562+
derefChain = derefChain0 + "." + impl.getId()
15741563
)
15751564
}
15761565

@@ -1818,8 +1807,11 @@ private module MethodResolution {
18181807
}
18191808

18201809
predicate receiverHasImplicitDeref(AstNode receiver) {
1821-
exists(this.resolveCallTarget(_, ".ref", TNoBorrowKind())) and
1822-
receiver = this.getArg(any(ArgumentPosition pos | pos.isSelf()))
1810+
exists(ImplicitDeref::DerefImplItemNode impl |
1811+
impl.isRefImpl() and
1812+
exists(this.resolveCallTarget(_, "." + impl.getId(), TNoBorrowKind())) and
1813+
receiver = this.getArg(any(ArgumentPosition pos | pos.isSelf()))
1814+
)
18231815
}
18241816

18251817
predicate argumentHasImplicitBorrow(AstNode arg, BorrowKind borrow) {
@@ -2089,6 +2081,98 @@ private module MethodResolution {
20892081
Location getLocation() { result = mc_.getLocation() }
20902082
}
20912083

2084+
module ImplicitDeref {
2085+
private import codeql.rust.elements.internal.generated.Raw
2086+
private import codeql.rust.elements.internal.generated.Synth
2087+
2088+
/** An `impl` block that implements the `Deref` trait. */
2089+
class DerefImplItemNode extends ImplItemNode {
2090+
DerefImplItemNode() { this.resolveTraitTy() instanceof DerefTrait }
2091+
2092+
/**
2093+
* Holds if this `impl` block is the special `Deref` implementation for
2094+
* `&T` or `&mut T`:
2095+
*
2096+
* ```rust
2097+
* impl<T: ?Sized> const Deref for &T
2098+
* ```
2099+
*
2100+
* or
2101+
*
2102+
* ```rust
2103+
* impl<T: ?Sized> const Deref for &mut T
2104+
* ```
2105+
*/
2106+
predicate isRefImpl() { this.resolveSelfTyBuiltin() instanceof Builtins::RefType }
2107+
2108+
/** Gets an internal unique ID used to identify this block amongst all `Deref` impl blocks. */
2109+
int getId() { idOfRaw(Synth::convertAstNodeToRaw(this), result) }
2110+
}
2111+
2112+
private class DerefImplItemRaw extends Raw::Impl {
2113+
DerefImplItemRaw() { this = Synth::convertAstNodeToRaw(any(DerefImplItemNode i)) }
2114+
}
2115+
2116+
private predicate id(DerefImplItemRaw x, DerefImplItemRaw y) { x = y }
2117+
2118+
private predicate idOfRaw(DerefImplItemRaw x, int y) = equivalenceRelation(id/2)(x, y)
2119+
2120+
private newtype TMethodCallDerefCand =
2121+
MkMethodCallDerefCand(MethodCall mc, string derefChain) {
2122+
mc.supportsAutoDerefAndBorrow() and
2123+
mc.hasNoCompatibleTargetMutBorrow(derefChain) and
2124+
exists(mc.getACandidateReceiverTypeAtNoBorrow(derefChain, _))
2125+
}
2126+
2127+
private class MethodCallDerefCand extends MkMethodCallDerefCand {
2128+
MethodCall mc_;
2129+
string derefChain;
2130+
2131+
MethodCallDerefCand() { this = MkMethodCallDerefCand(mc_, derefChain) }
2132+
2133+
MethodCall getMethodCall() { result = mc_ }
2134+
2135+
Type getTypeAt(TypePath path) {
2136+
result = substituteLookupTraits(mc_.getACandidateReceiverTypeAtNoBorrow(derefChain, path)) and
2137+
not result = TNeverType() and
2138+
not result = TUnknownType()
2139+
}
2140+
2141+
string toString() { result = mc_.toString() + " [" + derefChain + "]" }
2142+
2143+
Location getLocation() { result = mc_.getLocation() }
2144+
}
2145+
2146+
private module MethodCallSatisfiesConstraintInput implements
2147+
SatisfiesConstraintInputSig<MethodCallDerefCand>
2148+
{
2149+
pragma[nomagic]
2150+
predicate relevantConstraint(MethodCallDerefCand mc, Type constraint) {
2151+
exists(mc) and
2152+
constraint.(TraitType).getTrait() instanceof DerefTrait
2153+
}
2154+
2155+
predicate useUniversalConditions() { none() }
2156+
}
2157+
2158+
pragma[nomagic]
2159+
private AssociatedTypeTypeParameter getDerefTargetTypeParameter() {
2160+
result.getTypeAlias() = any(DerefTrait ft).getTargetType()
2161+
}
2162+
2163+
pragma[nomagic]
2164+
Type getDereferencedCandidateReceiverType(
2165+
MethodCall mc, DerefImplItemNode impl, string derefChain, TypePath path
2166+
) {
2167+
exists(MethodCallDerefCand mcc, TypePath exprPath |
2168+
mcc = MkMethodCallDerefCand(mc, derefChain) and
2169+
SatisfiesConstraint<MethodCallDerefCand, MethodCallSatisfiesConstraintInput>::satisfiesConstraintType(mcc,
2170+
impl, _, exprPath, result) and
2171+
exprPath.isCons(getDerefTargetTypeParameter(), path)
2172+
)
2173+
}
2174+
}
2175+
20922176
private module ReceiverSatisfiesBlanketLikeConstraintInput implements
20932177
BlanketImplementation::SatisfiesBlanketConstraintInputSig<MethodCallCand>
20942178
{
@@ -2444,9 +2528,12 @@ private Type inferMethodCallType1(AstNode n, boolean isReturn, TypePath path) {
24442528
path = path0
24452529
or
24462530
// adjust for implicit deref
2447-
apos.isSelf() and
2448-
derefChainBorrow = ".ref;" and
2449-
path = TypePath::cons(getRefTypeParameter(), path0)
2531+
exists(MethodResolution::ImplicitDeref::DerefImplItemNode impl |
2532+
impl.isRefImpl() and
2533+
apos.isSelf() and
2534+
derefChainBorrow = "." + impl.getId() + ";" and
2535+
path = TypePath::cons(getRefTypeParameter(), path0)
2536+
)
24502537
or
24512538
// adjust for implicit borrow
24522539
apos.isSelf() and
@@ -3292,9 +3379,6 @@ private Type inferTryExprType(TryExpr te, TypePath path) {
32923379
pragma[nomagic]
32933380
private StructType getStrStruct() { result = TStruct(any(Builtins::Str s)) }
32943381

3295-
pragma[nomagic]
3296-
private StructType getStringStruct() { result = TStruct(any(StringStruct s)) }
3297-
32983382
pragma[nomagic]
32993383
private Type inferLiteralType(LiteralExpr le, TypePath path, boolean certain) {
33003384
path.isEmpty() and

rust/ql/test/library-tests/dataflow/sources/file/InlineFlow.expected

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ models
3535
| 34 | Summary: <_ as tokio::io::util::async_read_ext::AsyncReadExt>::read_to_string; Argument[self].Reference; Argument[0].Reference; taint |
3636
| 35 | Summary: <_ as tokio::io::util::async_read_ext::AsyncReadExt>::read_u8; Argument[self].Reference; ReturnValue.Future.Field[core::result::Result::Ok(0)]; taint |
3737
| 36 | Summary: <core::result::Result>::unwrap; Argument[self].Field[core::result::Result::Ok(0)]; ReturnValue; value |
38-
| 37 | Summary: <std::path::PathBuf>::as_path; Argument[self]; ReturnValue; value |
38+
| 37 | Summary: <std::path::Path>::canonicalize; Argument[self].Reference.OptionalBarrier[normalize-path]; ReturnValue.Field[core::result::Result::Ok(0)]; taint |
39+
| 38 | Summary: <std::path::PathBuf>::as_path; Argument[self]; ReturnValue; value |
3940
edges
4041
| test.rs:12:13:12:18 | buffer | test.rs:13:14:13:19 | buffer | provenance | |
4142
| test.rs:12:31:12:43 | ...::read | test.rs:12:31:12:55 | ...::read(...) [Ok] | provenance | Src:MaD:11 |
@@ -51,12 +52,15 @@ edges
5152
| test.rs:22:22:22:52 | TryExpr | test.rs:22:13:22:18 | buffer | provenance | |
5253
| test.rs:29:13:29:16 | path | test.rs:30:14:30:17 | path | provenance | |
5354
| test.rs:29:13:29:16 | path | test.rs:31:14:31:17 | path | provenance | |
55+
| test.rs:29:13:29:16 | path | test.rs:40:14:40:17 | path | provenance | |
5456
| test.rs:29:13:29:16 | path | test.rs:41:14:41:17 | path | provenance | |
5557
| test.rs:29:20:29:27 | e.path() | test.rs:29:13:29:16 | path | provenance | |
5658
| test.rs:29:22:29:25 | path | test.rs:29:20:29:27 | e.path() | provenance | Src:MaD:4 MaD:4 |
5759
| test.rs:30:14:30:17 | path | test.rs:30:14:30:25 | path.clone() | provenance | MaD:18 |
5860
| test.rs:31:14:31:17 | path | test.rs:31:14:31:25 | path.clone() | provenance | MaD:18 |
59-
| test.rs:31:14:31:25 | path.clone() | test.rs:31:14:31:35 | ... .as_path() | provenance | MaD:37 |
61+
| test.rs:31:14:31:25 | path.clone() | test.rs:31:14:31:35 | ... .as_path() | provenance | MaD:38 |
62+
| test.rs:40:14:40:17 | path | test.rs:40:14:40:32 | path.canonicalize() [Ok] | provenance | MaD:37 |
63+
| test.rs:40:14:40:32 | path.canonicalize() [Ok] | test.rs:40:14:40:41 | ... .unwrap() | provenance | MaD:36 |
6064
| test.rs:43:13:43:21 | file_name | test.rs:44:14:44:22 | file_name | provenance | |
6165
| test.rs:43:13:43:21 | file_name | test.rs:49:14:49:22 | file_name | provenance | |
6266
| test.rs:43:25:43:37 | e.file_name() | test.rs:43:13:43:21 | file_name | provenance | |
@@ -273,6 +277,9 @@ nodes
273277
| test.rs:31:14:31:17 | path | semmle.label | path |
274278
| test.rs:31:14:31:25 | path.clone() | semmle.label | path.clone() |
275279
| test.rs:31:14:31:35 | ... .as_path() | semmle.label | ... .as_path() |
280+
| test.rs:40:14:40:17 | path | semmle.label | path |
281+
| test.rs:40:14:40:32 | path.canonicalize() [Ok] | semmle.label | path.canonicalize() [Ok] |
282+
| test.rs:40:14:40:41 | ... .unwrap() | semmle.label | ... .unwrap() |
276283
| test.rs:41:14:41:17 | path | semmle.label | path |
277284
| test.rs:43:13:43:21 | file_name | semmle.label | file_name |
278285
| test.rs:43:25:43:37 | e.file_name() | semmle.label | e.file_name() |
@@ -492,6 +499,7 @@ testFailures
492499
| test.rs:23:14:23:19 | buffer | test.rs:22:22:22:39 | ...::read_to_string | test.rs:23:14:23:19 | buffer | $@ | test.rs:22:22:22:39 | ...::read_to_string | ...::read_to_string |
493500
| test.rs:30:14:30:25 | path.clone() | test.rs:29:22:29:25 | path | test.rs:30:14:30:25 | path.clone() | $@ | test.rs:29:22:29:25 | path | path |
494501
| test.rs:31:14:31:35 | ... .as_path() | test.rs:29:22:29:25 | path | test.rs:31:14:31:35 | ... .as_path() | $@ | test.rs:29:22:29:25 | path | path |
502+
| test.rs:40:14:40:41 | ... .unwrap() | test.rs:29:22:29:25 | path | test.rs:40:14:40:41 | ... .unwrap() | $@ | test.rs:29:22:29:25 | path | path |
495503
| test.rs:41:14:41:17 | path | test.rs:29:22:29:25 | path | test.rs:41:14:41:17 | path | $@ | test.rs:29:22:29:25 | path | path |
496504
| test.rs:44:14:44:30 | file_name.clone() | test.rs:43:27:43:35 | file_name | test.rs:44:14:44:30 | file_name.clone() | $@ | test.rs:43:27:43:35 | file_name | file_name |
497505
| test.rs:49:14:49:22 | file_name | test.rs:43:27:43:35 | file_name | test.rs:49:14:49:22 | file_name | $@ | test.rs:43:27:43:35 | file_name | file_name |

rust/ql/test/library-tests/dataflow/sources/file/TaintSources.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
| test.rs:51:52:51:59 | read_dir | Flow source 'FileSource' of type file (DEFAULT). |
88
| test.rs:54:22:54:25 | path | Flow source 'FileSource' of type file (DEFAULT). |
99
| test.rs:55:27:55:35 | file_name | Flow source 'FileSource' of type file (DEFAULT). |
10+
| test.rs:57:56:57:63 | read_dir | Flow source 'FileSource' of type file (DEFAULT). |
11+
| test.rs:60:22:60:25 | path | Flow source 'FileSource' of type file (DEFAULT). |
12+
| test.rs:61:27:61:35 | file_name | Flow source 'FileSource' of type file (DEFAULT). |
1013
| test.rs:65:22:65:34 | ...::read_link | Flow source 'FileSource' of type file (DEFAULT). |
1114
| test.rs:74:31:74:45 | ...::read | Flow source 'FileSource' of type file (DEFAULT). |
1215
| test.rs:79:31:79:45 | ...::read | Flow source 'FileSource' of type file (DEFAULT). |

rust/ql/test/library-tests/dataflow/sources/file/test.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ fn test_fs() -> Result<(), Box<dyn std::error::Error>> {
3737
sink(path.to_path_buf()); // $ MISSING: hasTaintFlow
3838
sink(path.file_name().unwrap()); // $ MISSING: hasTaintFlow
3939
sink(path.extension().unwrap()); // $ MISSING: hasTaintFlow
40-
sink(path.canonicalize().unwrap()); // $ MISSING: hasTaintFlow
40+
sink(path.canonicalize().unwrap()); // $ hasTaintFlow
4141
sink(path); // $ hasTaintFlow
4242

4343
let file_name = e.file_name(); // $ Alert[rust/summary/taint-sources]
@@ -54,11 +54,11 @@ fn test_fs() -> Result<(), Box<dyn std::error::Error>> {
5454
let path = e.path(); // $ Alert[rust/summary/taint-sources]
5555
let file_name = e.file_name(); // $ Alert[rust/summary/taint-sources]
5656
}
57-
for entry in std::path::PathBuf::from("directory").read_dir()? {
57+
for entry in std::path::PathBuf::from("directory").read_dir()? { // $ Alert[rust/summary/taint-sources]
5858
let e = entry?;
5959

60-
let path = e.path(); // $ MISSING: Alert[rust/summary/taint-sources]
61-
let file_name = e.file_name(); // $ MISSING: Alert[rust/summary/taint-sources]
60+
let path = e.path(); // $ Alert[rust/summary/taint-sources]
61+
let file_name = e.file_name(); // $ Alert[rust/summary/taint-sources]
6262
}
6363

6464
{

0 commit comments

Comments
 (0)