From 6df5a1ef8036b7d50ebafc03569d243ba9194231 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 24 Apr 2025 14:14:21 +0200 Subject: [PATCH 1/2] Rust: Extract `SelfParam`s from crate graph --- rust/extractor/src/crate_graph.rs | 45 +++++++++++++++---- .../dataflow/modeled/inline-flow.expected | 18 ++++++++ .../type-inference/type-inference.ql | 3 +- .../PathResolutionConsistency.expected | 13 ++++++ 4 files changed, 70 insertions(+), 9 deletions(-) create mode 100644 rust/ql/test/query-tests/security/CWE-089/CONSISTENCY/PathResolutionConsistency.expected diff --git a/rust/extractor/src/crate_graph.rs b/rust/extractor/src/crate_graph.rs index 0203e8adc47d..8122248aba3c 100644 --- a/rust/extractor/src/crate_graph.rs +++ b/rust/extractor/src/crate_graph.rs @@ -383,25 +383,54 @@ fn emit_function( assert_eq!(sig.binders.len(Interner), parameters.len()); let sig = sig.skip_binders(); let ty_vars = &[parameters]; + let function_data = db.function_data(function); + let mut self_param = None; let params = sig .params() .iter() - .map(|p| { + .enumerate() + .filter_map(|(idx, p)| { let type_repr = emit_hir_ty(trap, db, ty_vars, p); - trap.emit(generated::Param { - id: trap::TrapId::Star, - attrs: vec![], - type_repr, - pat: None, - }) + + if idx == 0 && function_data.has_self_param() { + // Check if the self parameter is a reference + let (is_ref, is_mut) = match p.kind(Interner) { + chalk_ir::TyKind::Ref(mutability, _, _) => { + (true, matches!(mutability, chalk_ir::Mutability::Mut)) + } + chalk_ir::TyKind::Raw(mutability, _) => { + (false, matches!(mutability, chalk_ir::Mutability::Mut)) + } + _ => (false, false), + }; + + self_param = Some(trap.emit(generated::SelfParam { + id: trap::TrapId::Star, + attrs: vec![], + type_repr, + is_ref, + is_mut, + lifetime: None, + name: None, + })); + None + } else { + Some(trap.emit(generated::Param { + id: trap::TrapId::Star, + attrs: vec![], + type_repr, + pat: None, + })) + } }) .collect(); let ret_type = emit_hir_ty(trap, db, ty_vars, sig.ret()); + let param_list = trap.emit(generated::ParamList { id: trap::TrapId::Star, params, - self_param: None, + self_param, }); let ret_type = ret_type.map(|ret_type| { trap.emit(generated::RetTypeRepr { 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 a7d350ee5acd..ff86fd568dd3 100644 --- a/rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected +++ b/rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected @@ -8,19 +8,29 @@ models | 7 | Summary: lang:core; crate::ptr::write; Argument[1]; Argument[0].Reference; value | edges | main.rs:12:9:12:9 | a [Some] | main.rs:13:10:13:19 | a.unwrap() | provenance | MaD:2 | +| main.rs:12:9:12:9 | a [Some] | main.rs:14:13:14:13 | a [Some] | provenance | | | main.rs:12:9:12:9 | a [Some] | main.rs:14:13:14:21 | a.clone() [Some] | provenance | MaD:1 | | main.rs:12:9:12:9 | a [Some] | main.rs:14:13:14:21 | a.clone() [Some] | provenance | generated | | main.rs:12:13:12:28 | Some(...) [Some] | main.rs:12:9:12:9 | a [Some] | provenance | | | main.rs:12:18:12:27 | source(...) | main.rs:12:13:12:28 | Some(...) [Some] | provenance | | +| main.rs:14:9:14:9 | b [&ref, Some] | main.rs:15:10:15:10 | b [&ref, Some] | provenance | | | main.rs:14:9:14:9 | b [Some] | main.rs:15:10:15:19 | b.unwrap() | provenance | MaD:2 | +| main.rs:14:13:14:13 | a [Some] | main.rs:14:13:14:21 | a.clone() [&ref, Some] | provenance | generated | +| main.rs:14:13:14:21 | a.clone() [&ref, Some] | main.rs:14:9:14:9 | b [&ref, Some] | provenance | | | main.rs:14:13:14:21 | a.clone() [Some] | main.rs:14:9:14:9 | b [Some] | provenance | | +| main.rs:15:10:15:10 | b [&ref, Some] | main.rs:15:10:15:19 | b.unwrap() | provenance | MaD:2 | | main.rs:19:9:19:9 | a [Ok] | main.rs:20:10:20:19 | a.unwrap() | provenance | MaD:5 | +| main.rs:19:9:19:9 | a [Ok] | main.rs:21:13:21:13 | a [Ok] | provenance | | | main.rs:19:9:19:9 | a [Ok] | main.rs:21:13:21:21 | a.clone() [Ok] | provenance | MaD:4 | | main.rs:19:9:19:9 | a [Ok] | main.rs:21:13:21:21 | a.clone() [Ok] | provenance | generated | | main.rs:19:31:19:44 | Ok(...) [Ok] | main.rs:19:9:19:9 | a [Ok] | provenance | | | main.rs:19:34:19:43 | source(...) | main.rs:19:31:19:44 | Ok(...) [Ok] | provenance | | +| main.rs:21:9:21:9 | b [&ref, Ok] | main.rs:22:10:22:10 | b [&ref, Ok] | provenance | | | main.rs:21:9:21:9 | b [Ok] | main.rs:22:10:22:19 | b.unwrap() | provenance | MaD:5 | +| main.rs:21:13:21:13 | a [Ok] | main.rs:21:13:21:21 | a.clone() [&ref, Ok] | provenance | generated | +| main.rs:21:13:21:21 | a.clone() [&ref, Ok] | main.rs:21:9:21:9 | b [&ref, Ok] | provenance | | | main.rs:21:13:21:21 | a.clone() [Ok] | main.rs:21:9:21:9 | b [Ok] | provenance | | +| main.rs:22:10:22:10 | b [&ref, Ok] | main.rs:22:10:22:19 | b.unwrap() | provenance | MaD:5 | | main.rs:26:9:26:9 | a | main.rs:27:10:27:10 | a | provenance | | | main.rs:26:9:26:9 | a | main.rs:28:13:28:21 | a.clone() | provenance | generated | | main.rs:26:13:26:22 | source(...) | main.rs:26:9:26:9 | a | provenance | | @@ -56,15 +66,23 @@ nodes | main.rs:12:13:12:28 | Some(...) [Some] | semmle.label | Some(...) [Some] | | main.rs:12:18:12:27 | source(...) | semmle.label | source(...) | | main.rs:13:10:13:19 | a.unwrap() | semmle.label | a.unwrap() | +| main.rs:14:9:14:9 | b [&ref, Some] | semmle.label | b [&ref, Some] | | main.rs:14:9:14:9 | b [Some] | semmle.label | b [Some] | +| main.rs:14:13:14:13 | a [Some] | semmle.label | a [Some] | +| main.rs:14:13:14:21 | a.clone() [&ref, Some] | semmle.label | a.clone() [&ref, Some] | | main.rs:14:13:14:21 | a.clone() [Some] | semmle.label | a.clone() [Some] | +| main.rs:15:10:15:10 | b [&ref, Some] | semmle.label | b [&ref, Some] | | main.rs:15:10:15:19 | b.unwrap() | semmle.label | b.unwrap() | | main.rs:19:9:19:9 | a [Ok] | semmle.label | a [Ok] | | main.rs:19:31:19:44 | Ok(...) [Ok] | semmle.label | Ok(...) [Ok] | | main.rs:19:34:19:43 | source(...) | semmle.label | source(...) | | main.rs:20:10:20:19 | a.unwrap() | semmle.label | a.unwrap() | +| main.rs:21:9:21:9 | b [&ref, Ok] | semmle.label | b [&ref, Ok] | | main.rs:21:9:21:9 | b [Ok] | semmle.label | b [Ok] | +| main.rs:21:13:21:13 | a [Ok] | semmle.label | a [Ok] | +| main.rs:21:13:21:21 | a.clone() [&ref, Ok] | semmle.label | a.clone() [&ref, Ok] | | main.rs:21:13:21:21 | a.clone() [Ok] | semmle.label | a.clone() [Ok] | +| main.rs:22:10:22:10 | b [&ref, Ok] | semmle.label | b [&ref, Ok] | | main.rs:22:10:22:19 | b.unwrap() | semmle.label | b.unwrap() | | main.rs:26:9:26:9 | a | semmle.label | a | | main.rs:26:13:26:22 | source(...) | semmle.label | source(...) | diff --git a/rust/ql/test/library-tests/type-inference/type-inference.ql b/rust/ql/test/library-tests/type-inference/type-inference.ql index d83900e58402..2652699558ff 100644 --- a/rust/ql/test/library-tests/type-inference/type-inference.ql +++ b/rust/ql/test/library-tests/type-inference/type-inference.ql @@ -4,7 +4,8 @@ import codeql.rust.internal.TypeInference as TypeInference import TypeInference query predicate inferType(AstNode n, TypePath path, Type t) { - t = TypeInference::inferType(n, path) + t = TypeInference::inferType(n, path) and + n.fromSource() } module ResolveTest implements TestSig { diff --git a/rust/ql/test/query-tests/security/CWE-089/CONSISTENCY/PathResolutionConsistency.expected b/rust/ql/test/query-tests/security/CWE-089/CONSISTENCY/PathResolutionConsistency.expected new file mode 100644 index 000000000000..9567c4ea5177 --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-089/CONSISTENCY/PathResolutionConsistency.expected @@ -0,0 +1,13 @@ +multipleMethodCallTargets +| sqlx.rs:65:30:65:52 | unsafe_query_2.as_str() | file://:0:0:0:0 | fn as_str | +| sqlx.rs:65:30:65:52 | unsafe_query_2.as_str() | file://:0:0:0:0 | fn as_str | +| sqlx.rs:65:30:65:52 | unsafe_query_2.as_str() | file://:0:0:0:0 | fn as_str | +| sqlx.rs:65:30:65:52 | unsafe_query_2.as_str() | file://:0:0:0:0 | fn as_str | +| sqlx.rs:65:30:65:52 | unsafe_query_2.as_str() | file://:0:0:0:0 | fn as_str | +| sqlx.rs:65:30:65:52 | unsafe_query_2.as_str() | file://:0:0:0:0 | fn as_str | +| sqlx.rs:76:29:76:51 | unsafe_query_2.as_str() | file://:0:0:0:0 | fn as_str | +| sqlx.rs:76:29:76:51 | unsafe_query_2.as_str() | file://:0:0:0:0 | fn as_str | +| sqlx.rs:76:29:76:51 | unsafe_query_2.as_str() | file://:0:0:0:0 | fn as_str | +| sqlx.rs:76:29:76:51 | unsafe_query_2.as_str() | file://:0:0:0:0 | fn as_str | +| sqlx.rs:76:29:76:51 | unsafe_query_2.as_str() | file://:0:0:0:0 | fn as_str | +| sqlx.rs:76:29:76:51 | unsafe_query_2.as_str() | file://:0:0:0:0 | fn as_str | From 7e205366ab370d3d7bf532483dafb23278f924f1 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Fri, 25 Apr 2025 08:49:02 +0200 Subject: [PATCH 2/2] Rust: Adjust `clone` modeling --- .../lib/codeql/rust/frameworks/stdlib/Clone.qll | 4 +++- .../dataflow/modeled/inline-flow.expected | 16 ++-------------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll b/rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll index 0798343837e0..514c3781355e 100644 --- a/rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll +++ b/rust/ql/lib/codeql/rust/frameworks/stdlib/Clone.qll @@ -18,7 +18,9 @@ final class CloneCallable extends SummarizedCallable::Range { final override predicate propagatesFlow( string input, string output, boolean preservesValue, string model ) { - input = "Argument[self]" and + // The `clone` method takes a `&self` parameter and dereferences it; + // make sure to not clone the reference itself + input = ["Argument[self].Reference", "Argument[self].WithoutReference"] and output = "ReturnValue" and preservesValue = true and model = "generated" 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 ff86fd568dd3..b5e19855cfa5 100644 --- a/rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected +++ b/rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected @@ -13,24 +13,18 @@ edges | main.rs:12:9:12:9 | a [Some] | main.rs:14:13:14:21 | a.clone() [Some] | provenance | generated | | main.rs:12:13:12:28 | Some(...) [Some] | main.rs:12:9:12:9 | a [Some] | provenance | | | main.rs:12:18:12:27 | source(...) | main.rs:12:13:12:28 | Some(...) [Some] | provenance | | -| main.rs:14:9:14:9 | b [&ref, Some] | main.rs:15:10:15:10 | b [&ref, Some] | provenance | | | main.rs:14:9:14:9 | b [Some] | main.rs:15:10:15:19 | b.unwrap() | provenance | MaD:2 | -| main.rs:14:13:14:13 | a [Some] | main.rs:14:13:14:21 | a.clone() [&ref, Some] | provenance | generated | -| main.rs:14:13:14:21 | a.clone() [&ref, Some] | main.rs:14:9:14:9 | b [&ref, Some] | provenance | | +| main.rs:14:13:14:13 | a [Some] | main.rs:14:13:14:21 | a.clone() [Some] | provenance | generated | | main.rs:14:13:14:21 | a.clone() [Some] | main.rs:14:9:14:9 | b [Some] | provenance | | -| main.rs:15:10:15:10 | b [&ref, Some] | main.rs:15:10:15:19 | b.unwrap() | provenance | MaD:2 | | main.rs:19:9:19:9 | a [Ok] | main.rs:20:10:20:19 | a.unwrap() | provenance | MaD:5 | | main.rs:19:9:19:9 | a [Ok] | main.rs:21:13:21:13 | a [Ok] | provenance | | | main.rs:19:9:19:9 | a [Ok] | main.rs:21:13:21:21 | a.clone() [Ok] | provenance | MaD:4 | | main.rs:19:9:19:9 | a [Ok] | main.rs:21:13:21:21 | a.clone() [Ok] | provenance | generated | | main.rs:19:31:19:44 | Ok(...) [Ok] | main.rs:19:9:19:9 | a [Ok] | provenance | | | main.rs:19:34:19:43 | source(...) | main.rs:19:31:19:44 | Ok(...) [Ok] | provenance | | -| main.rs:21:9:21:9 | b [&ref, Ok] | main.rs:22:10:22:10 | b [&ref, Ok] | provenance | | | main.rs:21:9:21:9 | b [Ok] | main.rs:22:10:22:19 | b.unwrap() | provenance | MaD:5 | -| main.rs:21:13:21:13 | a [Ok] | main.rs:21:13:21:21 | a.clone() [&ref, Ok] | provenance | generated | -| main.rs:21:13:21:21 | a.clone() [&ref, Ok] | main.rs:21:9:21:9 | b [&ref, Ok] | provenance | | +| main.rs:21:13:21:13 | a [Ok] | main.rs:21:13:21:21 | a.clone() [Ok] | provenance | generated | | main.rs:21:13:21:21 | a.clone() [Ok] | main.rs:21:9:21:9 | b [Ok] | provenance | | -| main.rs:22:10:22:10 | b [&ref, Ok] | main.rs:22:10:22:19 | b.unwrap() | provenance | MaD:5 | | main.rs:26:9:26:9 | a | main.rs:27:10:27:10 | a | provenance | | | main.rs:26:9:26:9 | a | main.rs:28:13:28:21 | a.clone() | provenance | generated | | main.rs:26:13:26:22 | source(...) | main.rs:26:9:26:9 | a | provenance | | @@ -66,23 +60,17 @@ nodes | main.rs:12:13:12:28 | Some(...) [Some] | semmle.label | Some(...) [Some] | | main.rs:12:18:12:27 | source(...) | semmle.label | source(...) | | main.rs:13:10:13:19 | a.unwrap() | semmle.label | a.unwrap() | -| main.rs:14:9:14:9 | b [&ref, Some] | semmle.label | b [&ref, Some] | | main.rs:14:9:14:9 | b [Some] | semmle.label | b [Some] | | main.rs:14:13:14:13 | a [Some] | semmle.label | a [Some] | -| main.rs:14:13:14:21 | a.clone() [&ref, Some] | semmle.label | a.clone() [&ref, Some] | | main.rs:14:13:14:21 | a.clone() [Some] | semmle.label | a.clone() [Some] | -| main.rs:15:10:15:10 | b [&ref, Some] | semmle.label | b [&ref, Some] | | main.rs:15:10:15:19 | b.unwrap() | semmle.label | b.unwrap() | | main.rs:19:9:19:9 | a [Ok] | semmle.label | a [Ok] | | main.rs:19:31:19:44 | Ok(...) [Ok] | semmle.label | Ok(...) [Ok] | | main.rs:19:34:19:43 | source(...) | semmle.label | source(...) | | main.rs:20:10:20:19 | a.unwrap() | semmle.label | a.unwrap() | -| main.rs:21:9:21:9 | b [&ref, Ok] | semmle.label | b [&ref, Ok] | | main.rs:21:9:21:9 | b [Ok] | semmle.label | b [Ok] | | main.rs:21:13:21:13 | a [Ok] | semmle.label | a [Ok] | -| main.rs:21:13:21:21 | a.clone() [&ref, Ok] | semmle.label | a.clone() [&ref, Ok] | | main.rs:21:13:21:21 | a.clone() [Ok] | semmle.label | a.clone() [Ok] | -| main.rs:22:10:22:10 | b [&ref, Ok] | semmle.label | b [&ref, Ok] | | main.rs:22:10:22:19 | b.unwrap() | semmle.label | b.unwrap() | | main.rs:26:9:26:9 | a | semmle.label | a | | main.rs:26:13:26:22 | source(...) | semmle.label | source(...) |