Rust: Fix data flow through callbacks passed to library functions

This commit is contained in:
Tom Hvitved
2025-01-28 13:44:27 +01:00
parent f2564c351f
commit 8b82eaa633
5 changed files with 56 additions and 51 deletions

View File

@@ -49,16 +49,12 @@ final class DataFlowCallable extends TDataFlowCallable {
}
final class DataFlowCall extends TDataFlowCall {
private CallExprBaseCfgNode call;
DataFlowCall() { this = TCall(call) }
/** Gets the underlying call in the CFG, if any. */
CallExprCfgNode asCallExprCfgNode() { result = call }
CallExprCfgNode asCallExprCfgNode() { result = this.asCallBaseExprCfgNode() }
MethodCallExprCfgNode asMethodCallExprCfgNode() { result = call }
MethodCallExprCfgNode asMethodCallExprCfgNode() { result = this.asCallBaseExprCfgNode() }
CallExprBaseCfgNode asCallBaseExprCfgNode() { result = call }
CallExprBaseCfgNode asCallBaseExprCfgNode() { this = TCall(result) }
predicate isSummaryCall(
FlowSummaryImpl::Public::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNode receiver
@@ -67,7 +63,7 @@ final class DataFlowCall extends TDataFlowCall {
}
DataFlowCallable getEnclosingCallable() {
result = TCfgScope(call.getExpr().getEnclosingCfgScope())
result = TCfgScope(this.asCallBaseExprCfgNode().getExpr().getEnclosingCfgScope())
or
exists(FlowSummaryImpl::Public::SummarizedCallable c |
this.isSummaryCall(c, _) and
@@ -1298,10 +1294,14 @@ module RustDataFlow implements InputSig<Location> {
* invoked expression.
*/
predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) {
receiver.asExpr() = call.asCallExprCfgNode().getFunction() and
// All calls to complex expressions and local variable accesses are lambda call.
exists(Expr f | f = receiver.asExpr().getExpr() |
f instanceof PathExpr implies f = any(Variable v).getAnAccess()
(
receiver.asExpr() = call.asCallExprCfgNode().getFunction() and
// All calls to complex expressions and local variable accesses are lambda call.
exists(Expr f | f = receiver.asExpr().getExpr() |
f instanceof PathExpr implies f = any(Variable v).getAnAccess()
)
or
call.isSummaryCall(_, receiver.(Node::FlowSummaryNode).getSummaryNode())
) and
exists(kind)
}

View File

@@ -1,2 +1,2 @@
identityLocalStep
| main.rs:412:7:412:18 | phi(default_name) | Node steps to itself |
| main.rs:412:9:412:20 | phi(default_name) | Node steps to itself |

View File

@@ -471,26 +471,26 @@ localStep
| main.rs:403:13:403:19 | [post] mut_arr | main.rs:405:10:405:16 | mut_arr | |
| main.rs:403:13:403:19 | mut_arr | main.rs:405:10:405:16 | mut_arr | |
| main.rs:403:13:403:22 | mut_arr[1] | main.rs:403:9:403:9 | d | |
| main.rs:410:39:410:43 | [SSA] names | main.rs:412:23:412:27 | names | |
| main.rs:410:39:410:43 | [SSA] names | main.rs:412:25:412:29 | names | |
| main.rs:410:39:410:43 | names | main.rs:410:39:410:43 | [SSA] names | |
| main.rs:410:39:410:72 | ...: Vec::<...> | main.rs:410:39:410:43 | names | |
| main.rs:411:7:411:18 | default_name | main.rs:411:7:411:18 | [SSA] default_name | |
| main.rs:411:22:411:43 | ... .to_string(...) | main.rs:411:7:411:18 | default_name | |
| main.rs:411:22:411:43 | ... .to_string(...) | main.rs:412:7:412:18 | phi(default_name) | |
| main.rs:412:3:418:3 | for ... in ... { ... } | main.rs:410:75:419:1 | { ... } | |
| main.rs:412:7:412:18 | phi(default_name) | main.rs:412:7:412:18 | phi(default_name) | |
| main.rs:412:7:412:18 | phi(default_name) | main.rs:414:35:414:61 | default_name | |
| main.rs:412:8:412:11 | [SSA] cond | main.rs:413:8:413:11 | cond | |
| main.rs:412:8:412:11 | cond | main.rs:412:8:412:11 | [SSA] cond | |
| main.rs:412:14:412:17 | [SSA] name | main.rs:414:15:414:18 | name | |
| main.rs:412:14:412:17 | name | main.rs:412:14:412:17 | [SSA] name | |
| main.rs:413:5:417:5 | if cond {...} | main.rs:412:29:418:3 | { ... } | |
| main.rs:414:11:414:11 | [SSA] n | main.rs:415:12:415:12 | n | |
| main.rs:414:11:414:11 | n | main.rs:414:11:414:11 | [SSA] n | |
| main.rs:414:15:414:62 | name.unwrap_or_else(...) | main.rs:414:11:414:11 | n | |
| main.rs:414:35:414:61 | [post] default_name | main.rs:412:7:412:18 | phi(default_name) | |
| main.rs:414:35:414:61 | closure self in \|...\| ... | main.rs:414:38:414:49 | this | |
| main.rs:414:35:414:61 | default_name | main.rs:412:7:412:18 | phi(default_name) | |
| main.rs:411:9:411:20 | default_name | main.rs:411:9:411:20 | [SSA] default_name | |
| main.rs:411:24:411:45 | ... .to_string(...) | main.rs:411:9:411:20 | default_name | |
| main.rs:411:24:411:45 | ... .to_string(...) | main.rs:412:9:412:20 | phi(default_name) | |
| main.rs:412:5:418:5 | for ... in ... { ... } | main.rs:410:75:419:1 | { ... } | |
| main.rs:412:9:412:20 | phi(default_name) | main.rs:412:9:412:20 | phi(default_name) | |
| main.rs:412:9:412:20 | phi(default_name) | main.rs:414:41:414:67 | default_name | |
| main.rs:412:10:412:13 | [SSA] cond | main.rs:413:12:413:15 | cond | |
| main.rs:412:10:412:13 | cond | main.rs:412:10:412:13 | [SSA] cond | |
| main.rs:412:16:412:19 | [SSA] name | main.rs:414:21:414:24 | name | |
| main.rs:412:16:412:19 | name | main.rs:412:16:412:19 | [SSA] name | |
| main.rs:413:9:417:9 | if cond {...} | main.rs:412:31:418:5 | { ... } | |
| main.rs:414:17:414:17 | [SSA] n | main.rs:415:18:415:18 | n | |
| main.rs:414:17:414:17 | n | main.rs:414:17:414:17 | [SSA] n | |
| main.rs:414:21:414:68 | name.unwrap_or_else(...) | main.rs:414:17:414:17 | n | |
| main.rs:414:41:414:67 | [post] default_name | main.rs:412:9:412:20 | phi(default_name) | |
| main.rs:414:41:414:67 | closure self in \|...\| ... | main.rs:414:44:414:55 | this | |
| main.rs:414:41:414:67 | default_name | main.rs:412:9:412:20 | phi(default_name) | |
| main.rs:428:9:428:9 | [SSA] s | main.rs:429:10:429:10 | s | |
| main.rs:428:9:428:9 | s | main.rs:428:9:428:9 | [SSA] s | |
| main.rs:428:13:428:27 | MacroExpr | main.rs:428:9:428:9 | s | |
@@ -600,7 +600,7 @@ storeStep
| main.rs:399:27:399:27 | 2 | element | main.rs:399:23:399:31 | [...] |
| main.rs:399:30:399:30 | 3 | element | main.rs:399:23:399:31 | [...] |
| main.rs:402:18:402:27 | source(...) | element | main.rs:402:5:402:11 | [post] mut_arr |
| main.rs:414:35:414:61 | default_name | captured default_name | main.rs:414:35:414:61 | \|...\| ... |
| main.rs:414:41:414:67 | default_name | captured default_name | main.rs:414:41:414:67 | \|...\| ... |
| main.rs:436:27:436:27 | 0 | Some | main.rs:436:22:436:28 | Some(...) |
readStep
| file://:0:0:0:0 | [summary param] self in lang:core::_::<crate::option::Option>::expect | Some | file://:0:0:0:0 | [summary] read: Argument[self].Variant[crate::option::Option::Some(0)] in lang:core::_::<crate::option::Option>::expect |
@@ -689,8 +689,8 @@ readStep
| main.rs:402:5:402:11 | mut_arr | element | main.rs:402:5:402:14 | mut_arr[1] |
| main.rs:403:13:403:19 | mut_arr | element | main.rs:403:13:403:22 | mut_arr[1] |
| main.rs:405:10:405:16 | mut_arr | element | main.rs:405:10:405:19 | mut_arr[0] |
| main.rs:412:7:412:18 | TuplePat | tuple.0 | main.rs:412:8:412:11 | cond |
| main.rs:412:7:412:18 | TuplePat | tuple.1 | main.rs:412:14:412:17 | name |
| main.rs:412:23:412:27 | names | element | main.rs:412:7:412:18 | TuplePat |
| main.rs:414:35:414:61 | [post] \|...\| ... | captured default_name | main.rs:414:35:414:61 | [post] default_name |
| main.rs:414:38:414:49 | this | captured default_name | main.rs:414:38:414:49 | default_name |
| main.rs:412:9:412:20 | TuplePat | tuple.0 | main.rs:412:10:412:13 | cond |
| main.rs:412:9:412:20 | TuplePat | tuple.1 | main.rs:412:16:412:19 | name |
| main.rs:412:25:412:29 | names | element | main.rs:412:9:412:20 | TuplePat |
| main.rs:414:41:414:67 | [post] \|...\| ... | captured default_name | main.rs:414:41:414:67 | [post] default_name |
| main.rs:414:44:414:55 | this | captured default_name | main.rs:414:44:414:55 | default_name |

View File

@@ -2,9 +2,10 @@ models
| 1 | Summary: lang:core; <crate::option::Option>::unwrap; Argument[self].Variant[crate::option::Option::Some(0)]; ReturnValue; value |
| 2 | Summary: lang:core; <crate::option::Option>::unwrap_or; Argument[0]; ReturnValue; value |
| 3 | Summary: lang:core; <crate::option::Option>::unwrap_or; Argument[self].Variant[crate::option::Option::Some(0)]; ReturnValue; value |
| 4 | Summary: lang:core; <crate::option::Option>::unwrap_or_else; Argument[self].Variant[crate::option::Option::Some(0)]; ReturnValue; value |
| 5 | Summary: lang:core; <crate::result::Result>::expect; Argument[self].Variant[crate::result::Result::Ok(0)]; ReturnValue; value |
| 6 | Summary: lang:core; <crate::result::Result>::expect_err; Argument[self].Variant[crate::result::Result::Err(0)]; ReturnValue; value |
| 4 | Summary: lang:core; <crate::option::Option>::unwrap_or_else; Argument[0].ReturnValue; ReturnValue; value |
| 5 | Summary: lang:core; <crate::option::Option>::unwrap_or_else; Argument[self].Variant[crate::option::Option::Some(0)]; ReturnValue; value |
| 6 | Summary: lang:core; <crate::result::Result>::expect; Argument[self].Variant[crate::result::Result::Ok(0)]; ReturnValue; value |
| 7 | Summary: lang:core; <crate::result::Result>::expect_err; Argument[self].Variant[crate::result::Result::Err(0)]; ReturnValue; value |
edges
| main.rs:19:9:19:9 | s | main.rs:20:10:20:10 | s | provenance | |
| main.rs:19:13:19:21 | source(...) | main.rs:19:9:19:9 | s | provenance | |
@@ -72,7 +73,8 @@ edges
| main.rs:237:9:237:10 | s1 [Some] | main.rs:238:10:238:11 | s1 [Some] | provenance | |
| main.rs:237:14:237:29 | Some(...) [Some] | main.rs:237:9:237:10 | s1 [Some] | provenance | |
| main.rs:237:19:237:28 | source(...) | main.rs:237:14:237:29 | Some(...) [Some] | provenance | |
| main.rs:238:10:238:11 | s1 [Some] | main.rs:238:10:238:32 | s1.unwrap_or_else(...) | provenance | MaD:4 |
| main.rs:238:10:238:11 | s1 [Some] | main.rs:238:10:238:32 | s1.unwrap_or_else(...) | provenance | MaD:5 |
| main.rs:241:31:241:40 | source(...) | main.rs:241:10:241:41 | s2.unwrap_or_else(...) | provenance | MaD:4 |
| main.rs:245:9:245:10 | s1 [Some] | main.rs:247:14:247:15 | s1 [Some] | provenance | |
| main.rs:245:14:245:29 | Some(...) [Some] | main.rs:245:9:245:10 | s1 [Some] | provenance | |
| main.rs:245:19:245:28 | source(...) | main.rs:245:14:245:29 | Some(...) [Some] | provenance | |
@@ -88,11 +90,11 @@ edges
| main.rs:267:9:267:10 | s1 [Ok] | main.rs:268:10:268:11 | s1 [Ok] | provenance | |
| main.rs:267:32:267:45 | Ok(...) [Ok] | main.rs:267:9:267:10 | s1 [Ok] | provenance | |
| main.rs:267:35:267:44 | source(...) | main.rs:267:32:267:45 | Ok(...) [Ok] | provenance | |
| main.rs:268:10:268:11 | s1 [Ok] | main.rs:268:10:268:22 | s1.expect(...) | provenance | MaD:5 |
| main.rs:268:10:268:11 | s1 [Ok] | main.rs:268:10:268:22 | s1.expect(...) | provenance | MaD:6 |
| main.rs:271:9:271:10 | s2 [Err] | main.rs:273:10:273:11 | s2 [Err] | provenance | |
| main.rs:271:32:271:46 | Err(...) [Err] | main.rs:271:9:271:10 | s2 [Err] | provenance | |
| main.rs:271:36:271:45 | source(...) | main.rs:271:32:271:46 | Err(...) [Err] | provenance | |
| main.rs:273:10:273:11 | s2 [Err] | main.rs:273:10:273:26 | s2.expect_err(...) | provenance | MaD:6 |
| main.rs:273:10:273:11 | s2 [Err] | main.rs:273:10:273:26 | s2.expect_err(...) | provenance | MaD:7 |
| main.rs:282:9:282:10 | s1 [A] | main.rs:284:11:284:12 | s1 [A] | provenance | |
| main.rs:282:14:282:39 | ...::A(...) [A] | main.rs:282:9:282:10 | s1 [A] | provenance | |
| main.rs:282:29:282:38 | source(...) | main.rs:282:14:282:39 | ...::A(...) [A] | provenance | |
@@ -255,6 +257,8 @@ nodes
| main.rs:237:19:237:28 | source(...) | semmle.label | source(...) |
| main.rs:238:10:238:11 | s1 [Some] | semmle.label | s1 [Some] |
| main.rs:238:10:238:32 | s1.unwrap_or_else(...) | semmle.label | s1.unwrap_or_else(...) |
| main.rs:241:10:241:41 | s2.unwrap_or_else(...) | semmle.label | s2.unwrap_or_else(...) |
| main.rs:241:31:241:40 | source(...) | semmle.label | source(...) |
| main.rs:245:9:245:10 | s1 [Some] | semmle.label | s1 [Some] |
| main.rs:245:14:245:29 | Some(...) [Some] | semmle.label | Some(...) [Some] |
| main.rs:245:19:245:28 | source(...) | semmle.label | source(...) |
@@ -386,6 +390,7 @@ testFailures
| main.rs:230:10:230:24 | s1.unwrap_or(...) | main.rs:229:19:229:28 | source(...) | main.rs:230:10:230:24 | s1.unwrap_or(...) | $@ | main.rs:229:19:229:28 | source(...) | source(...) |
| main.rs:233:10:233:33 | s2.unwrap_or(...) | main.rs:233:23:233:32 | source(...) | main.rs:233:10:233:33 | s2.unwrap_or(...) | $@ | main.rs:233:23:233:32 | source(...) | source(...) |
| main.rs:238:10:238:32 | s1.unwrap_or_else(...) | main.rs:237:19:237:28 | source(...) | main.rs:238:10:238:32 | s1.unwrap_or_else(...) | $@ | main.rs:237:19:237:28 | source(...) | source(...) |
| main.rs:241:10:241:41 | s2.unwrap_or_else(...) | main.rs:241:31:241:40 | source(...) | main.rs:241:10:241:41 | s2.unwrap_or_else(...) | $@ | main.rs:241:31:241:40 | source(...) | source(...) |
| main.rs:248:10:248:11 | i1 | main.rs:245:19:245:28 | source(...) | main.rs:248:10:248:11 | i1 | $@ | main.rs:245:19:245:28 | source(...) | source(...) |
| main.rs:259:10:259:11 | i1 | main.rs:254:35:254:44 | source(...) | main.rs:259:10:259:11 | i1 | $@ | main.rs:254:35:254:44 | source(...) | source(...) |
| main.rs:268:10:268:22 | s1.expect(...) | main.rs:267:35:267:44 | source(...) | main.rs:268:10:268:22 | s1.expect(...) | $@ | main.rs:267:35:267:44 | source(...) | source(...) |

View File

@@ -238,7 +238,7 @@ fn option_unwrap_or_else() {
sink(s1.unwrap_or_else(|| 0)); // $ hasValueFlow=47
let s2 = None;
sink(s2.unwrap_or_else(|| source(48))); // $ MISSING: hasValueFlow=48
sink(s2.unwrap_or_else(|| source(48))); // $ hasValueFlow=48
}
fn option_questionmark() -> Option<i64> {
@@ -408,14 +408,14 @@ fn array_assignment() {
// Test data flow inconsistency occuring with captured variables and `continue`
// in a loop.
pub fn captured_variable_and_continue(names: Vec<(bool, Option<String>)>) {
let default_name = source(83).to_string();
for (cond, name) in names {
if cond {
let n = name.unwrap_or_else(|| default_name.to_string());
sink(n.len() as i64);
continue;
let default_name = source(83).to_string();
for (cond, name) in names {
if cond {
let n = name.unwrap_or_else(|| default_name.to_string());
sink(n.len() as i64);
continue;
}
}
}
}
macro_rules! get_source {