Skip to content

Commit c1e2197

Browse files
paldepindpaldepind
committed
Rust: Address review comments
1 parent d8c301a commit c1e2197

File tree

15 files changed

+157
-78
lines changed

15 files changed

+157
-78
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,7 @@ private class CapturedVariableContent extends Content, TCapturedVariableContent
712712
override string toString() { result = "captured " + v }
713713
}
714714

715-
/** A value refered to by a reference. */
715+
/** A value referred to by a reference. */
716716
final class ReferenceContent extends Content, TReferenceContent {
717717
override string toString() { result = "&ref" }
718718
}

‎rust/ql/lib/codeql/rust/dataflow/internal/SsaImpl.qll

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -88,22 +88,16 @@ module SsaInput implements SsaImplCommon::InputSig<Location> {
8888
|
8989
va instanceof VariableReadAccess
9090
or
91+
// For immutable variables, we model a read when they are borrowed
92+
// (although the actual read happens later, if at all).
93+
va = any(RefExpr re).getExpr()
94+
or
9195
// Although compound assignments, like `x += y`, may in fact not read `x`,
9296
// it makes sense to treat them as such
9397
va = any(CompoundAssignmentExpr cae).getLhs()
9498
) and
9599
certain = true
96100
or
97-
// For immutable variables, we model a read when they are borrowed (although the
98-
// actual read happens later, if at all). This only affects the SSA liveness
99-
// analysis.
100-
exists(VariableAccess va |
101-
va = any(RefExpr re).getExpr() and
102-
va = bb.getNode(i).getAstNode() and
103-
v = va.getVariable() and
104-
certain = false
105-
)
106-
or
107101
capturedCallRead(_, bb, i, v) and certain = false
108102
or
109103
capturedExitRead(bb, i, v) and certain = false
@@ -146,7 +140,9 @@ private predicate adjacentDefReadExt(
146140

147141
/** Holds if `v` is read at index `i` in basic block `bb`. */
148142
private predicate variableReadActual(BasicBlock bb, int i, Variable v) {
149-
exists(VariableReadAccess read |
143+
exists(VariableAccess read |
144+
read instanceof VariableReadAccess or read = any(RefExpr re).getExpr()
145+
|
150146
read.getVariable() = v and
151147
read = bb.getNode(i).getAstNode()
152148
)

‎rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,7 @@ module Impl {
484484
class VariableReadAccess extends VariableAccess {
485485
VariableReadAccess() {
486486
not this instanceof VariableWriteAccess and
487+
not this = any(RefExpr re).getExpr() and
487488
not this = any(CompoundAssignmentExpr cae).getLhs()
488489
}
489490
}

‎rust/ql/lib/codeql/rust/frameworks/reqwest.model.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ extensions:
33
pack: codeql/rust-all
44
extensible: summaryModel
55
data:
6-
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::blocking::response::Response>::text", "Argument[self]", "ReturnValue", "taint", "manual"]
6+
- ["repo:https://github.com/seanmonstar/reqwest:reqwest", "<crate::blocking::response::Response>::text", "Argument[self]", "ReturnValue.Variant[crate::result::Result::Ok(0)]", "taint", "manual"]

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ extensions:
88
- ["lang:core", "<crate::option::Option>::unwrap", "Argument[self]", "ReturnValue", "taint", "manual"]
99
- ["lang:core", "<crate::option::Option>::unwrap_or", "Argument[self].Variant[crate::option::Option::Some(0)]", "ReturnValue", "value", "manual"]
1010
- ["lang:core", "<crate::option::Option>::unwrap_or", "Argument[0]", "ReturnValue", "value", "manual"]
11-
- ["lang:core", "<crate::option::Option>::unwrap_or", "Argument[self]", "ReturnValue", "taint", "manual"]
1211
# Result
1312
- ["lang:core", "<crate::result::Result>::unwrap", "Argument[self].Variant[crate::result::Result::Ok(0)]", "ReturnValue", "value", "manual"]
1413
- ["lang:core", "<crate::result::Result>::unwrap", "Argument[self]", "ReturnValue", "taint", "manual"]

‎rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,7 @@ localStep
458458
| main.rs:398:7:398:14 | [SSA] [input] SSA phi read(default_name) | main.rs:394:7:394:18 | [SSA] SSA phi read(default_name) |
459459
| main.rs:425:13:425:33 | result_questionmark(...) | main.rs:425:9:425:9 | _ |
460460
storeStep
461+
| file://:0:0:0:0 | [summary] to write: ReturnValue.Variant[crate::result::Result::Ok(0)] in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::blocking::response::Response>::text | Ok | file://:0:0:0:0 | [summary] to write: ReturnValue in repo:https://github.com/seanmonstar/reqwest:reqwest::_::<crate::blocking::response::Response>::text |
461462
| main.rs:94:14:94:22 | source(...) | tuple.0 | main.rs:94:13:94:26 | TupleExpr |
462463
| main.rs:94:25:94:25 | 2 | tuple.1 | main.rs:94:13:94:26 | TupleExpr |
463464
| main.rs:100:14:100:14 | 2 | tuple.0 | main.rs:100:13:100:30 | TupleExpr |

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

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,45 @@
1-
ERROR: could not resolve module DefaultFlowTest (inline-flow.ql:7,8-23)
2-
ERROR: could not resolve module ValueFlow (inline-flow.ql:8,8-17)
3-
ERROR: could not resolve module ValueFlow (inline-flow.ql:10,6-15)
4-
ERROR: could not resolve module ValueFlow (inline-flow.ql:10,34-43)
5-
ERROR: could not resolve module ValueFlow (inline-flow.ql:11,7-16)
6-
ERROR: could not resolve module utils.InlineFlowTest (inline-flow.ql:6,8-28)
1+
models
2+
edges
3+
| main.rs:13:9:13:9 | a | main.rs:14:14:14:14 | a | provenance | |
4+
| main.rs:13:13:13:22 | source(...) | main.rs:13:9:13:9 | a | provenance | |
5+
| main.rs:14:9:14:9 | b [&ref] | main.rs:15:14:15:14 | b [&ref] | provenance | |
6+
| main.rs:14:13:14:14 | &a [&ref] | main.rs:14:9:14:9 | b [&ref] | provenance | |
7+
| main.rs:14:14:14:14 | a | main.rs:14:13:14:14 | &a [&ref] | provenance | |
8+
| main.rs:15:9:15:9 | c | main.rs:16:10:16:10 | c | provenance | |
9+
| main.rs:15:13:15:14 | * ... | main.rs:15:9:15:9 | c | provenance | |
10+
| main.rs:15:14:15:14 | b [&ref] | main.rs:15:13:15:14 | * ... | provenance | |
11+
| main.rs:40:18:40:21 | SelfParam [MyNumber] | main.rs:41:15:41:18 | self [MyNumber] | provenance | |
12+
| main.rs:41:15:41:18 | self [MyNumber] | main.rs:42:13:42:38 | ...::MyNumber(...) [MyNumber] | provenance | |
13+
| main.rs:42:13:42:38 | ...::MyNumber(...) [MyNumber] | main.rs:42:32:42:37 | number | provenance | |
14+
| main.rs:42:32:42:37 | number | main.rs:40:31:46:5 | { ... } | provenance | |
15+
| main.rs:58:9:58:17 | my_number [MyNumber] | main.rs:59:10:59:18 | my_number [MyNumber] | provenance | |
16+
| main.rs:58:21:58:50 | ...::MyNumber(...) [MyNumber] | main.rs:58:9:58:17 | my_number [MyNumber] | provenance | |
17+
| main.rs:58:40:58:49 | source(...) | main.rs:58:21:58:50 | ...::MyNumber(...) [MyNumber] | provenance | |
18+
| main.rs:59:10:59:18 | my_number [MyNumber] | main.rs:40:18:40:21 | SelfParam [MyNumber] | provenance | |
19+
| main.rs:59:10:59:18 | my_number [MyNumber] | main.rs:59:10:59:30 | my_number.to_number(...) | provenance | |
20+
nodes
21+
| main.rs:13:9:13:9 | a | semmle.label | a |
22+
| main.rs:13:13:13:22 | source(...) | semmle.label | source(...) |
23+
| main.rs:14:9:14:9 | b [&ref] | semmle.label | b [&ref] |
24+
| main.rs:14:13:14:14 | &a [&ref] | semmle.label | &a [&ref] |
25+
| main.rs:14:14:14:14 | a | semmle.label | a |
26+
| main.rs:15:9:15:9 | c | semmle.label | c |
27+
| main.rs:15:13:15:14 | * ... | semmle.label | * ... |
28+
| main.rs:15:14:15:14 | b [&ref] | semmle.label | b [&ref] |
29+
| main.rs:16:10:16:10 | c | semmle.label | c |
30+
| main.rs:40:18:40:21 | SelfParam [MyNumber] | semmle.label | SelfParam [MyNumber] |
31+
| main.rs:40:31:46:5 | { ... } | semmle.label | { ... } |
32+
| main.rs:41:15:41:18 | self [MyNumber] | semmle.label | self [MyNumber] |
33+
| main.rs:42:13:42:38 | ...::MyNumber(...) [MyNumber] | semmle.label | ...::MyNumber(...) [MyNumber] |
34+
| main.rs:42:32:42:37 | number | semmle.label | number |
35+
| main.rs:58:9:58:17 | my_number [MyNumber] | semmle.label | my_number [MyNumber] |
36+
| main.rs:58:21:58:50 | ...::MyNumber(...) [MyNumber] | semmle.label | ...::MyNumber(...) [MyNumber] |
37+
| main.rs:58:40:58:49 | source(...) | semmle.label | source(...) |
38+
| main.rs:59:10:59:18 | my_number [MyNumber] | semmle.label | my_number [MyNumber] |
39+
| main.rs:59:10:59:30 | my_number.to_number(...) | semmle.label | my_number.to_number(...) |
40+
subpaths
41+
| main.rs:59:10:59:18 | my_number [MyNumber] | main.rs:40:18:40:21 | SelfParam [MyNumber] | main.rs:40:31:46:5 | { ... } | main.rs:59:10:59:30 | my_number.to_number(...) |
42+
testFailures
43+
#select
44+
| main.rs:16:10:16:10 | c | main.rs:13:13:13:22 | source(...) | main.rs:16:10:16:10 | c | $@ | main.rs:13:13:13:22 | source(...) | source(...) |
45+
| main.rs:59:10:59:30 | my_number.to_number(...) | main.rs:58:40:58:49 | source(...) | main.rs:59:10:59:30 | my_number.to_number(...) | $@ | main.rs:58:40:58:49 | source(...) | source(...) |

‎rust/ql/test/library-tests/dataflow/pointers/inline-flow.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44

55
import rust
6-
import utils.InlineFlowTest
6+
import utils.test.InlineFlowTest
77
import DefaultFlowTest
88
import ValueFlow::PathGraph
99

‎rust/ql/test/library-tests/dataflow/strings/inline-taint-flow.expected

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,44 @@
1-
ERROR: could not resolve module DefaultFlowTest (inline-taint-flow.ql:7,8-23)
2-
ERROR: could not resolve module TaintFlow (inline-taint-flow.ql:8,8-17)
3-
ERROR: could not resolve module TaintFlow (inline-taint-flow.ql:10,6-15)
4-
ERROR: could not resolve module TaintFlow (inline-taint-flow.ql:10,34-43)
5-
ERROR: could not resolve module TaintFlow (inline-taint-flow.ql:11,7-16)
6-
ERROR: could not resolve module utils.InlineFlowTest (inline-taint-flow.ql:6,8-28)
1+
models
2+
| 1 | Summary: lang:alloc; <crate::string::String>::as_str; Argument[self]; ReturnValue; taint |
3+
edges
4+
| main.rs:20:9:20:9 | s | main.rs:21:9:21:14 | sliced | provenance | |
5+
| main.rs:20:9:20:9 | s | main.rs:21:19:21:25 | s[...] | provenance | |
6+
| main.rs:20:13:20:22 | source(...) | main.rs:20:9:20:9 | s | provenance | |
7+
| main.rs:21:9:21:14 | sliced | main.rs:22:16:22:21 | sliced | provenance | |
8+
| main.rs:21:9:21:14 | sliced [&ref] | main.rs:22:16:22:21 | sliced | provenance | |
9+
| main.rs:21:18:21:25 | &... [&ref] | main.rs:21:9:21:14 | sliced [&ref] | provenance | |
10+
| main.rs:21:19:21:25 | s[...] | main.rs:21:18:21:25 | &... [&ref] | provenance | |
11+
| main.rs:26:9:26:10 | s1 | main.rs:29:9:29:10 | s4 | provenance | |
12+
| main.rs:26:14:26:23 | source(...) | main.rs:26:9:26:10 | s1 | provenance | |
13+
| main.rs:29:9:29:10 | s4 | main.rs:32:10:32:11 | s4 | provenance | |
14+
| main.rs:37:9:37:10 | s1 | main.rs:40:10:40:35 | ... + ... | provenance | |
15+
| main.rs:37:14:37:23 | source(...) | main.rs:37:9:37:10 | s1 | provenance | |
16+
| main.rs:57:9:57:9 | s | main.rs:58:16:58:16 | s | provenance | |
17+
| main.rs:57:13:57:22 | source(...) | main.rs:57:9:57:9 | s | provenance | |
18+
| main.rs:58:16:58:16 | s | main.rs:58:16:58:25 | s.as_str(...) | provenance | MaD:1 |
19+
nodes
20+
| main.rs:20:9:20:9 | s | semmle.label | s |
21+
| main.rs:20:13:20:22 | source(...) | semmle.label | source(...) |
22+
| main.rs:21:9:21:14 | sliced | semmle.label | sliced |
23+
| main.rs:21:9:21:14 | sliced [&ref] | semmle.label | sliced [&ref] |
24+
| main.rs:21:18:21:25 | &... [&ref] | semmle.label | &... [&ref] |
25+
| main.rs:21:19:21:25 | s[...] | semmle.label | s[...] |
26+
| main.rs:22:16:22:21 | sliced | semmle.label | sliced |
27+
| main.rs:26:9:26:10 | s1 | semmle.label | s1 |
28+
| main.rs:26:14:26:23 | source(...) | semmle.label | source(...) |
29+
| main.rs:29:9:29:10 | s4 | semmle.label | s4 |
30+
| main.rs:32:10:32:11 | s4 | semmle.label | s4 |
31+
| main.rs:37:9:37:10 | s1 | semmle.label | s1 |
32+
| main.rs:37:14:37:23 | source(...) | semmle.label | source(...) |
33+
| main.rs:40:10:40:35 | ... + ... | semmle.label | ... + ... |
34+
| main.rs:57:9:57:9 | s | semmle.label | s |
35+
| main.rs:57:13:57:22 | source(...) | semmle.label | source(...) |
36+
| main.rs:58:16:58:16 | s | semmle.label | s |
37+
| main.rs:58:16:58:25 | s.as_str(...) | semmle.label | s.as_str(...) |
38+
subpaths
39+
testFailures
40+
#select
41+
| main.rs:22:16:22:21 | sliced | main.rs:20:13:20:22 | source(...) | main.rs:22:16:22:21 | sliced | $@ | main.rs:20:13:20:22 | source(...) | source(...) |
42+
| main.rs:32:10:32:11 | s4 | main.rs:26:14:26:23 | source(...) | main.rs:32:10:32:11 | s4 | $@ | main.rs:26:14:26:23 | source(...) | source(...) |
43+
| main.rs:40:10:40:35 | ... + ... | main.rs:37:14:37:23 | source(...) | main.rs:40:10:40:35 | ... + ... | $@ | main.rs:37:14:37:23 | source(...) | source(...) |
44+
| main.rs:58:16:58:25 | s.as_str(...) | main.rs:57:13:57:22 | source(...) | main.rs:58:16:58:25 | s.as_str(...) | $@ | main.rs:57:13:57:22 | source(...) | source(...) |

‎rust/ql/test/library-tests/dataflow/strings/inline-taint-flow.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44

55
import rust
6-
import utils.InlineFlowTest
6+
import utils.test.InlineFlowTest
77
import DefaultFlowTest
88
import TaintFlow::PathGraph
99

0 commit comments

Comments
 (0)