From 04d3f989ecdd774852483f85202472406ec74b24 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Tue, 4 Feb 2025 17:09:26 +0100 Subject: [PATCH 1/3] Rust: Add tests for flow through `clone` --- .../dataflow/modeled/inline-flow.expected | 48 +++++++++++++++ .../dataflow/modeled/inline-flow.ql | 12 ++++ .../library-tests/dataflow/modeled/main.rs | 58 +++++++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected create mode 100644 rust/ql/test/library-tests/dataflow/modeled/inline-flow.ql create mode 100644 rust/ql/test/library-tests/dataflow/modeled/main.rs diff --git a/rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected b/rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected new file mode 100644 index 000000000000..b1becc116640 --- /dev/null +++ b/rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected @@ -0,0 +1,48 @@ +models +| 1 | Summary: lang:core; ::unwrap; Argument[self].Variant[crate::option::Option::Some(0)]; ReturnValue; value | +| 2 | Summary: lang:core; ::unwrap; Argument[self].Variant[crate::result::Result::Ok(0)]; ReturnValue; value | +edges +| main.rs:13:9:13:9 | a [Some] | main.rs:14:10:14:10 | a [Some] | provenance | | +| main.rs:13:13:13:28 | Some(...) [Some] | main.rs:13:9:13:9 | a [Some] | provenance | | +| main.rs:13:18:13:27 | source(...) | main.rs:13:13:13:28 | Some(...) [Some] | provenance | | +| main.rs:14:10:14:10 | a [Some] | main.rs:14:10:14:19 | a.unwrap(...) | provenance | MaD:1 | +| main.rs:20:9:20:9 | a [Ok] | main.rs:21:10:21:10 | a [Ok] | provenance | | +| main.rs:20:31:20:44 | Ok(...) [Ok] | main.rs:20:9:20:9 | a [Ok] | provenance | | +| main.rs:20:34:20:43 | source(...) | main.rs:20:31:20:44 | Ok(...) [Ok] | provenance | | +| main.rs:21:10:21:10 | a [Ok] | main.rs:21:10:21:19 | a.unwrap(...) | provenance | MaD:2 | +| main.rs:27:9:27:9 | a | main.rs:28:10:28:10 | a | provenance | | +| main.rs:27:13:27:22 | source(...) | main.rs:27:9:27:9 | a | provenance | | +| main.rs:42:13:42:13 | w [Wrapper.n] | main.rs:43:15:43:15 | w [Wrapper.n] | provenance | | +| main.rs:42:17:42:41 | Wrapper {...} [Wrapper.n] | main.rs:42:13:42:13 | w [Wrapper.n] | provenance | | +| main.rs:42:30:42:39 | source(...) | main.rs:42:17:42:41 | Wrapper {...} [Wrapper.n] | provenance | | +| main.rs:43:15:43:15 | w [Wrapper.n] | main.rs:44:13:44:28 | Wrapper {...} [Wrapper.n] | provenance | | +| main.rs:44:13:44:28 | Wrapper {...} [Wrapper.n] | main.rs:44:26:44:26 | n | provenance | | +| main.rs:44:26:44:26 | n | main.rs:44:38:44:38 | n | provenance | | +nodes +| main.rs:13:9:13:9 | a [Some] | semmle.label | a [Some] | +| main.rs:13:13:13:28 | Some(...) [Some] | semmle.label | Some(...) [Some] | +| main.rs:13:18:13:27 | source(...) | semmle.label | source(...) | +| main.rs:14:10:14:10 | a [Some] | semmle.label | a [Some] | +| main.rs:14:10:14:19 | a.unwrap(...) | semmle.label | a.unwrap(...) | +| main.rs:20:9:20:9 | a [Ok] | semmle.label | a [Ok] | +| main.rs:20:31:20:44 | Ok(...) [Ok] | semmle.label | Ok(...) [Ok] | +| main.rs:20:34:20:43 | source(...) | semmle.label | source(...) | +| main.rs:21:10:21:10 | a [Ok] | semmle.label | a [Ok] | +| main.rs:21:10:21:19 | a.unwrap(...) | semmle.label | a.unwrap(...) | +| main.rs:27:9:27:9 | a | semmle.label | a | +| main.rs:27:13:27:22 | source(...) | semmle.label | source(...) | +| main.rs:28:10:28:10 | a | semmle.label | a | +| main.rs:42:13:42:13 | w [Wrapper.n] | semmle.label | w [Wrapper.n] | +| main.rs:42:17:42:41 | Wrapper {...} [Wrapper.n] | semmle.label | Wrapper {...} [Wrapper.n] | +| main.rs:42:30:42:39 | source(...) | semmle.label | source(...) | +| main.rs:43:15:43:15 | w [Wrapper.n] | semmle.label | w [Wrapper.n] | +| main.rs:44:13:44:28 | Wrapper {...} [Wrapper.n] | semmle.label | Wrapper {...} [Wrapper.n] | +| main.rs:44:26:44:26 | n | semmle.label | n | +| main.rs:44:38:44:38 | n | semmle.label | n | +subpaths +testFailures +#select +| main.rs:14:10:14:19 | a.unwrap(...) | main.rs:13:18:13:27 | source(...) | main.rs:14:10:14:19 | a.unwrap(...) | $@ | main.rs:13:18:13:27 | source(...) | source(...) | +| main.rs:21:10:21:19 | a.unwrap(...) | main.rs:20:34:20:43 | source(...) | main.rs:21:10:21:19 | a.unwrap(...) | $@ | main.rs:20:34:20:43 | source(...) | source(...) | +| main.rs:28:10:28:10 | a | main.rs:27:13:27:22 | source(...) | main.rs:28:10:28:10 | a | $@ | main.rs:27:13:27:22 | source(...) | source(...) | +| main.rs:44:38:44:38 | n | main.rs:42:30:42:39 | source(...) | main.rs:44:38:44:38 | n | $@ | main.rs:42:30:42:39 | source(...) | source(...) | diff --git a/rust/ql/test/library-tests/dataflow/modeled/inline-flow.ql b/rust/ql/test/library-tests/dataflow/modeled/inline-flow.ql new file mode 100644 index 000000000000..e399ea0e5d71 --- /dev/null +++ b/rust/ql/test/library-tests/dataflow/modeled/inline-flow.ql @@ -0,0 +1,12 @@ +/** + * @kind path-problem + */ + +import rust +import utils.test.InlineFlowTest +import DefaultFlowTest +import ValueFlow::PathGraph + +from ValueFlow::PathNode source, ValueFlow::PathNode sink +where ValueFlow::flowPath(source, sink) +select sink, source, sink, "$@", source, source.toString() diff --git a/rust/ql/test/library-tests/dataflow/modeled/main.rs b/rust/ql/test/library-tests/dataflow/modeled/main.rs new file mode 100644 index 000000000000..4a6423b4efc3 --- /dev/null +++ b/rust/ql/test/library-tests/dataflow/modeled/main.rs @@ -0,0 +1,58 @@ + +fn source(i: i64) -> i64 { + 1000 + i +} + +fn sink(s: i64) { + println!("{}", s); +} + +// Flow through `clone` methods + +fn option_clone() { + let a = Some(source(88)); + sink(a.unwrap()); // $ hasValueFlow=88 + let b = a.clone(); + sink(b.unwrap()); // $ MISSING: hasValueFlow=88 +} + +fn result_clone() { + let a: Result = Ok(source(37)); + sink(a.unwrap()); // $ hasValueFlow=37 + let b = a.clone(); + sink(b.unwrap()); // $ MISSING: hasValueFlow=37 +} + +fn i64_clone() { + let a = source(12); + sink(a); // $ hasValueFlow=12 + let b = a.clone(); + sink(b); // $ MISSING: hasValueFlow=12 +} + +mod my_clone { + use super::{source, sink}; + + #[derive(Clone)] + struct Wrapper { + n: i64 + } + + pub fn wrapper_clone() { + let w = Wrapper { n: source(73) }; + match w { + Wrapper { n: n } => sink(n) // $ hasValueFlow=73 + } + let u = w.clone(); + match u { + Wrapper { n: n } => sink(n) // $ MISSING: hasValueFlow=73 + } + } +} + +fn main() { + option_clone(); + result_clone(); + i64_clone(); + my_clone::wrapper_clone(); +} From 86d7feabc6883f59437f1a48c509acd120fc87e6 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Tue, 4 Feb 2025 17:16:06 +0100 Subject: [PATCH 2/3] Rust: Add value flow model for `clone` methods --- rust/ql/lib/codeql/rust/Frameworks.qll | 1 + .../codeql/rust/frameworks/stdlib/Clone.qll | 21 +++++++++ .../dataflow/modeled/inline-flow.expected | 46 +++++++++++++++++++ .../library-tests/dataflow/modeled/main.rs | 8 ++-- 4 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll diff --git a/rust/ql/lib/codeql/rust/Frameworks.qll b/rust/ql/lib/codeql/rust/Frameworks.qll index 6d9dcaea5058..daa96538e21f 100644 --- a/rust/ql/lib/codeql/rust/Frameworks.qll +++ b/rust/ql/lib/codeql/rust/Frameworks.qll @@ -4,3 +4,4 @@ private import codeql.rust.frameworks.rustcrypto.RustCrypto private import codeql.rust.frameworks.Sqlx +private import codeql.rust.frameworks.stdlib.Clone diff --git a/rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll b/rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll new file mode 100644 index 000000000000..aa63268dbafe --- /dev/null +++ b/rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll @@ -0,0 +1,21 @@ +/** A model for `clone` on the `Clone` trait. */ + +import rust +import codeql.rust.dataflow.FlowSummary + +/** A `clone` method. */ +final class CloneCallable extends SummarizedCallable::Range { + CloneCallable() { + // NOTE: The function target may not exist in the database, so we base this + // on method calls. + exists(MethodCallExpr c | + c.getNameRef().getText() = "clone" and + c.getArgList().getNumberOfArgs() = 0 and + this = c.getResolvedCrateOrigin() + "::_::" + c.getResolvedPath() + ) + } + + final override predicate propagatesFlow(string input, string output, boolean preservesValue) { + input = "Argument[self]" and output = "ReturnValue" and preservesValue = true + } +} diff --git a/rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected b/rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected index b1becc116640..10fbbbe6e79e 100644 --- a/rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected +++ b/rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected @@ -3,35 +3,70 @@ models | 2 | Summary: lang:core; ::unwrap; Argument[self].Variant[crate::result::Result::Ok(0)]; ReturnValue; value | edges | main.rs:13:9:13:9 | a [Some] | main.rs:14:10:14:10 | a [Some] | provenance | | +| main.rs:13:9:13:9 | a [Some] | main.rs:15:13:15:13 | a [Some] | provenance | | | main.rs:13:13:13:28 | Some(...) [Some] | main.rs:13:9:13:9 | a [Some] | provenance | | | main.rs:13:18:13:27 | source(...) | main.rs:13:13:13:28 | Some(...) [Some] | provenance | | | main.rs:14:10:14:10 | a [Some] | main.rs:14:10:14:19 | a.unwrap(...) | provenance | MaD:1 | +| main.rs:15:9:15:9 | b [Some] | main.rs:16:10:16:10 | b [Some] | provenance | | +| main.rs:15:13:15:13 | a [Some] | main.rs:15:13:15:21 | a.clone(...) [Some] | provenance | | +| main.rs:15:13:15:21 | a.clone(...) [Some] | main.rs:15:9:15:9 | b [Some] | provenance | | +| main.rs:16:10:16:10 | b [Some] | main.rs:16:10:16:19 | b.unwrap(...) | provenance | MaD:1 | | main.rs:20:9:20:9 | a [Ok] | main.rs:21:10:21:10 | a [Ok] | provenance | | +| main.rs:20:9:20:9 | a [Ok] | main.rs:22:13:22:13 | a [Ok] | provenance | | | main.rs:20:31:20:44 | Ok(...) [Ok] | main.rs:20:9:20:9 | a [Ok] | provenance | | | main.rs:20:34:20:43 | source(...) | main.rs:20:31:20:44 | Ok(...) [Ok] | provenance | | | main.rs:21:10:21:10 | a [Ok] | main.rs:21:10:21:19 | a.unwrap(...) | provenance | MaD:2 | +| main.rs:22:9:22:9 | b [Ok] | main.rs:23:10:23:10 | b [Ok] | provenance | | +| main.rs:22:13:22:13 | a [Ok] | main.rs:22:13:22:21 | a.clone(...) [Ok] | provenance | | +| main.rs:22:13:22:21 | a.clone(...) [Ok] | main.rs:22:9:22:9 | b [Ok] | provenance | | +| main.rs:23:10:23:10 | b [Ok] | main.rs:23:10:23:19 | b.unwrap(...) | provenance | MaD:2 | | main.rs:27:9:27:9 | a | main.rs:28:10:28:10 | a | provenance | | +| main.rs:27:9:27:9 | a | main.rs:29:13:29:13 | a | provenance | | | main.rs:27:13:27:22 | source(...) | main.rs:27:9:27:9 | a | provenance | | +| main.rs:29:9:29:9 | b | main.rs:30:10:30:10 | b | provenance | | +| main.rs:29:13:29:13 | a | main.rs:29:13:29:21 | a.clone(...) | provenance | | +| main.rs:29:13:29:21 | a.clone(...) | main.rs:29:9:29:9 | b | provenance | | | main.rs:42:13:42:13 | w [Wrapper.n] | main.rs:43:15:43:15 | w [Wrapper.n] | provenance | | | main.rs:42:17:42:41 | Wrapper {...} [Wrapper.n] | main.rs:42:13:42:13 | w [Wrapper.n] | provenance | | | main.rs:42:30:42:39 | source(...) | main.rs:42:17:42:41 | Wrapper {...} [Wrapper.n] | provenance | | | main.rs:43:15:43:15 | w [Wrapper.n] | main.rs:44:13:44:28 | Wrapper {...} [Wrapper.n] | provenance | | +| main.rs:43:15:43:15 | w [Wrapper.n] | main.rs:46:17:46:17 | w [Wrapper.n] | provenance | | | main.rs:44:13:44:28 | Wrapper {...} [Wrapper.n] | main.rs:44:26:44:26 | n | provenance | | | main.rs:44:26:44:26 | n | main.rs:44:38:44:38 | n | provenance | | +| main.rs:46:13:46:13 | u [Wrapper.n] | main.rs:47:15:47:15 | u [Wrapper.n] | provenance | | +| main.rs:46:17:46:17 | w [Wrapper.n] | main.rs:46:17:46:25 | w.clone(...) [Wrapper.n] | provenance | | +| main.rs:46:17:46:25 | w.clone(...) [Wrapper.n] | main.rs:46:13:46:13 | u [Wrapper.n] | provenance | | +| main.rs:47:15:47:15 | u [Wrapper.n] | main.rs:48:13:48:28 | Wrapper {...} [Wrapper.n] | provenance | | +| main.rs:48:13:48:28 | Wrapper {...} [Wrapper.n] | main.rs:48:26:48:26 | n | provenance | | +| main.rs:48:26:48:26 | n | main.rs:48:38:48:38 | n | provenance | | nodes | main.rs:13:9:13:9 | a [Some] | semmle.label | a [Some] | | main.rs:13:13:13:28 | Some(...) [Some] | semmle.label | Some(...) [Some] | | main.rs:13:18:13:27 | source(...) | semmle.label | source(...) | | main.rs:14:10:14:10 | a [Some] | semmle.label | a [Some] | | main.rs:14:10:14:19 | a.unwrap(...) | semmle.label | a.unwrap(...) | +| main.rs:15:9:15:9 | b [Some] | semmle.label | b [Some] | +| main.rs:15:13:15:13 | a [Some] | semmle.label | a [Some] | +| main.rs:15:13:15:21 | a.clone(...) [Some] | semmle.label | a.clone(...) [Some] | +| main.rs:16:10:16:10 | b [Some] | semmle.label | b [Some] | +| main.rs:16:10:16:19 | b.unwrap(...) | semmle.label | b.unwrap(...) | | main.rs:20:9:20:9 | a [Ok] | semmle.label | a [Ok] | | main.rs:20:31:20:44 | Ok(...) [Ok] | semmle.label | Ok(...) [Ok] | | main.rs:20:34:20:43 | source(...) | semmle.label | source(...) | | main.rs:21:10:21:10 | a [Ok] | semmle.label | a [Ok] | | main.rs:21:10:21:19 | a.unwrap(...) | semmle.label | a.unwrap(...) | +| main.rs:22:9:22:9 | b [Ok] | semmle.label | b [Ok] | +| main.rs:22:13:22:13 | a [Ok] | semmle.label | a [Ok] | +| main.rs:22:13:22:21 | a.clone(...) [Ok] | semmle.label | a.clone(...) [Ok] | +| main.rs:23:10:23:10 | b [Ok] | semmle.label | b [Ok] | +| main.rs:23:10:23:19 | b.unwrap(...) | semmle.label | b.unwrap(...) | | main.rs:27:9:27:9 | a | semmle.label | a | | main.rs:27:13:27:22 | source(...) | semmle.label | source(...) | | main.rs:28:10:28:10 | a | semmle.label | a | +| main.rs:29:9:29:9 | b | semmle.label | b | +| main.rs:29:13:29:13 | a | semmle.label | a | +| main.rs:29:13:29:21 | a.clone(...) | semmle.label | a.clone(...) | +| main.rs:30:10:30:10 | b | semmle.label | b | | main.rs:42:13:42:13 | w [Wrapper.n] | semmle.label | w [Wrapper.n] | | main.rs:42:17:42:41 | Wrapper {...} [Wrapper.n] | semmle.label | Wrapper {...} [Wrapper.n] | | main.rs:42:30:42:39 | source(...) | semmle.label | source(...) | @@ -39,10 +74,21 @@ nodes | main.rs:44:13:44:28 | Wrapper {...} [Wrapper.n] | semmle.label | Wrapper {...} [Wrapper.n] | | main.rs:44:26:44:26 | n | semmle.label | n | | main.rs:44:38:44:38 | n | semmle.label | n | +| main.rs:46:13:46:13 | u [Wrapper.n] | semmle.label | u [Wrapper.n] | +| main.rs:46:17:46:17 | w [Wrapper.n] | semmle.label | w [Wrapper.n] | +| main.rs:46:17:46:25 | w.clone(...) [Wrapper.n] | semmle.label | w.clone(...) [Wrapper.n] | +| main.rs:47:15:47:15 | u [Wrapper.n] | semmle.label | u [Wrapper.n] | +| main.rs:48:13:48:28 | Wrapper {...} [Wrapper.n] | semmle.label | Wrapper {...} [Wrapper.n] | +| main.rs:48:26:48:26 | n | semmle.label | n | +| main.rs:48:38:48:38 | n | semmle.label | n | subpaths testFailures #select | main.rs:14:10:14:19 | a.unwrap(...) | main.rs:13:18:13:27 | source(...) | main.rs:14:10:14:19 | a.unwrap(...) | $@ | main.rs:13:18:13:27 | source(...) | source(...) | +| main.rs:16:10:16:19 | b.unwrap(...) | main.rs:13:18:13:27 | source(...) | main.rs:16:10:16:19 | b.unwrap(...) | $@ | main.rs:13:18:13:27 | source(...) | source(...) | | main.rs:21:10:21:19 | a.unwrap(...) | main.rs:20:34:20:43 | source(...) | main.rs:21:10:21:19 | a.unwrap(...) | $@ | main.rs:20:34:20:43 | source(...) | source(...) | +| main.rs:23:10:23:19 | b.unwrap(...) | main.rs:20:34:20:43 | source(...) | main.rs:23:10:23:19 | b.unwrap(...) | $@ | main.rs:20:34:20:43 | source(...) | source(...) | | main.rs:28:10:28:10 | a | main.rs:27:13:27:22 | source(...) | main.rs:28:10:28:10 | a | $@ | main.rs:27:13:27:22 | source(...) | source(...) | +| main.rs:30:10:30:10 | b | main.rs:27:13:27:22 | source(...) | main.rs:30:10:30:10 | b | $@ | main.rs:27:13:27:22 | source(...) | source(...) | | main.rs:44:38:44:38 | n | main.rs:42:30:42:39 | source(...) | main.rs:44:38:44:38 | n | $@ | main.rs:42:30:42:39 | source(...) | source(...) | +| main.rs:48:38:48:38 | n | main.rs:42:30:42:39 | source(...) | main.rs:48:38:48:38 | n | $@ | main.rs:42:30:42:39 | source(...) | source(...) | diff --git a/rust/ql/test/library-tests/dataflow/modeled/main.rs b/rust/ql/test/library-tests/dataflow/modeled/main.rs index 4a6423b4efc3..a9c52adab9b1 100644 --- a/rust/ql/test/library-tests/dataflow/modeled/main.rs +++ b/rust/ql/test/library-tests/dataflow/modeled/main.rs @@ -13,21 +13,21 @@ fn option_clone() { let a = Some(source(88)); sink(a.unwrap()); // $ hasValueFlow=88 let b = a.clone(); - sink(b.unwrap()); // $ MISSING: hasValueFlow=88 + sink(b.unwrap()); // $ hasValueFlow=88 } fn result_clone() { let a: Result = Ok(source(37)); sink(a.unwrap()); // $ hasValueFlow=37 let b = a.clone(); - sink(b.unwrap()); // $ MISSING: hasValueFlow=37 + sink(b.unwrap()); // $ hasValueFlow=37 } fn i64_clone() { let a = source(12); sink(a); // $ hasValueFlow=12 let b = a.clone(); - sink(b); // $ MISSING: hasValueFlow=12 + sink(b); // $ hasValueFlow=12 } mod my_clone { @@ -45,7 +45,7 @@ mod my_clone { } let u = w.clone(); match u { - Wrapper { n: n } => sink(n) // $ MISSING: hasValueFlow=73 + Wrapper { n: n } => sink(n) // $ hasValueFlow=73 } } } From b2ba5f4f38e02558e538b21684e8b154c9f13c56 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Thu, 6 Feb 2025 16:07:25 +0100 Subject: [PATCH 3/3] Rust: Make imports private --- rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll b/rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll index aa63268dbafe..fef47f5ec972 100644 --- a/rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll +++ b/rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll @@ -1,7 +1,7 @@ /** A model for `clone` on the `Clone` trait. */ -import rust -import codeql.rust.dataflow.FlowSummary +private import rust +private import codeql.rust.dataflow.FlowSummary /** A `clone` method. */ final class CloneCallable extends SummarizedCallable::Range {