From 68a4ea3be06f87366e80cd250d9b60dfc3eb8f5d Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 21 Nov 2024 13:02:52 +0000
Subject: [PATCH 01/51] Rust: New query rust/ctor-initialization (placeholder).

undo
---
 .../security/CWE-696/BadCtorInitialization.ql    | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql

diff --git a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
new file mode 100644
index 000000000000..9d6fa7351e77
--- /dev/null
+++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
@@ -0,0 +1,16 @@
+/**
+ * @name Bad 'ctor' initialization
+ * @description TODO
+ * @kind path-problem
+ * @problem.severity error
+ * @ security-severity TODO
+ * @ precision TODO
+ * @id rust/ctor-initialization
+ * @tags security
+ *       external/cwe/cwe-696
+ *       external/cwe/cwe-665
+ */
+
+import rust
+
+select 0

From 9ead2dc03c5d8914f5d654ad511990c7ae076120 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 21 Nov 2024 15:12:09 +0000
Subject: [PATCH 02/51] Rust: Add a query test.

---
 .../CWE-696/BadCTorInitialization.expected    |   1 +
 .../CWE-696/BadCTorInitialization.qlref       |   2 +
 .../query-tests/security/CWE-696/options.yml  |   3 +
 .../test/query-tests/security/CWE-696/test.rs | 144 ++++++++++++++++++
 4 files changed, 150 insertions(+)
 create mode 100644 rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
 create mode 100644 rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.qlref
 create mode 100644 rust/ql/test/query-tests/security/CWE-696/options.yml
 create mode 100644 rust/ql/test/query-tests/security/CWE-696/test.rs

diff --git a/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected b/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
new file mode 100644
index 000000000000..f082a67fcf66
--- /dev/null
+++ b/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
@@ -0,0 +1 @@
+| 0 |
diff --git a/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.qlref b/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.qlref
new file mode 100644
index 000000000000..2b71705c98b8
--- /dev/null
+++ b/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.qlref
@@ -0,0 +1,2 @@
+query: queries/security/CWE-696/BadCtorInitialization.ql
+postprocess: utils/InlineExpectationsTestQuery.ql
diff --git a/rust/ql/test/query-tests/security/CWE-696/options.yml b/rust/ql/test/query-tests/security/CWE-696/options.yml
new file mode 100644
index 000000000000..1f1a13e589ea
--- /dev/null
+++ b/rust/ql/test/query-tests/security/CWE-696/options.yml
@@ -0,0 +1,3 @@
+qltest_cargo_check: true
+qltest_dependencies:
+    - ctor = { version = "0.2.9" }
diff --git a/rust/ql/test/query-tests/security/CWE-696/test.rs b/rust/ql/test/query-tests/security/CWE-696/test.rs
new file mode 100644
index 000000000000..9fb90a018e6b
--- /dev/null
+++ b/rust/ql/test/query-tests/security/CWE-696/test.rs
@@ -0,0 +1,144 @@
+
+// --- attribute variants ---
+
+use std::io::Write;
+
+fn harmless1_1() {
+    // ...
+}
+
+#[ctor::ctor]
+fn harmless1_2() {
+    // ...
+}
+
+#[ctor::dtor]
+fn harmless1_3() {
+    // ...
+}
+
+fn harmless1_4() {
+    _ = std::io::stdout().write(b"Hello, world!");
+}
+
+#[rustfmt::skip]
+fn harmless1_5() {
+    _ = std::io::stdout().write(b"Hello, world!");
+}
+
+#[ctor::ctor]
+fn bad1_1() { // $ MISSING: Alert[rust/ctor-initialization]
+    _ = std::io::stdout().write(b"Hello, world!");
+}
+
+#[ctor::dtor]
+fn bad1_2() { // $ MISSING: Alert[rust/ctor-initialization]
+    _ = std::io::stdout().write(b"Hello, world!");
+}
+
+#[rustfmt::skip]
+#[ctor::dtor]
+#[rustfmt::skip]
+fn bad1_3() { // $ MISSING: Alert[rust/ctor-initialization]
+    _ = std::io::stdout().write(b"Hello, world!");
+}
+
+// --- code variants ---
+
+use ctor::ctor;
+use std::io::*;
+
+#[ctor]
+fn bad2_1() { // $ MISSING: Alert[rust/ctor-initialization]
+    _ = stdout().write(b"Hello, world!");
+}
+
+#[ctor]
+fn bad2_2() { // $ MISSING: Alert[rust/ctor-initialization]
+    _ = stderr().write_all(b"Hello, world!");
+}
+
+#[ctor]
+fn bad2_3() { // $ MISSING: Alert[rust/ctor-initialization]
+    println!("Hello, world!");
+}
+
+#[ctor]
+fn bad2_4() { // $ MISSING: Alert[rust/ctor-initialization]
+    let mut buff = String::new();
+    _ = std::io::stdin().read_line(&mut buff);
+}
+
+use std::fs;
+
+#[ctor]
+fn bad2_5() { // $ MISSING: Alert[rust/ctor-initialization]
+    let _buff = fs::File::create("hello.txt").unwrap();
+}
+
+#[ctor]
+fn bad2_6() { // $ MISSING: Alert[rust/ctor-initialization]
+    let _t = std::time::Instant::now();
+}
+
+use std::time::Duration;
+
+const DURATION2_7: Duration = Duration::new(1, 0);
+
+#[ctor]
+fn bad2_7() { // $ MISSING: Alert[rust/ctor-initialization]
+    std::thread::sleep(DURATION2_7);
+}
+
+use std::process;
+
+#[ctor]
+fn bad2_8() { // $ MISSING: Alert[rust/ctor-initialization]
+    process::exit(1234);
+}
+
+// --- transitive cases ---
+
+fn call_target3_1() {
+    _ = stderr().write_all(b"Hello, world!");
+}
+
+#[ctor]
+fn bad3_1() { // $ MISSING: Alert[rust/ctor-initialization]
+    call_target3_1();
+}
+
+fn call_target3_2() {
+    for _x in 0..10 {
+        // ...
+    }
+}
+
+#[ctor]
+fn harmless3_2() {
+    call_target3_2();
+}
+
+#[ctor]
+fn bad3_3() { // $ MISSING: Alert[rust/ctor-initialization]
+    call_target3_1();
+    call_target3_2();
+}
+
+#[ctor]
+fn bad3_4() { // $ MISSING: Alert[rust/ctor-initialization]
+    bad3_3();
+}
+
+// --- macros ---
+
+macro_rules! macro4_1 {
+    () => {
+        _ = std::io::stdout().write(b"Hello, world!");
+    };
+}
+
+#[ctor]
+fn bad4_1() { // $ MISSING: Alert[rust/ctor-initialization]
+    macro4_1!();
+}

From 88fc7be0a233f8fa26bd13b2c22de6f6ca0e234c Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 21 Nov 2024 22:37:41 +0000
Subject: [PATCH 03/51] Rust: Implement the query.

---
 .../security/CWE-696/BadCtorInitialization.ql | 24 ++++++++++++++++++-
 .../CWE-696/BadCTorInitialization.expected    | 11 ++++++++-
 .../test/query-tests/security/CWE-696/test.rs | 20 ++++++++--------
 3 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
index 9d6fa7351e77..5999ac0e02ec 100644
--- a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
+++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
@@ -13,4 +13,26 @@
 
 import rust
 
-select 0
+/**
+ * A `#[ctor]` or `#[dtor]` attribute.
+ */
+class CtorAttr extends Attr {
+  CtorAttr() { this.getMeta().getPath().getPart().getNameRef().getText() = ["ctor", "dtor"] }
+}
+
+/**
+ * A call into the Rust standard library.
+ */
+class StdCall extends Expr {
+  StdCall() {
+    this.(CallExpr).getExpr().(PathExpr).getPath().getResolvedCrateOrigin() = "lang:std" or
+    this.(MethodCallExpr).getResolvedCrateOrigin() = "lang:std"
+  }
+}
+
+from CtorAttr ctor, Function f, StdCall call
+where
+  f.getAnAttr() = ctor and
+  call.getEnclosingCallable() = f
+select f.getName(), "This function has the $@ attribute but calls $@ in the standard library.",
+  ctor, ctor.toString(), call, call.toString()
diff --git a/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected b/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
index f082a67fcf66..36ab89a79bf4 100644
--- a/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
+++ b/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
@@ -1 +1,10 @@
-| 0 |
+| test.rs:30:4:30:9 | bad1_1 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:29:1:29:13 | Attr | Attr | test.rs:31:9:31:25 | ...::stdout(...) | ...::stdout(...) |
+| test.rs:35:4:35:9 | bad1_2 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:34:1:34:13 | Attr | Attr | test.rs:36:9:36:25 | ...::stdout(...) | ...::stdout(...) |
+| test.rs:42:4:42:9 | bad1_3 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:40:1:40:13 | Attr | Attr | test.rs:43:9:43:25 | ...::stdout(...) | ...::stdout(...) |
+| test.rs:52:4:52:9 | bad2_1 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:51:1:51:7 | Attr | Attr | test.rs:53:9:53:16 | stdout(...) | stdout(...) |
+| test.rs:57:4:57:9 | bad2_2 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:56:1:56:7 | Attr | Attr | test.rs:58:9:58:16 | stderr(...) | stderr(...) |
+| test.rs:62:4:62:9 | bad2_3 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:61:1:61:7 | Attr | Attr | test.rs:63:14:63:28 | ...::_print(...) | ...::_print(...) |
+| test.rs:67:4:67:9 | bad2_4 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:66:1:66:7 | Attr | Attr | test.rs:69:9:69:24 | ...::stdin(...) | ...::stdin(...) |
+| test.rs:89:4:89:9 | bad2_7 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:88:1:88:7 | Attr | Attr | test.rs:90:5:90:35 | ...::sleep(...) | ...::sleep(...) |
+| test.rs:96:4:96:9 | bad2_8 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:95:1:95:7 | Attr | Attr | test.rs:97:5:97:23 | ...::exit(...) | ...::exit(...) |
+| test.rs:142:4:142:9 | bad4_1 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:141:1:141:7 | Attr | Attr | test.rs:143:5:143:15 | ...::stdout(...) | ...::stdout(...) |
diff --git a/rust/ql/test/query-tests/security/CWE-696/test.rs b/rust/ql/test/query-tests/security/CWE-696/test.rs
index 9fb90a018e6b..4041efcae173 100644
--- a/rust/ql/test/query-tests/security/CWE-696/test.rs
+++ b/rust/ql/test/query-tests/security/CWE-696/test.rs
@@ -27,19 +27,19 @@ fn harmless1_5() {
 }
 
 #[ctor::ctor]
-fn bad1_1() { // $ MISSING: Alert[rust/ctor-initialization]
+fn bad1_1() { // $ Alert[rust/ctor-initialization]
     _ = std::io::stdout().write(b"Hello, world!");
 }
 
 #[ctor::dtor]
-fn bad1_2() { // $ MISSING: Alert[rust/ctor-initialization]
+fn bad1_2() { // $ Alert[rust/ctor-initialization]
     _ = std::io::stdout().write(b"Hello, world!");
 }
 
 #[rustfmt::skip]
 #[ctor::dtor]
 #[rustfmt::skip]
-fn bad1_3() { // $ MISSING: Alert[rust/ctor-initialization]
+fn bad1_3() { // $ Alert[rust/ctor-initialization]
     _ = std::io::stdout().write(b"Hello, world!");
 }
 
@@ -49,22 +49,22 @@ use ctor::ctor;
 use std::io::*;
 
 #[ctor]
-fn bad2_1() { // $ MISSING: Alert[rust/ctor-initialization]
+fn bad2_1() { // $ Alert[rust/ctor-initialization]
     _ = stdout().write(b"Hello, world!");
 }
 
 #[ctor]
-fn bad2_2() { // $ MISSING: Alert[rust/ctor-initialization]
+fn bad2_2() { // $ Alert[rust/ctor-initialization]
     _ = stderr().write_all(b"Hello, world!");
 }
 
 #[ctor]
-fn bad2_3() { // $ MISSING: Alert[rust/ctor-initialization]
+fn bad2_3() { // $ Alert[rust/ctor-initialization]
     println!("Hello, world!");
 }
 
 #[ctor]
-fn bad2_4() { // $ MISSING: Alert[rust/ctor-initialization]
+fn bad2_4() { // $ Alert[rust/ctor-initialization]
     let mut buff = String::new();
     _ = std::io::stdin().read_line(&mut buff);
 }
@@ -86,14 +86,14 @@ use std::time::Duration;
 const DURATION2_7: Duration = Duration::new(1, 0);
 
 #[ctor]
-fn bad2_7() { // $ MISSING: Alert[rust/ctor-initialization]
+fn bad2_7() { // $ Alert[rust/ctor-initialization]
     std::thread::sleep(DURATION2_7);
 }
 
 use std::process;
 
 #[ctor]
-fn bad2_8() { // $ MISSING: Alert[rust/ctor-initialization]
+fn bad2_8() { // $ Alert[rust/ctor-initialization]
     process::exit(1234);
 }
 
@@ -139,6 +139,6 @@ macro_rules! macro4_1 {
 }
 
 #[ctor]
-fn bad4_1() { // $ MISSING: Alert[rust/ctor-initialization]
+fn bad4_1() { // $ Alert[rust/ctor-initialization]
     macro4_1!();
 }

From 82f2c6075f30198bc48be6eeac82824717e1d939 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 25 Nov 2024 12:00:55 +0000
Subject: [PATCH 04/51] Rust: Add qhelp + examples.

---
 .../CWE-696/BadCtorInitialization.qhelp       | 44 +++++++++++++++++++
 .../CWE-696/BadCtorInitializationBad.rs       |  5 +++
 .../CWE-696/BadCtorInitializationGood.rs      |  5 +++
 3 files changed, 54 insertions(+)
 create mode 100644 rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
 create mode 100644 rust/ql/src/queries/security/CWE-696/BadCtorInitializationBad.rs
 create mode 100644 rust/ql/src/queries/security/CWE-696/BadCtorInitializationGood.rs

diff --git a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
new file mode 100644
index 000000000000..60f6e3eb45d0
--- /dev/null
+++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
@@ -0,0 +1,44 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+
+<p>
+Calling functions and methods in the Rust <code>std</code> library from a <code>#[ctor]</code> or <code>#[dtor]<code> function is not safe. This is because the <code>std</code> library only guarantees stability and portability between the beginning and end of <code>main</code>, whereas <code>#[ctor]</code> functions are called before <code>main</code>, and <code>#[dtor]</code> functions are called after it.
+</p>
+
+</overview>
+<recommendation>
+
+<p>
+Do not call any part of the <code>std</code> library from a <code>#[ctor]</code> or <code>#[dtor]<code> function. Instead either:
+</p>
+<ul>
+  <li>Move the code to a different location, such as inside your program's <code>main</code> function.</li>
+  <li>Rewrite the code using an alternative library.</li>
+</ul>
+
+</recommendation>
+<example>
+
+<p>
+In the following example, a <code>#[ctor]</code> function uses the <code>println!</code> macro which calls <code>std</code> library functions. This may cause unexpected behaviour at runtime.
+</p>
+
+<sample src="BadCtorInitializationBad.rs" />
+
+<p>
+The issue can be fixed by replacing <code>println!</code> with something that does not rely on the <code>std</code> library. In the fixed code below we use the <code>libc_println!</code> macro from the <code>libc-print</code> library:
+</p>
+
+<sample src="BadCtorInitializationGood.rs" />
+
+</example>
+<references>
+
+<li>GitHub: <a href="https://github.com/mmastrac/rust-ctor?tab=readme-ov-file#warnings">rust-ctor - Warnings</a>.</li>
+<li>Rust Programming Language: <a href="https://doc.rust-lang.org/std/#use-before-and-after-main">Crate std - Use before and after main()</a>.</li>
+
+</references>
+</qhelp>
diff --git a/rust/ql/src/queries/security/CWE-696/BadCtorInitializationBad.rs b/rust/ql/src/queries/security/CWE-696/BadCtorInitializationBad.rs
new file mode 100644
index 000000000000..2a6c60272d5c
--- /dev/null
+++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitializationBad.rs
@@ -0,0 +1,5 @@
+
+#[ctor::ctor]
+fn bad_example() {
+    println!("Hello, world!"); // BAD: the println! macro calls std library functions
+}
diff --git a/rust/ql/src/queries/security/CWE-696/BadCtorInitializationGood.rs b/rust/ql/src/queries/security/CWE-696/BadCtorInitializationGood.rs
new file mode 100644
index 000000000000..cbb8bc089a3f
--- /dev/null
+++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitializationGood.rs
@@ -0,0 +1,5 @@
+
+#[ctor::ctor]
+fn good_example() {
+    libc_print::libc_println!("Hello, world!"); // GOOD: libc-print does not use the std library
+}

From be5bd1da0a2a82d91688842920cecf56d244f075 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 25 Nov 2024 14:57:30 +0000
Subject: [PATCH 05/51] Rust: Also add the good example and a couple of other
 cited good cases to the test.

---
 .../CWE-696/BadCTorInitialization.expected    |  2 +-
 .../query-tests/security/CWE-696/options.yml  |  1 +
 .../test/query-tests/security/CWE-696/test.rs | 23 +++++++++++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected b/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
index 36ab89a79bf4..10313da17a6c 100644
--- a/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
+++ b/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
@@ -7,4 +7,4 @@
 | test.rs:67:4:67:9 | bad2_4 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:66:1:66:7 | Attr | Attr | test.rs:69:9:69:24 | ...::stdin(...) | ...::stdin(...) |
 | test.rs:89:4:89:9 | bad2_7 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:88:1:88:7 | Attr | Attr | test.rs:90:5:90:35 | ...::sleep(...) | ...::sleep(...) |
 | test.rs:96:4:96:9 | bad2_8 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:95:1:95:7 | Attr | Attr | test.rs:97:5:97:23 | ...::exit(...) | ...::exit(...) |
-| test.rs:142:4:142:9 | bad4_1 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:141:1:141:7 | Attr | Attr | test.rs:143:5:143:15 | ...::stdout(...) | ...::stdout(...) |
+| test.rs:165:4:165:9 | bad4_1 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:164:1:164:7 | Attr | Attr | test.rs:166:5:166:15 | ...::stdout(...) | ...::stdout(...) |
diff --git a/rust/ql/test/query-tests/security/CWE-696/options.yml b/rust/ql/test/query-tests/security/CWE-696/options.yml
index 1f1a13e589ea..cc3dc0f35196 100644
--- a/rust/ql/test/query-tests/security/CWE-696/options.yml
+++ b/rust/ql/test/query-tests/security/CWE-696/options.yml
@@ -1,3 +1,4 @@
 qltest_cargo_check: true
 qltest_dependencies:
     - ctor = { version = "0.2.9" }
+    - libc-print = { version = "0.1.23" }
diff --git a/rust/ql/test/query-tests/security/CWE-696/test.rs b/rust/ql/test/query-tests/security/CWE-696/test.rs
index 4041efcae173..2510a833b0f6 100644
--- a/rust/ql/test/query-tests/security/CWE-696/test.rs
+++ b/rust/ql/test/query-tests/security/CWE-696/test.rs
@@ -97,6 +97,29 @@ fn bad2_8() { // $ Alert[rust/ctor-initialization]
     process::exit(1234);
 }
 
+#[ctor::ctor]
+fn harmless2_9() {
+    libc_print::libc_println!("Hello, world!"); // does not use the std library
+}
+
+#[ctor::ctor]
+fn harmless2_10() {
+    core::assert!(true); // core library should be OK in this context
+}
+
+extern crate alloc;
+use alloc::alloc::{alloc, dealloc, Layout};
+
+#[ctor::ctor]
+unsafe fn harmless2_11() {
+    let layout = Layout::new::<u64>();
+    let ptr = alloc(layout); // alloc library should be OK in this context
+
+    if !ptr.is_null() {
+        dealloc(ptr, layout);
+    }
+}
+
 // --- transitive cases ---
 
 fn call_target3_1() {

From 77f51685903a056f2753b61b3fb11f7061fc6f19 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 25 Nov 2024 17:48:42 +0000
Subject: [PATCH 06/51] Rust: Query metadata and path edges.

---
 .../security/CWE-696/BadCtorInitialization.ql | 42 ++++++---
 .../CWE-696/BadCTorInitialization.expected    | 32 ++++---
 .../test/query-tests/security/CWE-696/test.rs | 86 +++++++++----------
 3 files changed, 96 insertions(+), 64 deletions(-)

diff --git a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
index 5999ac0e02ec..90a7acc69208 100644
--- a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
+++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
@@ -1,12 +1,12 @@
 /**
  * @name Bad 'ctor' initialization
- * @description TODO
+ * @description Calling functions in the Rust std library from a ctor or dtor function is not safe.
  * @kind path-problem
  * @problem.severity error
- * @ security-severity TODO
- * @ precision TODO
+ * @precision high
  * @id rust/ctor-initialization
- * @tags security
+ * @tags reliability
+ *       correctness
  *       external/cwe/cwe-696
  *       external/cwe/cwe-665
  */
@@ -17,7 +17,14 @@ import rust
  * A `#[ctor]` or `#[dtor]` attribute.
  */
 class CtorAttr extends Attr {
-  CtorAttr() { this.getMeta().getPath().getPart().getNameRef().getText() = ["ctor", "dtor"] }
+  string whichAttr;
+
+  CtorAttr() {
+    whichAttr = this.getMeta().getPath().getPart().getNameRef().getText() and
+    whichAttr = ["ctor", "dtor"]
+  }
+
+  string getWhichAttr() { result = whichAttr }
 }
 
 /**
@@ -30,9 +37,22 @@ class StdCall extends Expr {
   }
 }
 
-from CtorAttr ctor, Function f, StdCall call
-where
-  f.getAnAttr() = ctor and
-  call.getEnclosingCallable() = f
-select f.getName(), "This function has the $@ attribute but calls $@ in the standard library.",
-  ctor, ctor.toString(), call, call.toString()
+class PathElement = AstNode;
+
+query predicate edges(PathElement pred, PathElement succ) {
+  // starting edge
+  exists(CtorAttr ctor, Function f, StdCall call |
+    f.getAnAttr() = ctor and
+    call.getEnclosingCallable() = f and
+    pred = ctor and // source
+    succ = call // sink
+  )
+  // or
+  // transitive edge
+  // TODO
+}
+
+from CtorAttr ctor, StdCall call
+where edges*(ctor, call)
+select call, ctor, call, "Call to $@ in a function with the " + ctor.getWhichAttr() + " attribute.",
+  call, call.toString()
diff --git a/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected b/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
index 10313da17a6c..8ad81870e06d 100644
--- a/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
+++ b/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
@@ -1,10 +1,22 @@
-| test.rs:30:4:30:9 | bad1_1 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:29:1:29:13 | Attr | Attr | test.rs:31:9:31:25 | ...::stdout(...) | ...::stdout(...) |
-| test.rs:35:4:35:9 | bad1_2 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:34:1:34:13 | Attr | Attr | test.rs:36:9:36:25 | ...::stdout(...) | ...::stdout(...) |
-| test.rs:42:4:42:9 | bad1_3 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:40:1:40:13 | Attr | Attr | test.rs:43:9:43:25 | ...::stdout(...) | ...::stdout(...) |
-| test.rs:52:4:52:9 | bad2_1 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:51:1:51:7 | Attr | Attr | test.rs:53:9:53:16 | stdout(...) | stdout(...) |
-| test.rs:57:4:57:9 | bad2_2 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:56:1:56:7 | Attr | Attr | test.rs:58:9:58:16 | stderr(...) | stderr(...) |
-| test.rs:62:4:62:9 | bad2_3 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:61:1:61:7 | Attr | Attr | test.rs:63:14:63:28 | ...::_print(...) | ...::_print(...) |
-| test.rs:67:4:67:9 | bad2_4 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:66:1:66:7 | Attr | Attr | test.rs:69:9:69:24 | ...::stdin(...) | ...::stdin(...) |
-| test.rs:89:4:89:9 | bad2_7 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:88:1:88:7 | Attr | Attr | test.rs:90:5:90:35 | ...::sleep(...) | ...::sleep(...) |
-| test.rs:96:4:96:9 | bad2_8 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:95:1:95:7 | Attr | Attr | test.rs:97:5:97:23 | ...::exit(...) | ...::exit(...) |
-| test.rs:165:4:165:9 | bad4_1 | This function has the $@ attribute but calls $@ in the standard library. | test.rs:164:1:164:7 | Attr | Attr | test.rs:166:5:166:15 | ...::stdout(...) | ...::stdout(...) |
+#select
+| test.rs:31:9:31:25 | ...::stdout(...) | test.rs:29:1:29:13 | Attr | test.rs:31:9:31:25 | ...::stdout(...) | Call to $@ in a function with the ctor attribute. | test.rs:31:9:31:25 | ...::stdout(...) | ...::stdout(...) |
+| test.rs:36:9:36:25 | ...::stdout(...) | test.rs:34:1:34:13 | Attr | test.rs:36:9:36:25 | ...::stdout(...) | Call to $@ in a function with the dtor attribute. | test.rs:36:9:36:25 | ...::stdout(...) | ...::stdout(...) |
+| test.rs:43:9:43:25 | ...::stdout(...) | test.rs:40:1:40:13 | Attr | test.rs:43:9:43:25 | ...::stdout(...) | Call to $@ in a function with the dtor attribute. | test.rs:43:9:43:25 | ...::stdout(...) | ...::stdout(...) |
+| test.rs:53:9:53:16 | stdout(...) | test.rs:51:1:51:7 | Attr | test.rs:53:9:53:16 | stdout(...) | Call to $@ in a function with the ctor attribute. | test.rs:53:9:53:16 | stdout(...) | stdout(...) |
+| test.rs:58:9:58:16 | stderr(...) | test.rs:56:1:56:7 | Attr | test.rs:58:9:58:16 | stderr(...) | Call to $@ in a function with the ctor attribute. | test.rs:58:9:58:16 | stderr(...) | stderr(...) |
+| test.rs:63:14:63:28 | ...::_print(...) | test.rs:61:1:61:7 | Attr | test.rs:63:14:63:28 | ...::_print(...) | Call to $@ in a function with the ctor attribute. | test.rs:63:14:63:28 | ...::_print(...) | ...::_print(...) |
+| test.rs:69:9:69:24 | ...::stdin(...) | test.rs:66:1:66:7 | Attr | test.rs:69:9:69:24 | ...::stdin(...) | Call to $@ in a function with the ctor attribute. | test.rs:69:9:69:24 | ...::stdin(...) | ...::stdin(...) |
+| test.rs:90:5:90:35 | ...::sleep(...) | test.rs:88:1:88:7 | Attr | test.rs:90:5:90:35 | ...::sleep(...) | Call to $@ in a function with the ctor attribute. | test.rs:90:5:90:35 | ...::sleep(...) | ...::sleep(...) |
+| test.rs:97:5:97:23 | ...::exit(...) | test.rs:95:1:95:7 | Attr | test.rs:97:5:97:23 | ...::exit(...) | Call to $@ in a function with the ctor attribute. | test.rs:97:5:97:23 | ...::exit(...) | ...::exit(...) |
+| test.rs:166:5:166:15 | ...::stdout(...) | test.rs:164:1:164:7 | Attr | test.rs:166:5:166:15 | ...::stdout(...) | Call to $@ in a function with the ctor attribute. | test.rs:166:5:166:15 | ...::stdout(...) | ...::stdout(...) |
+edges
+| test.rs:29:1:29:13 | Attr | test.rs:31:9:31:25 | ...::stdout(...) |
+| test.rs:34:1:34:13 | Attr | test.rs:36:9:36:25 | ...::stdout(...) |
+| test.rs:40:1:40:13 | Attr | test.rs:43:9:43:25 | ...::stdout(...) |
+| test.rs:51:1:51:7 | Attr | test.rs:53:9:53:16 | stdout(...) |
+| test.rs:56:1:56:7 | Attr | test.rs:58:9:58:16 | stderr(...) |
+| test.rs:61:1:61:7 | Attr | test.rs:63:14:63:28 | ...::_print(...) |
+| test.rs:66:1:66:7 | Attr | test.rs:69:9:69:24 | ...::stdin(...) |
+| test.rs:88:1:88:7 | Attr | test.rs:90:5:90:35 | ...::sleep(...) |
+| test.rs:95:1:95:7 | Attr | test.rs:97:5:97:23 | ...::exit(...) |
+| test.rs:164:1:164:7 | Attr | test.rs:166:5:166:15 | ...::stdout(...) |
diff --git a/rust/ql/test/query-tests/security/CWE-696/test.rs b/rust/ql/test/query-tests/security/CWE-696/test.rs
index 2510a833b0f6..87f544be85c1 100644
--- a/rust/ql/test/query-tests/security/CWE-696/test.rs
+++ b/rust/ql/test/query-tests/security/CWE-696/test.rs
@@ -26,21 +26,21 @@ fn harmless1_5() {
     _ = std::io::stdout().write(b"Hello, world!");
 }
 
-#[ctor::ctor]
-fn bad1_1() { // $ Alert[rust/ctor-initialization]
-    _ = std::io::stdout().write(b"Hello, world!");
+#[ctor::ctor] // $ Source=source1_1
+fn bad1_1() {
+    _ = std::io::stdout().write(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source1_1
 }
 
-#[ctor::dtor]
-fn bad1_2() { // $ Alert[rust/ctor-initialization]
-    _ = std::io::stdout().write(b"Hello, world!");
+#[ctor::dtor] // $ Source=source1_2
+fn bad1_2() {
+    _ = std::io::stdout().write(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source1_2
 }
 
 #[rustfmt::skip]
-#[ctor::dtor]
+#[ctor::dtor] // $ Source=source1_3
 #[rustfmt::skip]
-fn bad1_3() { // $ Alert[rust/ctor-initialization]
-    _ = std::io::stdout().write(b"Hello, world!");
+fn bad1_3() {
+    _ = std::io::stdout().write(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source1_3
 }
 
 // --- code variants ---
@@ -48,53 +48,53 @@ fn bad1_3() { // $ Alert[rust/ctor-initialization]
 use ctor::ctor;
 use std::io::*;
 
-#[ctor]
-fn bad2_1() { // $ Alert[rust/ctor-initialization]
-    _ = stdout().write(b"Hello, world!");
+#[ctor] // $ Source=source2_1
+fn bad2_1() {
+    _ = stdout().write(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source2_1
 }
 
-#[ctor]
-fn bad2_2() { // $ Alert[rust/ctor-initialization]
-    _ = stderr().write_all(b"Hello, world!");
+#[ctor] // $ Source=source2_2
+fn bad2_2() {
+    _ = stderr().write_all(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source2_2
 }
 
-#[ctor]
-fn bad2_3() { // $ Alert[rust/ctor-initialization]
-    println!("Hello, world!");
+#[ctor] // $ Source=source2_3
+fn bad2_3() {
+    println!("Hello, world!"); // $ Alert[rust/ctor-initialization]=source2_3
 }
 
-#[ctor]
-fn bad2_4() { // $ Alert[rust/ctor-initialization]
+#[ctor] // $ Source=source2_4
+fn bad2_4() {
     let mut buff = String::new();
-    _ = std::io::stdin().read_line(&mut buff);
+    _ = std::io::stdin().read_line(&mut buff); // $ Alert[rust/ctor-initialization]=source2_4
 }
 
 use std::fs;
 
-#[ctor]
-fn bad2_5() { // $ MISSING: Alert[rust/ctor-initialization]
-    let _buff = fs::File::create("hello.txt").unwrap();
+#[ctor] // $ MISSING: Source=source2_5
+fn bad2_5() {
+    let _buff = fs::File::create("hello.txt").unwrap(); // $ MISSING: Alert[rust/ctor-initialization]=source2_5
 }
 
-#[ctor]
-fn bad2_6() { // $ MISSING: Alert[rust/ctor-initialization]
-    let _t = std::time::Instant::now();
+#[ctor] // $ MISSING: Source=source2_6
+fn bad2_6() {
+    let _t = std::time::Instant::now(); // $ MISSING: Alert[rust/ctor-initialization]=source2_6
 }
 
 use std::time::Duration;
 
 const DURATION2_7: Duration = Duration::new(1, 0);
 
-#[ctor]
-fn bad2_7() { // $ Alert[rust/ctor-initialization]
-    std::thread::sleep(DURATION2_7);
+#[ctor] // $ Source=source2_7
+fn bad2_7() {
+    std::thread::sleep(DURATION2_7); // $ Alert[rust/ctor-initialization]=source2_7
 }
 
 use std::process;
 
-#[ctor]
-fn bad2_8() { // $ Alert[rust/ctor-initialization]
-    process::exit(1234);
+#[ctor] // $ Source=source2_8
+fn bad2_8() {
+    process::exit(1234); // $ Alert[rust/ctor-initialization]=source2_8
 }
 
 #[ctor::ctor]
@@ -123,11 +123,11 @@ unsafe fn harmless2_11() {
 // --- transitive cases ---
 
 fn call_target3_1() {
-    _ = stderr().write_all(b"Hello, world!");
+    _ = stderr().write_all(b"Hello, world!"); // $ MISSING: Alert=source3_1 Alert=source3_3 Alert=source3_4
 }
 
-#[ctor]
-fn bad3_1() { // $ MISSING: Alert[rust/ctor-initialization]
+#[ctor] // $ MISSING: Source=source3_1
+fn bad3_1() {
     call_target3_1();
 }
 
@@ -137,19 +137,19 @@ fn call_target3_2() {
     }
 }
 
-#[ctor]
+#[ctor] // $ MISSING: Source=source3_2
 fn harmless3_2() {
     call_target3_2();
 }
 
 #[ctor]
-fn bad3_3() { // $ MISSING: Alert[rust/ctor-initialization]
+fn bad3_3() {
     call_target3_1();
     call_target3_2();
 }
 
-#[ctor]
-fn bad3_4() { // $ MISSING: Alert[rust/ctor-initialization]
+#[ctor] // $ MISSING: Source=source3_4
+fn bad3_4() {
     bad3_3();
 }
 
@@ -161,7 +161,7 @@ macro_rules! macro4_1 {
     };
 }
 
-#[ctor]
-fn bad4_1() { // $ Alert[rust/ctor-initialization]
-    macro4_1!();
+#[ctor] // $ Source=source4_1
+fn bad4_1() {
+    macro4_1!(); // $ Alert[rust/ctor-initialization]=source4_1
 }

From e8981a505d2fb1db293c801b25cbe80f51271a58 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 25 Nov 2024 20:00:22 +0000
Subject: [PATCH 07/51] Rust: Fix qhelp.

---
 .../ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
index 60f6e3eb45d0..3a3b2e48c6d0 100644
--- a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
+++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
@@ -5,7 +5,7 @@
 <overview>
 
 <p>
-Calling functions and methods in the Rust <code>std</code> library from a <code>#[ctor]</code> or <code>#[dtor]<code> function is not safe. This is because the <code>std</code> library only guarantees stability and portability between the beginning and end of <code>main</code>, whereas <code>#[ctor]</code> functions are called before <code>main</code>, and <code>#[dtor]</code> functions are called after it.
+Calling functions and methods in the Rust <code>std</code> library from a <code>#[ctor]</code> or <code>#[dtor]</code> function is not safe. This is because the <code>std</code> library only guarantees stability and portability between the beginning and end of <code>main</code>, whereas <code>#[ctor]</code> functions are called before <code>main</code>, and <code>#[dtor]</code> functions are called after it.
 </p>
 
 </overview>

From e6302cae535e38eaca403a1dc868a968d9393a17 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 25 Nov 2024 20:07:47 +0000
Subject: [PATCH 08/51] Rust: Address CI and ql-for-ql issues.

---
 .../CWE-696/BadCtorInitialization.qhelp       |  2 +-
 .../security/CWE-696/BadCtorInitialization.ql |  3 +--
 .../CWE-696/BadCTorInitialization.expected    | 20 +++++++++----------
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
index 3a3b2e48c6d0..82683ccfdba4 100644
--- a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
+++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
@@ -12,7 +12,7 @@ Calling functions and methods in the Rust <code>std</code> library from a <code>
 <recommendation>
 
 <p>
-Do not call any part of the <code>std</code> library from a <code>#[ctor]</code> or <code>#[dtor]<code> function. Instead either:
+Do not call any part of the <code>std</code> library from a <code>#[ctor]</code> or <code>#[dtor]</code> function. Instead either:
 </p>
 <ul>
   <li>Move the code to a different location, such as inside your program's <code>main</code> function.</li>
diff --git a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
index 90a7acc69208..9d9c698db245 100644
--- a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
+++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
@@ -54,5 +54,4 @@ query predicate edges(PathElement pred, PathElement succ) {
 
 from CtorAttr ctor, StdCall call
 where edges*(ctor, call)
-select call, ctor, call, "Call to $@ in a function with the " + ctor.getWhichAttr() + " attribute.",
-  call, call.toString()
+select call, ctor, call, "Call to " + call.toString() + " in a function with the " + ctor.getWhichAttr() + " attribute."
diff --git a/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected b/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
index 8ad81870e06d..508a359b0c0b 100644
--- a/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
+++ b/rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
@@ -1,14 +1,14 @@
 #select
-| test.rs:31:9:31:25 | ...::stdout(...) | test.rs:29:1:29:13 | Attr | test.rs:31:9:31:25 | ...::stdout(...) | Call to $@ in a function with the ctor attribute. | test.rs:31:9:31:25 | ...::stdout(...) | ...::stdout(...) |
-| test.rs:36:9:36:25 | ...::stdout(...) | test.rs:34:1:34:13 | Attr | test.rs:36:9:36:25 | ...::stdout(...) | Call to $@ in a function with the dtor attribute. | test.rs:36:9:36:25 | ...::stdout(...) | ...::stdout(...) |
-| test.rs:43:9:43:25 | ...::stdout(...) | test.rs:40:1:40:13 | Attr | test.rs:43:9:43:25 | ...::stdout(...) | Call to $@ in a function with the dtor attribute. | test.rs:43:9:43:25 | ...::stdout(...) | ...::stdout(...) |
-| test.rs:53:9:53:16 | stdout(...) | test.rs:51:1:51:7 | Attr | test.rs:53:9:53:16 | stdout(...) | Call to $@ in a function with the ctor attribute. | test.rs:53:9:53:16 | stdout(...) | stdout(...) |
-| test.rs:58:9:58:16 | stderr(...) | test.rs:56:1:56:7 | Attr | test.rs:58:9:58:16 | stderr(...) | Call to $@ in a function with the ctor attribute. | test.rs:58:9:58:16 | stderr(...) | stderr(...) |
-| test.rs:63:14:63:28 | ...::_print(...) | test.rs:61:1:61:7 | Attr | test.rs:63:14:63:28 | ...::_print(...) | Call to $@ in a function with the ctor attribute. | test.rs:63:14:63:28 | ...::_print(...) | ...::_print(...) |
-| test.rs:69:9:69:24 | ...::stdin(...) | test.rs:66:1:66:7 | Attr | test.rs:69:9:69:24 | ...::stdin(...) | Call to $@ in a function with the ctor attribute. | test.rs:69:9:69:24 | ...::stdin(...) | ...::stdin(...) |
-| test.rs:90:5:90:35 | ...::sleep(...) | test.rs:88:1:88:7 | Attr | test.rs:90:5:90:35 | ...::sleep(...) | Call to $@ in a function with the ctor attribute. | test.rs:90:5:90:35 | ...::sleep(...) | ...::sleep(...) |
-| test.rs:97:5:97:23 | ...::exit(...) | test.rs:95:1:95:7 | Attr | test.rs:97:5:97:23 | ...::exit(...) | Call to $@ in a function with the ctor attribute. | test.rs:97:5:97:23 | ...::exit(...) | ...::exit(...) |
-| test.rs:166:5:166:15 | ...::stdout(...) | test.rs:164:1:164:7 | Attr | test.rs:166:5:166:15 | ...::stdout(...) | Call to $@ in a function with the ctor attribute. | test.rs:166:5:166:15 | ...::stdout(...) | ...::stdout(...) |
+| test.rs:31:9:31:25 | ...::stdout(...) | test.rs:29:1:29:13 | Attr | test.rs:31:9:31:25 | ...::stdout(...) | Call to ...::stdout(...) in a function with the ctor attribute. |
+| test.rs:36:9:36:25 | ...::stdout(...) | test.rs:34:1:34:13 | Attr | test.rs:36:9:36:25 | ...::stdout(...) | Call to ...::stdout(...) in a function with the dtor attribute. |
+| test.rs:43:9:43:25 | ...::stdout(...) | test.rs:40:1:40:13 | Attr | test.rs:43:9:43:25 | ...::stdout(...) | Call to ...::stdout(...) in a function with the dtor attribute. |
+| test.rs:53:9:53:16 | stdout(...) | test.rs:51:1:51:7 | Attr | test.rs:53:9:53:16 | stdout(...) | Call to stdout(...) in a function with the ctor attribute. |
+| test.rs:58:9:58:16 | stderr(...) | test.rs:56:1:56:7 | Attr | test.rs:58:9:58:16 | stderr(...) | Call to stderr(...) in a function with the ctor attribute. |
+| test.rs:63:14:63:28 | ...::_print(...) | test.rs:61:1:61:7 | Attr | test.rs:63:14:63:28 | ...::_print(...) | Call to ...::_print(...) in a function with the ctor attribute. |
+| test.rs:69:9:69:24 | ...::stdin(...) | test.rs:66:1:66:7 | Attr | test.rs:69:9:69:24 | ...::stdin(...) | Call to ...::stdin(...) in a function with the ctor attribute. |
+| test.rs:90:5:90:35 | ...::sleep(...) | test.rs:88:1:88:7 | Attr | test.rs:90:5:90:35 | ...::sleep(...) | Call to ...::sleep(...) in a function with the ctor attribute. |
+| test.rs:97:5:97:23 | ...::exit(...) | test.rs:95:1:95:7 | Attr | test.rs:97:5:97:23 | ...::exit(...) | Call to ...::exit(...) in a function with the ctor attribute. |
+| test.rs:166:5:166:15 | ...::stdout(...) | test.rs:164:1:164:7 | Attr | test.rs:166:5:166:15 | ...::stdout(...) | Call to ...::stdout(...) in a function with the ctor attribute. |
 edges
 | test.rs:29:1:29:13 | Attr | test.rs:31:9:31:25 | ...::stdout(...) |
 | test.rs:34:1:34:13 | Attr | test.rs:36:9:36:25 | ...::stdout(...) |

From 28c0e899b7e4ce67e3d1ea7c8ef3e752ba24d5ae Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Mon, 25 Nov 2024 20:50:56 +0000
Subject: [PATCH 09/51] Rust: Autoformat.

---
 rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
index 9d9c698db245..8d434b1f6e4d 100644
--- a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
+++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
@@ -54,4 +54,5 @@ query predicate edges(PathElement pred, PathElement succ) {
 
 from CtorAttr ctor, StdCall call
 where edges*(ctor, call)
-select call, ctor, call, "Call to " + call.toString() + " in a function with the " + ctor.getWhichAttr() + " attribute."
+select call, ctor, call,
+  "Call to " + call.toString() + " in a function with the " + ctor.getWhichAttr() + " attribute."

From 7402276ec7be327b4bec3c0c6bc2907a26f3bde0 Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Tue, 26 Nov 2024 14:44:18 +0100
Subject: [PATCH 10/51] Data flow: Move more logic into `DataFlowImplCommon`

---
 .../cpp/ir/dataflow/internal/ProductFlow.qll  |   4 +-
 .../codeql/dataflow/internal/DataFlowImpl.qll |  76 +++------
 .../dataflow/internal/DataFlowImplCommon.qll  | 158 +++++++++++++-----
 .../dataflow/internal/FlowSummaryImpl.qll     |   2 +-
 .../internal/ModelGeneratorImpl.qll           |   6 +-
 5 files changed, 147 insertions(+), 99 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ProductFlow.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ProductFlow.qll
index 0c474b0e75d0..ff5f3e46e648 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ProductFlow.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ProductFlow.qll
@@ -545,7 +545,7 @@ module ProductFlow {
     private predicate outImpl1(Flow1::PathNode pred1, Flow1::PathNode succ1, DataFlowCall call) {
       Flow1::PathGraph::edges(pred1, succ1, _, _) and
       exists(ReturnKindExt returnKind |
-        succ1.getNode() = returnKind.getAnOutNode(call) and
+        succ1.getNode() = getAnOutNodeExt(call, returnKind) and
         returnKind = getParamReturnPosition(_, pred1.asParameterReturnNode()).getKind()
       )
     }
@@ -573,7 +573,7 @@ module ProductFlow {
     private predicate outImpl2(Flow2::PathNode pred2, Flow2::PathNode succ2, DataFlowCall call) {
       Flow2::PathGraph::edges(pred2, succ2, _, _) and
       exists(ReturnKindExt returnKind |
-        succ2.getNode() = returnKind.getAnOutNode(call) and
+        succ2.getNode() = getAnOutNodeExt(call, returnKind) and
         returnKind = getParamReturnPosition(_, pred2.asParameterReturnNode()).getKind()
       )
     }
diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll
index d4c73c234beb..c67d21fb1ac1 100644
--- a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll
+++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll
@@ -343,7 +343,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
     bindingset[n, cc]
     pragma[inline_late]
     private predicate isUnreachableInCall1(NodeEx n, LocalCallContextSpecificCall cc) {
-      cc.unreachable(n.asNode())
+      cc.unreachable(n)
     }
 
     /**
@@ -423,7 +423,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
     pragma[nomagic]
     private predicate readSetEx(NodeEx node1, ContentSet c, NodeEx node2) {
-      readSet(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode())) and
+      readEx(node1, c, node2) and
       stepFilter(node1, node2)
       or
       exists(Node n |
@@ -450,20 +450,19 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
     bindingset[c]
     private predicate expectsContentEx(NodeEx n, Content c) {
       exists(ContentSet cs |
-        expectsContentCached(n.asNode(), cs) and
+        expectsContentSet(n, cs) and
         pragma[only_bind_out](c) = pragma[only_bind_into](cs).getAReadContent()
       )
     }
 
     pragma[nomagic]
-    private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
+    private predicate notExpectsContent(NodeEx n) { not expectsContentSet(n, _) }
 
     pragma[nomagic]
     private predicate storeExUnrestricted(
       NodeEx node1, Content c, NodeEx node2, DataFlowType contentType, DataFlowType containerType
     ) {
-      store(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode()),
-        contentType, containerType) and
+      storeEx(node1, c, node2, contentType, containerType) and
       stepFilter(node1, node2)
     }
 
@@ -471,23 +470,13 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
     private predicate hasReadStep(Content c) { read(_, c, _) }
 
     pragma[nomagic]
-    private predicate storeEx(
+    private predicate storeExRestricted(
       NodeEx node1, Content c, NodeEx node2, DataFlowType contentType, DataFlowType containerType
     ) {
       storeExUnrestricted(node1, c, node2, contentType, containerType) and
       hasReadStep(c)
     }
 
-    pragma[nomagic]
-    private predicate viableReturnPosOutEx(DataFlowCall call, ReturnPosition pos, NodeEx out) {
-      viableReturnPosOut(call, pos, out.asNode())
-    }
-
-    pragma[nomagic]
-    private predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx arg) {
-      viableParamArg(call, p.asNode(), arg.asNode())
-    }
-
     /**
      * Holds if field flow should be used for the given configuration.
      */
@@ -520,7 +509,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       exists(ParameterPosition pos | p.isParameterOf(_, pos) |
         not kind.(ParamUpdateReturnKind).getPosition() = pos
         or
-        allowParameterReturnInSelfCached(p.asNode())
+        allowParameterReturnInSelfEx(p)
       )
     }
 
@@ -558,7 +547,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         exists(NodeEx mid |
           useFieldFlow() and
           fwdFlow(mid, cc) and
-          storeEx(mid, _, node, _, _)
+          storeExRestricted(mid, _, node, _, _)
         )
         or
         // read
@@ -653,7 +642,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
           not fullBarrier(node) and
           useFieldFlow() and
           fwdFlow(mid, _) and
-          storeEx(mid, c, node, _, _)
+          storeExRestricted(mid, c, node, _, _)
         )
       }
 
@@ -796,7 +785,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         exists(NodeEx mid |
           revFlow(mid, toReturn) and
           fwdFlowConsCand(c) and
-          storeEx(node, c, mid, _, _)
+          storeExRestricted(node, c, mid, _, _)
         )
       }
 
@@ -893,7 +882,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       ) {
         revFlowIsReadAndStored(c) and
         revFlow(node2) and
-        storeEx(node1, c, node2, contentType, containerType) and
+        storeExRestricted(node1, c, node2, contentType, containerType) and
         exists(ap1)
       }
 
@@ -1152,7 +1141,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         flowOutOfCallNodeCand1(call, ret, _, out) and
         c = ret.getEnclosingCallable()
       |
-        scope = getSecondLevelScopeCached(ret.asNode())
+        scope = getSecondLevelScopeEx(ret)
         or
         ret = TParamReturnNode(_, scope)
       )
@@ -1496,7 +1485,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
           PrevStage::revFlow(node, state, apa) and
           filter(node, state, t0, ap, t) and
           (
-            if castingNodeEx(node)
+            if node instanceof CastingNodeEx
             then
               ap instanceof ApNil or
               compatibleContainer(getHeadContent(ap), node.getDataFlowType()) or
@@ -2627,10 +2616,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
             FlowCheckNode() {
               revFlow(this, _, _) and
               (
-                castNode(this.asNode()) or
-                clearsContentCached(this.asNode(), _) or
-                expectsContentCached(this.asNode(), _) or
-                neverSkipInPathGraph(this.asNode()) or
+                flowCheckNode(this) or
                 Config::neverSkip(this.asNode())
               )
             }
@@ -2665,7 +2651,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
               or
               node instanceof ParamNodeEx
               or
-              node.asNode() instanceof OutNodeExt
+              node instanceof OutNodeEx
               or
               storeStepCand(_, _, _, node, _, _)
               or
@@ -2899,15 +2885,9 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
             predicate isHidden() {
               not Config::includeHiddenNodes() and
-              (
-                hiddenNode(this.getNodeEx().asNode()) and
-                not this.isSource() and
-                not this instanceof PathNodeSink
-                or
-                this.getNodeEx() instanceof TNodeImplicitRead
-                or
-                hiddenNode(this.getNodeEx().asParamReturnNode())
-              )
+              hiddenNode(this.getNodeEx()) and
+              not this.isSource() and
+              not this instanceof PathNodeSink
             }
 
             /** Gets a textual representation of this element. */
@@ -3770,11 +3750,6 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
     private module Stage2 = MkStage<Stage1>::Stage<Stage2Param>;
 
-    pragma[nomagic]
-    private predicate castingNodeEx(NodeEx node) {
-      node.asNode() instanceof CastingNode or exists(node.asParamReturnNode())
-    }
-
     private module Stage3Param implements MkStage<Stage2>::StageParam {
       private module PrevStage = Stage2;
 
@@ -3888,7 +3863,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
     bindingset[node, t0]
     private predicate strengthenType(NodeEx node, DataFlowType t0, DataFlowType t) {
-      if castingNodeEx(node)
+      if node instanceof CastingNodeEx
       then
         exists(DataFlowType nt | nt = node.getDataFlowType() |
           if typeStrongerThanFilter(nt, t0)
@@ -3945,7 +3920,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       pragma[nomagic]
       private predicate clearSet(NodeEx node, ContentSet c) {
         PrevStage::revFlow(node) and
-        clearsContentCached(node.asNode(), c)
+        clearsContentSet(node, c)
       }
 
       pragma[nomagic]
@@ -5024,7 +4999,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       bindingset[c]
       private predicate clearsContentEx(NodeEx n, Content c) {
         exists(ContentSet cs |
-          clearsContentCached(n.asNode(), cs) and
+          clearsContentSet(n, cs) and
           pragma[only_bind_out](c) = pragma[only_bind_into](cs).getAReadContent()
         )
       }
@@ -5442,9 +5417,8 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         PartialAccessPath ap
       ) {
         exists(ReturnKindExt kind, DataFlowCall call |
-          partialPathOutOfCallable1(mid, call, kind, state, cc, t, ap)
-        |
-          out.asNode() = kind.getAnOutNode(call)
+          partialPathOutOfCallable1(mid, call, kind, state, cc, t, ap) and
+          out = kind.getAnOutNodeEx(call)
         )
       }
 
@@ -5529,7 +5503,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       ) {
         exists(DataFlowCall call, ReturnKindExt kind |
           partialPathThroughCallable0(call, mid, kind, state, cc, t, ap) and
-          out.asNode() = kind.getAnOutNode(call)
+          out = kind.getAnOutNodeEx(call)
         )
       }
 
@@ -5745,7 +5719,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       ) {
         exists(DataFlowCall call, ArgumentPosition pos |
           revPartialPathThroughCallable0(call, mid, pos, state, ap) and
-          node.asNode().(ArgNode).argumentOf(call, pos)
+          node.argumentOf(call, pos)
         )
       }
 
diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll
index e477913a59bd..75d68cf247c1 100644
--- a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll
+++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll
@@ -286,7 +286,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     ) {
       revLambdaFlow0(lambdaCall, kind, node, t, toReturn, toJump, lastCall) and
       not expectsContent(node, _) and
-      if castNode(node) or node instanceof ArgNode or node instanceof ReturnNode
+      if node instanceof CastNode or node instanceof ArgNode or node instanceof ReturnNode
       then compatibleTypesFilter(t, getNodeDataFlowType(node))
       else any()
     }
@@ -372,7 +372,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     ) {
       revLambdaFlow(lambdaCall, kind, out, t, _, toJump, lastCall) and
       exists(ReturnKindExt rk |
-        out = rk.getAnOutNode(call) and
+        out = getAnOutNodeExt(call, rk) and
         lambdaCall(call, _, _)
       )
     }
@@ -901,20 +901,36 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     Location getLocation() { result = this.projectToNode().getLocation() }
   }
 
+  /**
+   * A `Node` at which a cast can occur such that the type should be checked.
+   */
+  final class CastingNodeEx extends NodeEx {
+    CastingNodeEx() { castingNodeEx(this) }
+  }
+
   final class ArgNodeEx extends NodeEx {
-    ArgNodeEx() { this.asNode() instanceof ArgNode }
+    private DataFlowCall call_;
+    private ArgumentPosition pos_;
+
+    ArgNodeEx() { this.asNode().(ArgNode).argumentOf(call_, pos_) }
+
+    predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
+      call = call_ and
+      pos = pos_
+    }
 
-    DataFlowCall getCall() { this.asNode().(ArgNode).argumentOf(result, _) }
+    DataFlowCall getCall() { result = call_ }
   }
 
   final class ParamNodeEx extends NodeEx {
-    ParamNodeEx() { this.asNode() instanceof ParamNode }
+    private DataFlowCallable c_;
+    private ParameterPosition pos_;
 
-    predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
-      this.asNode().(ParamNode).isParameterOf(c, pos)
-    }
+    ParamNodeEx() { this.asNode().(ParamNode).isParameterOf(c_, pos_) }
+
+    predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { c = c_ and pos = pos_ }
 
-    ParameterPosition getPosition() { this.isParameterOf(_, result) }
+    ParameterPosition getPosition() { result = pos_ }
   }
 
   /**
@@ -931,6 +947,18 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     ReturnKindExt getKind() { result = pos.getKind() }
   }
 
+  final class OutNodeEx extends NodeEx {
+    OutNodeEx() { this.asNode() instanceof OutNodeExt }
+  }
+
+  pragma[nomagic]
+  private SndLevelScopeOption getSecondLevelScope0(Node n) {
+    result = SndLevelScopeOption::some(getSecondLevelScope(n))
+    or
+    result instanceof SndLevelScopeOption::None and
+    not exists(getSecondLevelScope(n))
+  }
+
   cached
   private module Cached {
     /**
@@ -942,11 +970,8 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     predicate forceCachingInSameStage() { any() }
 
     cached
-    SndLevelScopeOption getSecondLevelScopeCached(Node n) {
-      result = SndLevelScopeOption::some(getSecondLevelScope(n))
-      or
-      result instanceof SndLevelScopeOption::None and
-      not exists(getSecondLevelScope(n))
+    SndLevelScopeOption getSecondLevelScopeEx(NodeEx n) {
+      result = getSecondLevelScope0(n.asNode())
     }
 
     cached
@@ -978,11 +1003,14 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     predicate jumpStepCached(Node node1, Node node2) { jumpStep(node1, node2) }
 
     cached
-    predicate clearsContentCached(Node n, ContentSet c) { clearsContent(n, c) }
+    predicate clearsContentSet(NodeEx n, ContentSet c) { clearsContent(n.asNode(), c) }
 
     cached
     predicate expectsContentCached(Node n, ContentSet c) { expectsContent(n, c) }
 
+    cached
+    predicate expectsContentSet(NodeEx n, ContentSet c) { expectsContent(n.asNode(), c) }
+
     cached
     predicate isUnreachableInCallCached(NodeRegion nr, DataFlowCall call) {
       isUnreachableInCall(nr, call)
@@ -996,16 +1024,15 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     }
 
     cached
-    predicate hiddenNode(Node n) { nodeIsHidden(n) }
+    predicate hiddenNode(NodeEx n) {
+      nodeIsHidden([n.asNode(), n.asParamReturnNode()])
+      or
+      n instanceof TNodeImplicitRead
+    }
 
     cached
-    OutNodeExt getAnOutNodeExt(DataFlowCall call, ReturnKindExt k) {
-      result = getAnOutNode(call, k.(ValueReturnKind).getKind())
-      or
-      exists(ArgNode arg |
-        result.(PostUpdateNode).getPreUpdateNode() = arg and
-        arg.argumentOf(call, k.(ParamUpdateReturnKind).getAMatchingArgumentPosition())
-      )
+    OutNodeEx getAnOutNodeEx(DataFlowCall call, ReturnKindExt k) {
+      result.asNode() = getAnOutNodeExt(call, k)
     }
 
     pragma[nomagic]
@@ -1016,22 +1043,22 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
         parameterValueFlowsToPreUpdate(p, n) and
         p.isParameterOf(_, pos) and
         k = TParamUpdate(pos) and
-        scope = getSecondLevelScopeCached(n)
+        scope = getSecondLevelScope0(n)
       )
     }
 
     cached
-    predicate castNode(Node n) { n instanceof CastNode }
+    predicate flowCheckNode(NodeEx n) {
+      n.asNode() instanceof CastNode or
+      clearsContentSet(n, _) or
+      expectsContentSet(n, _) or
+      neverSkipInPathGraph(n.asNode())
+    }
 
     cached
-    predicate castingNode(Node n) {
-      castNode(n) or
-      n instanceof ParamNode or
-      n instanceof OutNodeExt or
-      // For reads, `x.f`, we want to check that the tracked type after the read (which
-      // is obtained by popping the head of the access path stack) is compatible with
-      // the type of `x.f`.
-      readSet(_, _, n)
+    predicate castingNodeEx(NodeEx n) {
+      n.asNode() instanceof CastingNode or
+      exists(n.asParamReturnNode())
     }
 
     cached
@@ -1208,6 +1235,15 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
       )
     }
 
+    /**
+     * Holds if `arg` is a possible argument to `p` in `call`, taking virtual
+     * dispatch into account.
+     */
+    cached
+    predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx arg) {
+      viableParamArg(call, p.asNode(), arg.asNode())
+    }
+
     pragma[nomagic]
     private ReturnPosition viableReturnPos(DataFlowCall call, ReturnKindExt kind) {
       viableCallableExt(call) = result.getCallable() and
@@ -1219,13 +1255,22 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
      * taking virtual dispatch into account.
      */
     cached
-    predicate viableReturnPosOut(DataFlowCall call, ReturnPosition pos, Node out) {
+    predicate viableReturnPosOut(DataFlowCall call, ReturnPosition pos, OutNodeExt out) {
       exists(ReturnKindExt kind |
         pos = viableReturnPos(call, kind) and
-        out = kind.getAnOutNode(call)
+        out = getAnOutNodeExt(call, kind)
       )
     }
 
+    /**
+     * Holds if a value at return position `pos` can be returned to `out` via `call`,
+     * taking virtual dispatch into account.
+     */
+    cached
+    predicate viableReturnPosOutEx(DataFlowCall call, ReturnPosition pos, OutNodeEx out) {
+      viableReturnPosOut(call, pos, out.asNode())
+    }
+
     /** Provides predicates for calculating flow-through summaries. */
     private module FlowThrough {
       /**
@@ -1559,6 +1604,11 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     cached
     predicate readSet(Node node1, ContentSet c, Node node2) { readStep(node1, c, node2) }
 
+    cached
+    predicate readEx(NodeEx node1, ContentSet c, NodeEx node2) {
+      readSet(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode()))
+    }
+
     cached
     predicate storeSet(
       Node node1, ContentSet c, Node node2, DataFlowType contentType, DataFlowType containerType
@@ -1587,11 +1637,13 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
      * been stored into, in order to handle cases like `x.f1.f2 = y`.
      */
     cached
-    predicate store(
-      Node node1, Content c, Node node2, DataFlowType contentType, DataFlowType containerType
+    predicate storeEx(
+      NodeEx node1, Content c, NodeEx node2, DataFlowType contentType, DataFlowType containerType
     ) {
       exists(ContentSet cs |
-        c = cs.getAStoreContent() and storeSet(node1, cs, node2, contentType, containerType)
+        c = cs.getAStoreContent() and
+        storeSet(pragma[only_bind_into](node1.asNode()), cs, pragma[only_bind_into](node2.asNode()),
+          contentType, containerType)
       )
     }
 
@@ -1627,7 +1679,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     }
 
     cached
-    predicate allowParameterReturnInSelfCached(ParamNode p) { allowParameterReturnInSelf(p) }
+    predicate allowParameterReturnInSelfEx(ParamNodeEx p) { allowParameterReturnInSelf(p.asNode()) }
 
     cached
     predicate paramMustFlow(ParamNode p, ArgNode arg) { localMustFlowStep+(p, arg) }
@@ -1657,7 +1709,9 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
       string toString() { result = "Unreachable" }
 
       cached
-      predicate contains(Node n) { exists(NodeRegion nr | super.contains(nr) and nr.contains(n)) }
+      predicate contains(NodeEx n) {
+        exists(NodeRegion nr | super.contains(nr) and nr.contains(n.asNode()))
+      }
 
       cached
       DataFlowCallable getEnclosingCallable() {
@@ -2209,7 +2263,15 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
    * A `Node` at which a cast can occur such that the type should be checked.
    */
   class CastingNode extends NodeFinal {
-    CastingNode() { castingNode(this) }
+    CastingNode() {
+      this instanceof CastNode or
+      this instanceof ParamNode or
+      this instanceof OutNodeExt or
+      // For reads, `x.f`, we want to check that the tracked type after the read (which
+      // is obtained by popping the head of the access path stack) is compatible with
+      // the type of `x.f`.
+      readSet(_, _, this)
+    }
   }
 
   private predicate readStepWithTypes(
@@ -2266,7 +2328,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     }
 
     /** Holds if this call context makes `n` unreachable. */
-    predicate unreachable(Node n) { ns.contains(n) }
+    predicate unreachable(NodeEx n) { ns.contains(n) }
   }
 
   private DataFlowCallable getNodeRegionEnclosingCallable(NodeRegion nr) {
@@ -2307,6 +2369,16 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     OutNodeExt() { outNodeExt(this) }
   }
 
+  pragma[nomagic]
+  OutNodeExt getAnOutNodeExt(DataFlowCall call, ReturnKindExt k) {
+    result = getAnOutNode(call, k.(ValueReturnKind).getKind())
+    or
+    exists(ArgNode arg |
+      result.(PostUpdateNode).getPreUpdateNode() = arg and
+      arg.argumentOf(call, k.(ParamUpdateReturnKind).getAMatchingArgumentPosition())
+    )
+  }
+
   /**
    * An extended return kind. A return kind describes how data can be returned
    * from a callable. This can either be through a returned value or an updated
@@ -2317,7 +2389,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     abstract string toString();
 
     /** Gets a node corresponding to data flow out of `call`. */
-    final OutNodeExt getAnOutNode(DataFlowCall call) { result = getAnOutNodeExt(call, this) }
+    final OutNodeEx getAnOutNodeEx(DataFlowCall call) { result = getAnOutNodeEx(call, this) }
   }
 
   class ValueReturnKind extends ReturnKindExt, TValueReturn {
diff --git a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll
index 4f6a88b6b28a..e7e588ff9ebe 100644
--- a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll
+++ b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll
@@ -1355,7 +1355,7 @@ module Make<
         exists(DataFlowCall call, ReturnKindExt rk |
           result = summaryArgParam(call, arg, sc) and
           summaryReturnNodeExt(ret, pragma[only_bind_into](rk)) and
-          out = pragma[only_bind_into](rk).getAnOutNode(call)
+          out = getAnOutNodeExt(call, pragma[only_bind_into](rk))
         )
       }
 
diff --git a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
index 2e22ae2c2ba8..5b53943ff832 100644
--- a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
+++ b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
@@ -464,8 +464,10 @@ module MakeModelGenerator<
     predicate isAdditionalFlowStep(
       DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
     ) {
-      exists(DataFlow::ContentSet c |
-        DataFlow::store(node1, c.getAStoreContent(), node2, _, _) and
+      exists(DataFlow::NodeEx n1, DataFlow::NodeEx n2, DataFlow::ContentSet c |
+        node1 = n1.asNode() and
+        node2 = n2.asNode() and
+        DataFlow::storeEx(n1, c.getAStoreContent(), n2, _, _) and
         isRelevantContent0(c) and
         (
           state1 instanceof TaintRead and state2.(TaintStore).getStep() = 1

From e96f15d9b4640db0f1bd12a4290af805b9610bb8 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 22 Nov 2024 13:12:46 +0000
Subject: [PATCH 11/51] Rust: Add a test exposing SQL Injection sinks directly.

---
 .../security/CWE-089/SqlSinks.expected        |   2 +
 .../query-tests/security/CWE-089/SqlSinks.ql  |  19 +++
 .../test/query-tests/security/CWE-089/sqlx.rs | 112 +++++++++---------
 3 files changed, 77 insertions(+), 56 deletions(-)
 create mode 100644 rust/ql/test/query-tests/security/CWE-089/SqlSinks.expected
 create mode 100644 rust/ql/test/query-tests/security/CWE-089/SqlSinks.ql

diff --git a/rust/ql/test/query-tests/security/CWE-089/SqlSinks.expected b/rust/ql/test/query-tests/security/CWE-089/SqlSinks.expected
new file mode 100644
index 000000000000..8ec8033d086e
--- /dev/null
+++ b/rust/ql/test/query-tests/security/CWE-089/SqlSinks.expected
@@ -0,0 +1,2 @@
+testFailures
+failures
diff --git a/rust/ql/test/query-tests/security/CWE-089/SqlSinks.ql b/rust/ql/test/query-tests/security/CWE-089/SqlSinks.ql
new file mode 100644
index 000000000000..d1cae882e681
--- /dev/null
+++ b/rust/ql/test/query-tests/security/CWE-089/SqlSinks.ql
@@ -0,0 +1,19 @@
+import rust
+import codeql.rust.security.SqlInjectionExtensions
+import utils.InlineExpectationsTest
+
+module SqlSinksTest implements TestSig {
+  string getARelevantTag() { result = ["sql-sink"] }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(SqlInjection::Sink sink |
+      location = sink.getLocation() and
+      location.getFile().getBaseName() != "" and
+      element = sink.toString() and
+      tag = "sql-sink" and
+      value = ""
+    )
+  }
+}
+
+import MakeTest<SqlSinksTest>
diff --git a/rust/ql/test/query-tests/security/CWE-089/sqlx.rs b/rust/ql/test/query-tests/security/CWE-089/sqlx.rs
index b5cc25000f99..d4039200b4ff 100644
--- a/rust/ql/test/query-tests/security/CWE-089/sqlx.rs
+++ b/rust/ql/test/query-tests/security/CWE-089/sqlx.rs
@@ -44,8 +44,8 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err
 
     // construct queries (with extra variants)
     let const_string = String::from("Alice");
-    let arg_string = std::env::args().nth(1).unwrap_or(String::from("Alice")); // $ MISSING Source=args1
-    let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING Source=remote1
+    let arg_string = std::env::args().nth(1).unwrap_or(String::from("Alice")); // $ MISSING: Source=args1
+    let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING: Source=remote1
     let remote_number = remote_string.parse::<i32>().unwrap_or(0);
     let safe_query_1 = String::from("SELECT * FROM people WHERE firstname='Alice'");
     let safe_query_2 = String::from("SELECT * FROM people WHERE firstname='") + &const_string + "'";
@@ -57,31 +57,31 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err
     let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=?"); // (prepared arguments are safe)
 
     // direct execution
-    let _ = conn.execute(safe_query_1.as_str()).await?;
-    let _ = conn.execute(safe_query_2.as_str()).await?;
-    let _ = conn.execute(safe_query_3.as_str()).await?;
-    let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING Alert[sql-injection]=args1
+    let _ = conn.execute(safe_query_1.as_str()).await?; // $ MISSING: sql-sink
+    let _ = conn.execute(safe_query_2.as_str()).await?; // $ MISSING: sql-sink
+    let _ = conn.execute(safe_query_3.as_str()).await?; // $ MISSING: sql-sink
+    let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING: sql-sink Alert[sql-injection]=args1
     if enable_remote {
-        let _ = conn.execute(unsafe_query_2.as_str()).await?; // $ MISSING Alert[sql-injection]=remote1
-        let _ = conn.execute(unsafe_query_3.as_str()).await?; // $ MISSING Alert[sql-injection]=remote1
-        let _ = conn.execute(unsafe_query_4.as_str()).await?; // $ MISSING Alert[sql-injection]=remote1
+        let _ = conn.execute(unsafe_query_2.as_str()).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote1
+        let _ = conn.execute(unsafe_query_3.as_str()).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote1
+        let _ = conn.execute(unsafe_query_4.as_str()).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote1
     }
 
     // prepared queries
-    let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?;
-    let _ = sqlx::query(safe_query_2.as_str()).execute(&pool).await?;
-    let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?;
-    let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=args1
+    let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ MISSING: sql-sink
+    let _ = sqlx::query(safe_query_2.as_str()).execute(&pool).await?; // $ MISSING: sql-sink
+    let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?; // $ MISSING: sql-sink
+    let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ MISSING: sql-sink Alert[sql-injection]=args1
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_2.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=remote1
-        let _ = sqlx::query(unsafe_query_3.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=remote1
-        let _ = sqlx::query(unsafe_query_4.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=remote1
+        let _ = sqlx::query(unsafe_query_2.as_str()).execute(&pool).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote1
+        let _ = sqlx::query(unsafe_query_3.as_str()).execute(&pool).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote1
+        let _ = sqlx::query(unsafe_query_4.as_str()).execute(&pool).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote1
     }
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(const_string).execute(&pool).await?;
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(arg_string).execute(&pool).await?;
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(const_string).execute(&pool).await?; // $ MISSING: sql-sink
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(arg_string).execute(&pool).await?; // $ MISSING: sql-sink
     if enable_remote {
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_string).execute(&pool).await?;
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_number).execute(&pool).await?;
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_string).execute(&pool).await?; // $ MISSING: sql-sink
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_number).execute(&pool).await?; // $ MISSING: sql-sink
     }
 
     Ok(())
@@ -93,67 +93,67 @@ async fn test_sqlx_sqlite(url: &str, enable_remote: bool) -> Result<(), sqlx::Er
 
     // construct queries
     let const_string = String::from("Alice");
-    let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING Source=remote2
+    let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING: Source=remote2
     let safe_query_1 = String::from("SELECT * FROM people WHERE firstname='") + &const_string + "'";
     let unsafe_query_1 = String::from("SELECT * FROM people WHERE firstname='") + &remote_string + "'";
     let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=?"); // (prepared arguments are safe)
 
     // direct execution (with extra variants)
-    let _ = conn.execute(safe_query_1.as_str()).await?;
+    let _ = conn.execute(safe_query_1.as_str()).await?; // $ MISSING: sql-sink
     if enable_remote {
-        let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING Alert[sql-injection]=remote2
+        let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote2
     }
     // ...
-    let _ = sqlx::raw_sql(safe_query_1.as_str()).execute(&mut conn).await?;
+    let _ = sqlx::raw_sql(safe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING: ql-sink
     if enable_remote {
-        let _ = sqlx::raw_sql(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING Alert[sql-injection]=remote2
+        let _ = sqlx::raw_sql(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote2
     }
 
     // prepared queries (with extra variants)
-    let _ = sqlx::query(safe_query_1.as_str()).execute(&mut conn).await?;
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&mut conn).await?;
+    let _ = sqlx::query(safe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING: sql-sink
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&mut conn).await?; // $ MISSING: sql-sink
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING Alert[sql-injection]=remote2
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&mut conn).await?;
+        let _ = sqlx::query(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote2
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&mut conn).await?; // $ MISSING: sql-sink
     }
     // ...
-    let _ = sqlx::query(safe_query_1.as_str()).fetch(&mut conn);
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch(&mut conn);
+    let _ = sqlx::query(safe_query_1.as_str()).fetch(&mut conn); // $ MISSING: sql-sink
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch(&mut conn); // $ MISSING: sql-sink
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_1.as_str()).fetch(&mut conn); // $ MISSING Alert[sql-injection]=remote2
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch(&mut conn);
+        let _ = sqlx::query(unsafe_query_1.as_str()).fetch(&mut conn); // $ MISSING: ql-sink Alert[sql-injection]=remote2
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch(&mut conn); // $ MISSING: sql-sink
     }
     // ...
-    let row1: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_one(&mut conn).await?;
+    let row1: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_one(&mut conn).await?; // $ MISSING: sql-sink
     println!("  row1 = {:?}", row1);
-    let row2: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_one(&mut conn).await?;
+    let row2: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_one(&mut conn).await?; // $ MISSING: sql-sink
     println!("  row2 = {:?}", row2);
     if enable_remote {
-        let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_one(&mut conn).await?; // $ MISSING Alert[sql-injection]=remote2
-        let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_one(&mut conn).await?;
+        let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_one(&mut conn).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote2
+        let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_one(&mut conn).await?; // $ MISSING: sql-sink
     }
     // ...
-    let row3: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data");
+    let row3: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ MISSING: sql-sink
     println!("  row3 = {:?}", row3);
-    let row4: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_optional(&mut conn).await?.expect("no data");
+    let row4: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_optional(&mut conn).await?.expect("no data"); // $ MISSING: sql-sink
     println!("  row4 = {:?}", row4);
     if enable_remote {
-        let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ MISSING Alert[sql-injection]=remote2
-        let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_optional(&mut conn).await?.expect("no data");
+        let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ MISSING: sql-sink Alert[sql-injection]=remote2
+        let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_optional(&mut conn).await?.expect("no data"); // $ MISSING: sql-sink
     }
     // ...
-    let _ = sqlx::query(safe_query_1.as_str()).fetch_all(&mut conn).await?;
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch_all(&mut conn).await?;
-    let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&const_string).fetch_all(&mut conn).await?;
+    let _ = sqlx::query(safe_query_1.as_str()).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
+    let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&const_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_1.as_str()).fetch_all(&mut conn).await?; // $ MISSING Alert[sql-injection]=remote2
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch_all(&mut conn).await?;
-        let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&remote_string).fetch_all(&mut conn).await?;
+        let _ = sqlx::query(unsafe_query_1.as_str()).fetch_all(&mut conn).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote2
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
+        let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&remote_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
     }
     // ...
-    let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", const_string).fetch_all(&mut conn).await?; // (only takes string literals, so can't be vulnerable)
+    let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", const_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink (only takes string literals, so can't be vulnerable)
     if enable_remote {
-        let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", remote_string).fetch_all(&mut conn).await?;
+        let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", remote_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
     }
 
     Ok(())
@@ -166,23 +166,23 @@ async fn test_sqlx_postgres(url: &str, enable_remote: bool) -> Result<(), sqlx::
 
     // construct queries
     let const_string = String::from("Alice");
-    let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING Source=remote3
+    let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING: Source=remote3
     let safe_query_1 = String::from("SELECT * FROM people WHERE firstname='") + &const_string + "'";
     let unsafe_query_1 = String::from("SELECT * FROM people WHERE firstname='") + &remote_string + "'";
     let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=$1"); // (prepared arguments are safe)
 
     // direct execution
-    let _ = conn.execute(safe_query_1.as_str()).await?;
+    let _ = conn.execute(safe_query_1.as_str()).await?; // $ MISSING: sql-sink
     if enable_remote {
-        let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING Alert[sql-injection]=remote3
+        let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote3
     }
 
     // prepared queries
-    let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?;
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&pool).await?;
+    let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ MISSING: sql-sink
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&pool).await?; // $ MISSING: sql-sink
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=remote3
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&pool).await?;
+        let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote3
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&pool).await?; // $ MISSING: sql-sink
     }
 
     Ok(())

From ba560f2fe97bb811bd25dd16848c4ebf666a60fc Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 22 Nov 2024 15:04:57 +0000
Subject: [PATCH 12/51] Rust: Model SQLx.

---
 rust/ql/lib/codeql/rust/Frameworks.qll        |   1 +
 rust/ql/lib/codeql/rust/frameworks/Sqlx.qll   |  40 +++++++
 .../test/query-tests/security/CWE-089/sqlx.rs | 100 +++++++++---------
 3 files changed, 91 insertions(+), 50 deletions(-)
 create mode 100644 rust/ql/lib/codeql/rust/frameworks/Sqlx.qll

diff --git a/rust/ql/lib/codeql/rust/Frameworks.qll b/rust/ql/lib/codeql/rust/Frameworks.qll
index 601e87ef6ebc..0c6fc573d0fb 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.Reqwest
 private import codeql.rust.frameworks.stdlib.Env
+private import codeql.rust.frameworks.Sqlx
diff --git a/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll b/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll
new file mode 100644
index 000000000000..65361eae8147
--- /dev/null
+++ b/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll
@@ -0,0 +1,40 @@
+/**
+ * Provides modeling for the `SQLx` library.
+ */
+
+private import rust
+private import codeql.rust.Concepts
+private import codeql.rust.dataflow.DataFlow
+
+/**
+ * A call to `sqlx::query` and variations.
+ */
+private class SqlxQuery extends SqlConstruction::Range {
+  CallExpr call;
+
+  SqlxQuery() {
+    this.asExpr().getExpr() = call and
+    call.getExpr().(PathExpr).getPath().getResolvedPath() =
+      [
+        "crate::query::query", "crate::query_as::query_as", "crate::query_with::query_with",
+        "crate::query_as_with::query_as_with", "crate::query_scalar::query_scalar",
+        "crate::query_scalar_with::query_scalar_with", "crate::raw_sql::raw_sql"
+      ]
+  }
+
+  override DataFlow::Node getSql() { result.asExpr().getExpr() = call.getArgList().getArg(0) }
+}
+
+/**
+ * A call to `sqlx::Executor::execute`.
+ */
+private class SqlxExecute extends SqlConstruction::Range {
+  MethodCallExpr call;
+
+  SqlxExecute() {
+    this.asExpr().getExpr() = call and
+    call.(Resolvable).getResolvedPath() = "crate::executor::Executor::execute"
+  }
+
+  override DataFlow::Node getSql() { result.asExpr().getExpr() = call.getArgList().getArg(0) }
+}
diff --git a/rust/ql/test/query-tests/security/CWE-089/sqlx.rs b/rust/ql/test/query-tests/security/CWE-089/sqlx.rs
index d4039200b4ff..9f6e9c08f0c3 100644
--- a/rust/ql/test/query-tests/security/CWE-089/sqlx.rs
+++ b/rust/ql/test/query-tests/security/CWE-089/sqlx.rs
@@ -57,31 +57,31 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err
     let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=?"); // (prepared arguments are safe)
 
     // direct execution
-    let _ = conn.execute(safe_query_1.as_str()).await?; // $ MISSING: sql-sink
-    let _ = conn.execute(safe_query_2.as_str()).await?; // $ MISSING: sql-sink
-    let _ = conn.execute(safe_query_3.as_str()).await?; // $ MISSING: sql-sink
-    let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING: sql-sink Alert[sql-injection]=args1
+    let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
+    let _ = conn.execute(safe_query_2.as_str()).await?; // $ sql-sink
+    let _ = conn.execute(safe_query_3.as_str()).await?; // $ sql-sink
+    let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink AMISSING: lert[sql-injection]=args1
     if enable_remote {
-        let _ = conn.execute(unsafe_query_2.as_str()).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote1
-        let _ = conn.execute(unsafe_query_3.as_str()).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote1
-        let _ = conn.execute(unsafe_query_4.as_str()).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote1
+        let _ = conn.execute(unsafe_query_2.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
+        let _ = conn.execute(unsafe_query_3.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
+        let _ = conn.execute(unsafe_query_4.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
     }
 
     // prepared queries
-    let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ MISSING: sql-sink
-    let _ = sqlx::query(safe_query_2.as_str()).execute(&pool).await?; // $ MISSING: sql-sink
-    let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?; // $ MISSING: sql-sink
-    let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ MISSING: sql-sink Alert[sql-injection]=args1
+    let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ sql-sink
+    let _ = sqlx::query(safe_query_2.as_str()).execute(&pool).await?; // $ sql-sink
+    let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?; // $ sql-sink
+    let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=args1
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_2.as_str()).execute(&pool).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote1
-        let _ = sqlx::query(unsafe_query_3.as_str()).execute(&pool).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote1
-        let _ = sqlx::query(unsafe_query_4.as_str()).execute(&pool).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote1
+        let _ = sqlx::query(unsafe_query_2.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
+        let _ = sqlx::query(unsafe_query_3.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
+        let _ = sqlx::query(unsafe_query_4.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
     }
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(const_string).execute(&pool).await?; // $ MISSING: sql-sink
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(arg_string).execute(&pool).await?; // $ MISSING: sql-sink
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(const_string).execute(&pool).await?; // $ sql-sink
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(arg_string).execute(&pool).await?; // $ sql-sink
     if enable_remote {
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_string).execute(&pool).await?; // $ MISSING: sql-sink
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_number).execute(&pool).await?; // $ MISSING: sql-sink
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_string).execute(&pool).await?; // $ sql-sink
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_number).execute(&pool).await?; // $ sql-sink
     }
 
     Ok(())
@@ -99,56 +99,56 @@ async fn test_sqlx_sqlite(url: &str, enable_remote: bool) -> Result<(), sqlx::Er
     let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=?"); // (prepared arguments are safe)
 
     // direct execution (with extra variants)
-    let _ = conn.execute(safe_query_1.as_str()).await?; // $ MISSING: sql-sink
+    let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
     if enable_remote {
-        let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote2
+        let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
     }
     // ...
-    let _ = sqlx::raw_sql(safe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING: ql-sink
+    let _ = sqlx::raw_sql(safe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink
     if enable_remote {
-        let _ = sqlx::raw_sql(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote2
+        let _ = sqlx::raw_sql(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
     }
 
     // prepared queries (with extra variants)
-    let _ = sqlx::query(safe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING: sql-sink
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&mut conn).await?; // $ MISSING: sql-sink
+    let _ = sqlx::query(safe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&mut conn).await?; // $ sql-sink
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote2
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&mut conn).await?; // $ MISSING: sql-sink
+        let _ = sqlx::query(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&mut conn).await?; // $ sql-sink
     }
     // ...
-    let _ = sqlx::query(safe_query_1.as_str()).fetch(&mut conn); // $ MISSING: sql-sink
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch(&mut conn); // $ MISSING: sql-sink
+    let _ = sqlx::query(safe_query_1.as_str()).fetch(&mut conn); // $ sql-sink
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch(&mut conn); // $ sql-sink
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_1.as_str()).fetch(&mut conn); // $ MISSING: ql-sink Alert[sql-injection]=remote2
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch(&mut conn); // $ MISSING: sql-sink
+        let _ = sqlx::query(unsafe_query_1.as_str()).fetch(&mut conn); // $ sql-sink MISSING: Alert[sql-injection]=remote2
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch(&mut conn); // $ sql-sink
     }
     // ...
-    let row1: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_one(&mut conn).await?; // $ MISSING: sql-sink
+    let row1: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_one(&mut conn).await?; // $ sql-sink
     println!("  row1 = {:?}", row1);
-    let row2: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_one(&mut conn).await?; // $ MISSING: sql-sink
+    let row2: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_one(&mut conn).await?; // $ sql-sink
     println!("  row2 = {:?}", row2);
     if enable_remote {
-        let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_one(&mut conn).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote2
-        let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_one(&mut conn).await?; // $ MISSING: sql-sink
+        let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_one(&mut conn).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
+        let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_one(&mut conn).await?; // $ sql-sink
     }
     // ...
-    let row3: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ MISSING: sql-sink
+    let row3: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ sql-sink
     println!("  row3 = {:?}", row3);
-    let row4: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_optional(&mut conn).await?.expect("no data"); // $ MISSING: sql-sink
+    let row4: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_optional(&mut conn).await?.expect("no data"); // $ sql-sink
     println!("  row4 = {:?}", row4);
     if enable_remote {
-        let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ MISSING: sql-sink Alert[sql-injection]=remote2
-        let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_optional(&mut conn).await?.expect("no data"); // $ MISSING: sql-sink
+        let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ sql-sink $ MISSING: Alert[sql-injection]=remote2
+        let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_optional(&mut conn).await?.expect("no data"); // $ sql-sink
     }
     // ...
-    let _ = sqlx::query(safe_query_1.as_str()).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
-    let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&const_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
+    let _ = sqlx::query(safe_query_1.as_str()).fetch_all(&mut conn).await?; // $ sql-sink
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch_all(&mut conn).await?; // $ sql-sink
+    let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&const_string).fetch_all(&mut conn).await?; // $ sql-sink
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_1.as_str()).fetch_all(&mut conn).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote2
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
-        let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&remote_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
+        let _ = sqlx::query(unsafe_query_1.as_str()).fetch_all(&mut conn).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch_all(&mut conn).await?; // $ sql-sink
+        let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&remote_string).fetch_all(&mut conn).await?; // $ sql-sink
     }
     // ...
     let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", const_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink (only takes string literals, so can't be vulnerable)
@@ -172,17 +172,17 @@ async fn test_sqlx_postgres(url: &str, enable_remote: bool) -> Result<(), sqlx::
     let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=$1"); // (prepared arguments are safe)
 
     // direct execution
-    let _ = conn.execute(safe_query_1.as_str()).await?; // $ MISSING: sql-sink
+    let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
     if enable_remote {
-        let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote3
+        let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote3
     }
 
     // prepared queries
-    let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ MISSING: sql-sink
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&pool).await?; // $ MISSING: sql-sink
+    let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ sql-sink
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&pool).await?; // $ sql-sink
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ MISSING: sql-sink Alert[sql-injection]=remote3
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&pool).await?; // $ MISSING: sql-sink
+        let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote3
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&pool).await?; // $ sql-sink
     }
 
     Ok(())

From 60c212bb10afc1132e00f3f7e333543cd5c825a8 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 27 Nov 2024 10:42:24 +0000
Subject: [PATCH 13/51] Rust: Update for changes on main.

---
 rust/ql/lib/codeql/rust/frameworks/Sqlx.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll b/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll
index 65361eae8147..701cafd277a6 100644
--- a/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll
+++ b/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll
@@ -14,7 +14,7 @@ private class SqlxQuery extends SqlConstruction::Range {
 
   SqlxQuery() {
     this.asExpr().getExpr() = call and
-    call.getExpr().(PathExpr).getPath().getResolvedPath() =
+    call.getFunction().(PathExpr).getPath().getResolvedPath() =
       [
         "crate::query::query", "crate::query_as::query_as", "crate::query_with::query_with",
         "crate::query_as_with::query_as_with", "crate::query_scalar::query_scalar",

From c113a0b5a1f69a614d24792d4b7593874b583175 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Wed, 27 Nov 2024 10:51:42 +0000
Subject: [PATCH 14/51] Rust: Fix typo.

---
 rust/ql/test/query-tests/security/CWE-089/sqlx.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/ql/test/query-tests/security/CWE-089/sqlx.rs b/rust/ql/test/query-tests/security/CWE-089/sqlx.rs
index 9f6e9c08f0c3..173f6c730b29 100644
--- a/rust/ql/test/query-tests/security/CWE-089/sqlx.rs
+++ b/rust/ql/test/query-tests/security/CWE-089/sqlx.rs
@@ -60,7 +60,7 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err
     let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
     let _ = conn.execute(safe_query_2.as_str()).await?; // $ sql-sink
     let _ = conn.execute(safe_query_3.as_str()).await?; // $ sql-sink
-    let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink AMISSING: lert[sql-injection]=args1
+    let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=args1
     if enable_remote {
         let _ = conn.execute(unsafe_query_2.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
         let _ = conn.execute(unsafe_query_3.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1

From 2bc89900fb60af30bd046945ea88e8f748ae30ce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=93scar=20San=20Jos=C3=A9?= <oscarsj@github.com>
Date: Wed, 27 Nov 2024 16:16:45 +0100
Subject: [PATCH 15/51] Update codespaces default config to ubuntu 24

---
 .devcontainer/devcontainer.json | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
index 0b718747d9d1..a51284e3a1f6 100644
--- a/.devcontainer/devcontainer.json
+++ b/.devcontainer/devcontainer.json
@@ -1,4 +1,5 @@
 {
+  "image": "mcr.microsoft.com/devcontainers/base:ubuntu-24.04",
   "extensions": [
     "rust-lang.rust-analyzer",
     "bungcip.better-toml",

From 5790f5d5dce75df9b45bdb851d6e76d38c6fd152 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=93scar=20San=20Jos=C3=A9?= <oscarsj@github.com>
Date: Wed, 27 Nov 2024 18:37:12 +0100
Subject: [PATCH 16/51] Include paths on pull_request event trigger for
 compile-queries.yml workflow

---
 .github/workflows/compile-queries.yml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/.github/workflows/compile-queries.yml b/.github/workflows/compile-queries.yml
index dfd99f6b806a..aea37cdbe83a 100644
--- a/.github/workflows/compile-queries.yml
+++ b/.github/workflows/compile-queries.yml
@@ -7,6 +7,11 @@ on:
       - "rc/*"
       - "codeql-cli-*"
   pull_request:
+    paths:
+      - '*.ql'
+      - '*.qll'
+      - '**/qlpack.yml'
+      - '*.dbscheme'
 
 permissions:
   contents: read

From 1a0442c5a6ff6b18234986f67ae4f9d502290921 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=93scar=20San=20Jos=C3=A9?= <oscarsj@github.com>
Date: Wed, 27 Nov 2024 19:34:34 +0100
Subject: [PATCH 17/51] Adding correct wildcard

---
 .github/workflows/compile-queries.yml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/compile-queries.yml b/.github/workflows/compile-queries.yml
index aea37cdbe83a..945515c0c532 100644
--- a/.github/workflows/compile-queries.yml
+++ b/.github/workflows/compile-queries.yml
@@ -8,10 +8,10 @@ on:
       - "codeql-cli-*"
   pull_request:
     paths:
-      - '*.ql'
-      - '*.qll'
+      - '**.ql'
+      - '**.qll'
       - '**/qlpack.yml'
-      - '*.dbscheme'
+      - '**.dbscheme'
 
 permissions:
   contents: read

From b05d290bf0e521193f23987e8a762d274f48db94 Mon Sep 17 00:00:00 2001
From: Simon Friis Vindum <simonfv@gmail.com>
Date: Thu, 28 Nov 2024 11:31:39 +0100
Subject: [PATCH 18/51] Rust: Exclude data flow inconsistencies that stem from
 other inconsistencies

---
 .../DataFlowConsistency.ql                    | 10 +----
 rust/ql/lib/codeql/rust/AstConsistency.qll    |  8 ++++
 .../dataflow/internal/DataFlowConsistency.qll | 17 ++++++++
 .../diagnostics/DataFlowConsistencyCounts.ql  | 11 +----
 .../CONSISTENCY/AstConsistency.expected       | 42 +++++++++++++++++++
 .../CONSISTENCY/DataFlowConsistency.expected  | 11 -----
 .../diagnostics/AstConsistencyCounts.expected |  1 +
 7 files changed, 71 insertions(+), 29 deletions(-)
 create mode 100644 rust/ql/lib/codeql/rust/dataflow/internal/DataFlowConsistency.qll
 create mode 100644 rust/ql/test/extractor-tests/generated/MacroItems/CONSISTENCY/AstConsistency.expected
 delete mode 100644 rust/ql/test/extractor-tests/generated/MacroItems/CONSISTENCY/DataFlowConsistency.expected

diff --git a/rust/ql/consistency-queries/DataFlowConsistency.ql b/rust/ql/consistency-queries/DataFlowConsistency.ql
index 3513121f2608..ed50c3876aa9 100644
--- a/rust/ql/consistency-queries/DataFlowConsistency.ql
+++ b/rust/ql/consistency-queries/DataFlowConsistency.ql
@@ -5,12 +5,4 @@
  * @id rust/diagnostics/data-flow-consistency
  */
 
-import codeql.rust.dataflow.DataFlow::DataFlow as DataFlow
-private import rust
-private import codeql.rust.dataflow.internal.DataFlowImpl
-private import codeql.rust.dataflow.internal.TaintTrackingImpl
-private import codeql.dataflow.internal.DataFlowImplConsistency
-
-private module Input implements InputSig<Location, RustDataFlow> { }
-
-import MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>
+import codeql.rust.dataflow.internal.DataFlowConsistency
diff --git a/rust/ql/lib/codeql/rust/AstConsistency.qll b/rust/ql/lib/codeql/rust/AstConsistency.qll
index 3902f56d20ba..2358bddb3b9d 100644
--- a/rust/ql/lib/codeql/rust/AstConsistency.qll
+++ b/rust/ql/lib/codeql/rust/AstConsistency.qll
@@ -21,6 +21,11 @@ query predicate multipleToStrings(Element e, string cls, string s) {
  */
 query predicate multipleLocations(Locatable e) { strictcount(e.getLocation()) > 1 }
 
+/**
+ * Holds if `e` does not have a `Location`.
+ */
+query predicate noLocation(Locatable e) { not exists(e.getLocation()) }
+
 private predicate multiplePrimaryQlClasses(Element e) {
   strictcount(string cls | cls = e.getAPrimaryQlClass() and cls != "VariableAccess") > 1
 }
@@ -58,6 +63,9 @@ int getAstInconsistencyCounts(string type) {
   type = "Multiple locations" and
   result = count(Element e | multipleLocations(e) | e)
   or
+  type = "No location" and
+  result = count(Element e | noLocation(e) | e)
+  or
   type = "Multiple primary QL classes" and
   result = count(Element e | multiplePrimaryQlClasses(e) | e)
   or
diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowConsistency.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowConsistency.qll
new file mode 100644
index 000000000000..17b74ee28845
--- /dev/null
+++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowConsistency.qll
@@ -0,0 +1,17 @@
+import codeql.rust.dataflow.DataFlow::DataFlow as DataFlow
+private import rust
+private import codeql.rust.dataflow.internal.DataFlowImpl
+private import codeql.rust.dataflow.internal.TaintTrackingImpl
+private import codeql.dataflow.internal.DataFlowImplConsistency
+
+private module Input implements InputSig<Location, RustDataFlow> {
+  predicate uniqueNodeLocationExclude(RustDataFlow::Node n) {
+    // Exclude nodes where the missing location can be explained by the
+    // underlying AST node not having a location.
+    not exists(n.asExpr().getLocation())
+  }
+
+  predicate missingLocationExclude(RustDataFlow::Node n) { not exists(n.asExpr().getLocation()) }
+}
+
+import MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>
diff --git a/rust/ql/src/queries/diagnostics/DataFlowConsistencyCounts.ql b/rust/ql/src/queries/diagnostics/DataFlowConsistencyCounts.ql
index 79c582adb08d..ecc7d1eb55a6 100644
--- a/rust/ql/src/queries/diagnostics/DataFlowConsistencyCounts.ql
+++ b/rust/ql/src/queries/diagnostics/DataFlowConsistencyCounts.ql
@@ -5,17 +5,10 @@
  * @id rust/diagnostics/data-flow-consistency-counts
  */
 
-private import rust
-private import codeql.rust.dataflow.internal.DataFlowImpl
-private import codeql.rust.dataflow.internal.TaintTrackingImpl
-private import codeql.dataflow.internal.DataFlowImplConsistency
-
-private module Input implements InputSig<Location, RustDataFlow> { }
+import codeql.rust.dataflow.internal.DataFlowConsistency as Consistency
 
 // see also `rust/diagnostics/data-flow-consistency`, which lists the
 // individual inconsistency results.
 from string type, int num
-where
-  num =
-    MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>::getInconsistencyCounts(type)
+where num = Consistency::getInconsistencyCounts(type)
 select type, num
diff --git a/rust/ql/test/extractor-tests/generated/MacroItems/CONSISTENCY/AstConsistency.expected b/rust/ql/test/extractor-tests/generated/MacroItems/CONSISTENCY/AstConsistency.expected
new file mode 100644
index 000000000000..f0508f1853fe
--- /dev/null
+++ b/rust/ql/test/extractor-tests/generated/MacroItems/CONSISTENCY/AstConsistency.expected
@@ -0,0 +1,42 @@
+noLocation
+| file://:0:0:0:0 | ... .parent(...) |
+| file://:0:0:0:0 | ... .unwrap(...) |
+| file://:0:0:0:0 | ...: ... |
+| file://:0:0:0:0 | ...::Path |
+| file://:0:0:0:0 | ...::path |
+| file://:0:0:0:0 | ArgList |
+| file://:0:0:0:0 | ArgList |
+| file://:0:0:0:0 | MacroItems |
+| file://:0:0:0:0 | ParamList |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | RefType |
+| file://:0:0:0:0 | RefType |
+| file://:0:0:0:0 | RetType |
+| file://:0:0:0:0 | StmtList |
+| file://:0:0:0:0 | Use |
+| file://:0:0:0:0 | UseTree |
+| file://:0:0:0:0 | fn get_parent |
+| file://:0:0:0:0 | get_parent |
+| file://:0:0:0:0 | parent |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | std |
+| file://:0:0:0:0 | std |
+| file://:0:0:0:0 | std |
+| file://:0:0:0:0 | unwrap |
+| file://:0:0:0:0 | { ... } |
diff --git a/rust/ql/test/extractor-tests/generated/MacroItems/CONSISTENCY/DataFlowConsistency.expected b/rust/ql/test/extractor-tests/generated/MacroItems/CONSISTENCY/DataFlowConsistency.expected
deleted file mode 100644
index 28012e97348f..000000000000
--- a/rust/ql/test/extractor-tests/generated/MacroItems/CONSISTENCY/DataFlowConsistency.expected
+++ /dev/null
@@ -1,11 +0,0 @@
-uniqueNodeLocation
-| file://:0:0:0:0 | ... .parent(...) | Node should have one location but has 0. |
-| file://:0:0:0:0 | ... .parent(...) | Node should have one location but has 0. |
-| file://:0:0:0:0 | ... .unwrap(...) | Node should have one location but has 0. |
-| file://:0:0:0:0 | ...: ... | Node should have one location but has 0. |
-| file://:0:0:0:0 | path | Node should have one location but has 0. |
-| file://:0:0:0:0 | path | Node should have one location but has 0. |
-| file://:0:0:0:0 | path | Node should have one location but has 0. |
-| file://:0:0:0:0 | { ... } | Node should have one location but has 0. |
-missingLocation
-| Nodes without location: 8 |
diff --git a/rust/ql/test/query-tests/diagnostics/AstConsistencyCounts.expected b/rust/ql/test/query-tests/diagnostics/AstConsistencyCounts.expected
index 62ee879ab1eb..72147226e42b 100644
--- a/rust/ql/test/query-tests/diagnostics/AstConsistencyCounts.expected
+++ b/rust/ql/test/query-tests/diagnostics/AstConsistencyCounts.expected
@@ -2,3 +2,4 @@
 | Multiple parents | 0 |
 | Multiple primary QL classes | 0 |
 | Multiple toStrings | 0 |
+| No location | 0 |

From 2810d64b223fd92276a33cd11997cf3416892bf2 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 28 Nov 2024 12:43:24 +0000
Subject: [PATCH 19/51] Rust: Fix ql-for-ql warning.

---
 rust/ql/test/query-tests/security/CWE-089/SqlSinks.ql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/ql/test/query-tests/security/CWE-089/SqlSinks.ql b/rust/ql/test/query-tests/security/CWE-089/SqlSinks.ql
index d1cae882e681..ac5efd72db91 100644
--- a/rust/ql/test/query-tests/security/CWE-089/SqlSinks.ql
+++ b/rust/ql/test/query-tests/security/CWE-089/SqlSinks.ql
@@ -3,7 +3,7 @@ import codeql.rust.security.SqlInjectionExtensions
 import utils.InlineExpectationsTest
 
 module SqlSinksTest implements TestSig {
-  string getARelevantTag() { result = ["sql-sink"] }
+  string getARelevantTag() { result = "sql-sink" }
 
   predicate hasActualResult(Location location, string element, string tag, string value) {
     exists(SqlInjection::Sink sink |

From 61a4b251c0eb7e038b1dc471e9efd55ffd784cfa Mon Sep 17 00:00:00 2001
From: Ed Minnix <egregius313@github.com>
Date: Wed, 27 Nov 2024 23:24:48 -0500
Subject: [PATCH 20/51] `NavigationManager::Uri` and URI-parsing utilities

---
 ...navigationmanager.uri-and-uri-parsing-utilities.md |  8 ++++++++
 .../lib/ext/Microsoft.AspNetCore.Components.model.yml | 11 +++++++++++
 .../ext/Microsoft.AspNetCore.WebUtilities.model.yml   |  7 +++++++
 csharp/ql/lib/ext/System.Web.model.yml                |  1 +
 csharp/ql/lib/ext/generated/System.Web.model.yml      |  2 --
 5 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 csharp/ql/lib/change-notes/2024-11-27-navigationmanager.uri-and-uri-parsing-utilities.md
 create mode 100644 csharp/ql/lib/ext/Microsoft.AspNetCore.Components.model.yml
 create mode 100644 csharp/ql/lib/ext/Microsoft.AspNetCore.WebUtilities.model.yml

diff --git a/csharp/ql/lib/change-notes/2024-11-27-navigationmanager.uri-and-uri-parsing-utilities.md b/csharp/ql/lib/change-notes/2024-11-27-navigationmanager.uri-and-uri-parsing-utilities.md
new file mode 100644
index 000000000000..c19076bf5501
--- /dev/null
+++ b/csharp/ql/lib/change-notes/2024-11-27-navigationmanager.uri-and-uri-parsing-utilities.md
@@ -0,0 +1,8 @@
+---
+category: minorAnalysis
+---
+* Added `Microsoft.AspNetCore.Components.NagivationManager::Uri` as a remote flow source, since this value may contain user-specified values.
+* Added the following URI-parsing methods as summaries, as they may be tainted with user-specified values:
+  - `System.Web.HttpUtility::ParseQueryString`
+  - `Microsoft.AspNetCore.WebUtilities.QueryHelpers::ParseQueryString`
+  - `Microsoft.AspNetCore.WebUtilities.QueryHelpers::ParseNullableQuery`
diff --git a/csharp/ql/lib/ext/Microsoft.AspNetCore.Components.model.yml b/csharp/ql/lib/ext/Microsoft.AspNetCore.Components.model.yml
new file mode 100644
index 000000000000..562860f43e1b
--- /dev/null
+++ b/csharp/ql/lib/ext/Microsoft.AspNetCore.Components.model.yml
@@ -0,0 +1,11 @@
+extensions:
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: sourceModel
+    data:
+      - ["Microsoft.AspNetCore.Components", "NagivationManager", True, "get_Uri", "", "", "ReturnValue", "remote", "manual"]
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: summaryModel
+    data:
+      - ["Microsoft.AspNetCore.Components", "NagivationManager", True, "ToAbsoluteUri", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
diff --git a/csharp/ql/lib/ext/Microsoft.AspNetCore.WebUtilities.model.yml b/csharp/ql/lib/ext/Microsoft.AspNetCore.WebUtilities.model.yml
new file mode 100644
index 000000000000..13341c66a929
--- /dev/null
+++ b/csharp/ql/lib/ext/Microsoft.AspNetCore.WebUtilities.model.yml
@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: summaryModel
+    data:
+      - ["Microsoft.AspNetCore.WebUtilities", "QueryHelpers", False, "ParseQueryString", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
+      - ["Microsoft.AspNetCore.WebUtilities", "QueryHelpers", False, "ParseNullableQuery", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
diff --git a/csharp/ql/lib/ext/System.Web.model.yml b/csharp/ql/lib/ext/System.Web.model.yml
index a2a7470ef8e9..ba644e1cc70d 100644
--- a/csharp/ql/lib/ext/System.Web.model.yml
+++ b/csharp/ql/lib/ext/System.Web.model.yml
@@ -22,6 +22,7 @@ extensions:
       - ["System.Web", "HttpUtility", False, "HtmlEncode", "(System.String,System.IO.TextWriter)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["System.Web", "HttpUtility", False, "JavaScriptStringEncode", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["System.Web", "HttpUtility", False, "JavaScriptStringEncode", "(System.String,System.Boolean)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
+      - ["System.Web", "HttpUtility", False, "ParseQueryString", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["System.Web", "HttpUtility", False, "UrlEncode", "(System.Byte[])", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["System.Web", "HttpUtility", False, "UrlEncode", "(System.Byte[],System.Int32,System.Int32)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["System.Web", "HttpUtility", False, "UrlEncode", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
diff --git a/csharp/ql/lib/ext/generated/System.Web.model.yml b/csharp/ql/lib/ext/generated/System.Web.model.yml
index 19ef940f9ea1..af5047a0fc21 100644
--- a/csharp/ql/lib/ext/generated/System.Web.model.yml
+++ b/csharp/ql/lib/ext/generated/System.Web.model.yml
@@ -25,8 +25,6 @@ extensions:
       - ["System.Web", "AspNetHostingPermissionAttribute", "AspNetHostingPermissionAttribute", "(System.Security.Permissions.SecurityAction)", "summary", "df-generated"]
       - ["System.Web", "AspNetHostingPermissionAttribute", "CreatePermission", "()", "summary", "df-generated"]
       - ["System.Web", "HttpUtility", "HtmlDecode", "(System.String,System.IO.TextWriter)", "summary", "df-generated"]
-      - ["System.Web", "HttpUtility", "ParseQueryString", "(System.String)", "summary", "df-generated"]
-      - ["System.Web", "HttpUtility", "ParseQueryString", "(System.String,System.Text.Encoding)", "summary", "df-generated"]
       - ["System.Web", "HttpUtility", "UrlDecode", "(System.Byte[],System.Int32,System.Int32,System.Text.Encoding)", "summary", "df-generated"]
       - ["System.Web", "HttpUtility", "UrlDecode", "(System.Byte[],System.Text.Encoding)", "summary", "df-generated"]
       - ["System.Web", "HttpUtility", "UrlDecode", "(System.String)", "summary", "df-generated"]

From 1c06c4aae09e7916a4f9392f55cf80a2c453ba99 Mon Sep 17 00:00:00 2001
From: Ed Minnix <egregius313@github.com>
Date: Wed, 27 Nov 2024 23:31:06 -0500
Subject: [PATCH 21/51] Fix summaries

---
 .../library-tests/dataflow/library/FlowSummaries.expected    | 5 +++--
 .../dataflow/library/FlowSummariesFiltered.expected          | 3 +++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected b/csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected
index efb6d30e660d..fd6dbc49d072 100644
--- a/csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected
+++ b/csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected
@@ -1699,6 +1699,7 @@ summary
 | Microsoft.AspNetCore.WebUtilities;HttpResponseStreamWriter;WriteLineAsync;(System.String);Argument[0];ReturnValue;taint;df-generated |
 | Microsoft.AspNetCore.WebUtilities;HttpResponseStreamWriter;WriteLineAsync;(System.String);Argument[this];ReturnValue;taint;df-generated |
 | Microsoft.AspNetCore.WebUtilities;HttpResponseStreamWriter;get_Encoding;();Argument[this];ReturnValue;taint;df-generated |
+| Microsoft.AspNetCore.WebUtilities;QueryHelpers;ParseNullableQuery;(System.String);Argument[0];ReturnValue;taint;manual |
 | Microsoft.AspNetCore;WebHost;Start;(Microsoft.AspNetCore.Http.RequestDelegate);Argument[0];Argument[0].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore;WebHost;Start;(System.Action<Microsoft.AspNetCore.Routing.IRouteBuilder>);Argument[0];Argument[0].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore;WebHost;Start;(System.String,Microsoft.AspNetCore.Http.RequestDelegate);Argument[1];Argument[1].Parameter[delegate-self];value;hq-generated |
@@ -18321,6 +18322,8 @@ summary
 | System.Web;HttpUtility;HtmlEncode;(System.String,System.IO.TextWriter);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;JavaScriptStringEncode;(System.String);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;JavaScriptStringEncode;(System.String,System.Boolean);Argument[0];ReturnValue;taint;manual |
+| System.Web;HttpUtility;ParseQueryString;(System.String);Argument[0];ReturnValue;taint;manual |
+| System.Web;HttpUtility;ParseQueryString;(System.String,System.Text.Encoding);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;UrlEncode;(System.Byte[]);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;UrlEncode;(System.Byte[],System.Int32,System.Int32);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;UrlEncode;(System.String);Argument[0];ReturnValue;taint;manual |
@@ -51126,8 +51129,6 @@ neutral
 | System.Web;AspNetHostingPermissionAttribute;CreatePermission;();summary;df-generated |
 | System.Web;HtmlString;ToHtmlString;();summary;df-generated |
 | System.Web;HttpUtility;HtmlDecode;(System.String,System.IO.TextWriter);summary;df-generated |
-| System.Web;HttpUtility;ParseQueryString;(System.String);summary;df-generated |
-| System.Web;HttpUtility;ParseQueryString;(System.String,System.Text.Encoding);summary;df-generated |
 | System.Web;HttpUtility;UrlDecode;(System.Byte[],System.Int32,System.Int32,System.Text.Encoding);summary;df-generated |
 | System.Web;HttpUtility;UrlDecode;(System.Byte[],System.Text.Encoding);summary;df-generated |
 | System.Web;HttpUtility;UrlDecode;(System.String);summary;df-generated |
diff --git a/csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.expected b/csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.expected
index 6fb375dae859..d6da40e60964 100644
--- a/csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.expected
+++ b/csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.expected
@@ -650,6 +650,7 @@
 | Microsoft.AspNetCore.WebUtilities;FileBufferingReadStream;FileBufferingReadStream;(System.IO.Stream,System.Int32,System.Nullable<System.Int64>,System.Func<System.String>);Argument[3];Argument[3].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore.WebUtilities;FileBufferingReadStream;FileBufferingReadStream;(System.IO.Stream,System.Int32,System.Nullable<System.Int64>,System.Func<System.String>,System.Buffers.ArrayPool<System.Byte>);Argument[3];Argument[3].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore.WebUtilities;FileBufferingWriteStream;FileBufferingWriteStream;(System.Int32,System.Nullable<System.Int64>,System.Func<System.String>);Argument[2];Argument[2].Parameter[delegate-self];value;hq-generated |
+| Microsoft.AspNetCore.WebUtilities;QueryHelpers;ParseNullableQuery;(System.String);Argument[0];ReturnValue;taint;manual |
 | Microsoft.AspNetCore;WebHost;Start;(Microsoft.AspNetCore.Http.RequestDelegate);Argument[0];Argument[0].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore;WebHost;Start;(System.Action<Microsoft.AspNetCore.Routing.IRouteBuilder>);Argument[0];Argument[0].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore;WebHost;Start;(System.String,Microsoft.AspNetCore.Http.RequestDelegate);Argument[1];Argument[1].Parameter[delegate-self];value;hq-generated |
@@ -13988,6 +13989,8 @@
 | System.Web;HttpUtility;HtmlEncode;(System.String,System.IO.TextWriter);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;JavaScriptStringEncode;(System.String);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;JavaScriptStringEncode;(System.String,System.Boolean);Argument[0];ReturnValue;taint;manual |
+| System.Web;HttpUtility;ParseQueryString;(System.String);Argument[0];ReturnValue;taint;manual |
+| System.Web;HttpUtility;ParseQueryString;(System.String,System.Text.Encoding);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;UrlEncode;(System.Byte[]);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;UrlEncode;(System.Byte[],System.Int32,System.Int32);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;UrlEncode;(System.String);Argument[0];ReturnValue;taint;manual |

From 5bcc694f6afce6ba3988b9839761fe662e15a80b Mon Sep 17 00:00:00 2001
From: Ed Minnix <egregius313@github.com>
Date: Thu, 28 Nov 2024 09:06:18 -0500
Subject: [PATCH 22/51] Fix typo

---
 ...024-11-27-navigationmanager.uri-and-uri-parsing-utilities.md | 2 +-
 csharp/ql/lib/ext/Microsoft.AspNetCore.WebUtilities.model.yml   | 2 +-
 .../test/library-tests/dataflow/library/FlowSummaries.expected  | 1 +
 .../dataflow/library/FlowSummariesFiltered.expected             | 1 +
 4 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/csharp/ql/lib/change-notes/2024-11-27-navigationmanager.uri-and-uri-parsing-utilities.md b/csharp/ql/lib/change-notes/2024-11-27-navigationmanager.uri-and-uri-parsing-utilities.md
index c19076bf5501..2d9866c2e158 100644
--- a/csharp/ql/lib/change-notes/2024-11-27-navigationmanager.uri-and-uri-parsing-utilities.md
+++ b/csharp/ql/lib/change-notes/2024-11-27-navigationmanager.uri-and-uri-parsing-utilities.md
@@ -4,5 +4,5 @@ category: minorAnalysis
 * Added `Microsoft.AspNetCore.Components.NagivationManager::Uri` as a remote flow source, since this value may contain user-specified values.
 * Added the following URI-parsing methods as summaries, as they may be tainted with user-specified values:
   - `System.Web.HttpUtility::ParseQueryString`
-  - `Microsoft.AspNetCore.WebUtilities.QueryHelpers::ParseQueryString`
+  - `Microsoft.AspNetCore.WebUtilities.QueryHelpers::ParseQuery`
   - `Microsoft.AspNetCore.WebUtilities.QueryHelpers::ParseNullableQuery`
diff --git a/csharp/ql/lib/ext/Microsoft.AspNetCore.WebUtilities.model.yml b/csharp/ql/lib/ext/Microsoft.AspNetCore.WebUtilities.model.yml
index 13341c66a929..3554f5bb7a61 100644
--- a/csharp/ql/lib/ext/Microsoft.AspNetCore.WebUtilities.model.yml
+++ b/csharp/ql/lib/ext/Microsoft.AspNetCore.WebUtilities.model.yml
@@ -3,5 +3,5 @@ extensions:
       pack: codeql/csharp-all
       extensible: summaryModel
     data:
-      - ["Microsoft.AspNetCore.WebUtilities", "QueryHelpers", False, "ParseQueryString", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
+      - ["Microsoft.AspNetCore.WebUtilities", "QueryHelpers", False, "ParseQuery", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["Microsoft.AspNetCore.WebUtilities", "QueryHelpers", False, "ParseNullableQuery", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
diff --git a/csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected b/csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected
index fd6dbc49d072..033c83df3bdf 100644
--- a/csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected
+++ b/csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected
@@ -1700,6 +1700,7 @@ summary
 | Microsoft.AspNetCore.WebUtilities;HttpResponseStreamWriter;WriteLineAsync;(System.String);Argument[this];ReturnValue;taint;df-generated |
 | Microsoft.AspNetCore.WebUtilities;HttpResponseStreamWriter;get_Encoding;();Argument[this];ReturnValue;taint;df-generated |
 | Microsoft.AspNetCore.WebUtilities;QueryHelpers;ParseNullableQuery;(System.String);Argument[0];ReturnValue;taint;manual |
+| Microsoft.AspNetCore.WebUtilities;QueryHelpers;ParseQuery;(System.String);Argument[0];ReturnValue;taint;manual |
 | Microsoft.AspNetCore;WebHost;Start;(Microsoft.AspNetCore.Http.RequestDelegate);Argument[0];Argument[0].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore;WebHost;Start;(System.Action<Microsoft.AspNetCore.Routing.IRouteBuilder>);Argument[0];Argument[0].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore;WebHost;Start;(System.String,Microsoft.AspNetCore.Http.RequestDelegate);Argument[1];Argument[1].Parameter[delegate-self];value;hq-generated |
diff --git a/csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.expected b/csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.expected
index d6da40e60964..199ccee5a503 100644
--- a/csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.expected
+++ b/csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.expected
@@ -651,6 +651,7 @@
 | Microsoft.AspNetCore.WebUtilities;FileBufferingReadStream;FileBufferingReadStream;(System.IO.Stream,System.Int32,System.Nullable<System.Int64>,System.Func<System.String>,System.Buffers.ArrayPool<System.Byte>);Argument[3];Argument[3].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore.WebUtilities;FileBufferingWriteStream;FileBufferingWriteStream;(System.Int32,System.Nullable<System.Int64>,System.Func<System.String>);Argument[2];Argument[2].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore.WebUtilities;QueryHelpers;ParseNullableQuery;(System.String);Argument[0];ReturnValue;taint;manual |
+| Microsoft.AspNetCore.WebUtilities;QueryHelpers;ParseQuery;(System.String);Argument[0];ReturnValue;taint;manual |
 | Microsoft.AspNetCore;WebHost;Start;(Microsoft.AspNetCore.Http.RequestDelegate);Argument[0];Argument[0].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore;WebHost;Start;(System.Action<Microsoft.AspNetCore.Routing.IRouteBuilder>);Argument[0];Argument[0].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore;WebHost;Start;(System.String,Microsoft.AspNetCore.Http.RequestDelegate);Argument[1];Argument[1].Parameter[delegate-self];value;hq-generated |

From 1d0338444ae710ce25238b7eff002b0477a66586 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 28 Nov 2024 13:30:43 +0000
Subject: [PATCH 23/51] Rust: Fix SqlExecute.

---
 rust/ql/lib/codeql/rust/frameworks/Sqlx.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll b/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll
index 701cafd277a6..f00c73754486 100644
--- a/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll
+++ b/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll
@@ -28,7 +28,7 @@ private class SqlxQuery extends SqlConstruction::Range {
 /**
  * A call to `sqlx::Executor::execute`.
  */
-private class SqlxExecute extends SqlConstruction::Range {
+private class SqlxExecute extends SqlExecution::Range {
   MethodCallExpr call;
 
   SqlxExecute() {

From 5b50a8270df5bd5c70ec02db9363e4a60f02e48e Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Thu, 28 Nov 2024 13:30:57 +0000
Subject: [PATCH 24/51] Rust: Clarify the doc on the two models a little.

---
 rust/ql/lib/codeql/rust/Concepts.qll | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/rust/ql/lib/codeql/rust/Concepts.qll b/rust/ql/lib/codeql/rust/Concepts.qll
index 9369668923f8..7e3ec0990ce3 100644
--- a/rust/ql/lib/codeql/rust/Concepts.qll
+++ b/rust/ql/lib/codeql/rust/Concepts.qll
@@ -105,7 +105,7 @@ module RemoteSource {
 }
 
 /**
- * A data-flow node that constructs a SQL statement.
+ * A data-flow node that constructs a SQL statement (for later execution).
  *
  * Often, it is worthy of an alert if a SQL statement is constructed such that
  * executing it would be a security risk.
@@ -133,10 +133,10 @@ module SqlConstruction {
 }
 
 /**
- * A data-flow node that executes SQL statements.
+ * A data-flow node that constructs and executes SQL statements.
  *
  * If the context of interest is such that merely constructing a SQL statement
- * would be valuable to report, consider using `SqlConstruction`.
+ * would be valuable to report, consider also using `SqlConstruction`.
  *
  * Extend this class to refine existing API models. If you want to model new APIs,
  * extend `SqlExecution::Range` instead.

From a7a77a5f23b30dc39ccc73b942773adbe3454768 Mon Sep 17 00:00:00 2001
From: Ed Minnix <egregius313@github.com>
Date: Thu, 28 Nov 2024 09:16:45 -0500
Subject: [PATCH 25/51] Added `NavigationManager::BaseUri`

---
 csharp/ql/lib/ext/Microsoft.AspNetCore.Components.model.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/csharp/ql/lib/ext/Microsoft.AspNetCore.Components.model.yml b/csharp/ql/lib/ext/Microsoft.AspNetCore.Components.model.yml
index 562860f43e1b..13c7168a1d19 100644
--- a/csharp/ql/lib/ext/Microsoft.AspNetCore.Components.model.yml
+++ b/csharp/ql/lib/ext/Microsoft.AspNetCore.Components.model.yml
@@ -3,6 +3,7 @@ extensions:
       pack: codeql/csharp-all
       extensible: sourceModel
     data:
+      - ["Microsoft.AspNetCore.Components", "NagivationManager", True, "get_BaseUri", "", "", "ReturnValue", "remote", "manual"]
       - ["Microsoft.AspNetCore.Components", "NagivationManager", True, "get_Uri", "", "", "ReturnValue", "remote", "manual"]
   - addsTo:
       pack: codeql/csharp-all

From a6f20a6ac1505e54cb9769482a50a2233864f1b2 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 29 Nov 2024 09:09:59 +0000
Subject: [PATCH 26/51] Apply suggestions from code review

Co-authored-by: mc <42146119+mchammer01@users.noreply.github.com>
---
 .../queries/security/CWE-696/BadCtorInitialization.qhelp    | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
index 82683ccfdba4..8dd16d6f6b13 100644
--- a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
+++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
@@ -5,7 +5,7 @@
 <overview>
 
 <p>
-Calling functions and methods in the Rust <code>std</code> library from a <code>#[ctor]</code> or <code>#[dtor]</code> function is not safe. This is because the <code>std</code> library only guarantees stability and portability between the beginning and end of <code>main</code>, whereas <code>#[ctor]</code> functions are called before <code>main</code>, and <code>#[dtor]</code> functions are called after it.
+Calling functions and methods in the Rust <code>std</code> library from a <code>#[ctor]</code> or <code>#[dtor]</code> function is not safe. This is because the <code>std</code> library only guarantees stability and portability between the beginning and the end of <code>main</code>, whereas <code>#[ctor]</code> functions are called before <code>main</code>, and <code>#[dtor]</code> functions are called after it.
 </p>
 
 </overview>
@@ -23,13 +23,13 @@ Do not call any part of the <code>std</code> library from a <code>#[ctor]</code>
 <example>
 
 <p>
-In the following example, a <code>#[ctor]</code> function uses the <code>println!</code> macro which calls <code>std</code> library functions. This may cause unexpected behaviour at runtime.
+In the following example, a <code>#[ctor]</code> function uses the <code>println!</code> macro which calls <code>std</code> library functions. This may cause unexpected behavior at runtime.
 </p>
 
 <sample src="BadCtorInitializationBad.rs" />
 
 <p>
-The issue can be fixed by replacing <code>println!</code> with something that does not rely on the <code>std</code> library. In the fixed code below we use the <code>libc_println!</code> macro from the <code>libc-print</code> library:
+The issue can be fixed by replacing <code>println!</code> with something that does not rely on the <code>std</code> library. In the fixed code below, we used the <code>libc_println!</code> macro from the <code>libc-print</code> library:
 </p>
 
 <sample src="BadCtorInitializationGood.rs" />

From 49b569cc4b58d757cd6b2d02a1ce865effe32431 Mon Sep 17 00:00:00 2001
From: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
Date: Fri, 29 Nov 2024 09:36:11 +0000
Subject: [PATCH 27/51] Rust: Update for changes on main.

---
 rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
index 8d434b1f6e4d..22ea6514e02e 100644
--- a/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
+++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
@@ -32,7 +32,7 @@ class CtorAttr extends Attr {
  */
 class StdCall extends Expr {
   StdCall() {
-    this.(CallExpr).getExpr().(PathExpr).getPath().getResolvedCrateOrigin() = "lang:std" or
+    this.(CallExpr).getFunction().(PathExpr).getPath().getResolvedCrateOrigin() = "lang:std" or
     this.(MethodCallExpr).getResolvedCrateOrigin() = "lang:std"
   }
 }

From e93ce7c7d5406d826caf3b676b49df77ad913ebe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=93scar=20San=20Jos=C3=A9?= <oscarsj@github.com>
Date: Fri, 29 Nov 2024 12:55:11 +0100
Subject: [PATCH 28/51] Add .devcontainer folder to CODEOWNERS

---
 CODEOWNERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/CODEOWNERS b/CODEOWNERS
index 6e1c975ace7d..1912a138758c 100644
--- a/CODEOWNERS
+++ b/CODEOWNERS
@@ -42,3 +42,6 @@ MODULE.bazel @github/codeql-ci-reviewers
 # Misc
 /misc/scripts/accept-expected-changes-from-ci.py @RasmusWL
 /misc/scripts/generate-code-scanning-query-list.py @RasmusWL
+
+# .devcontainer
+/.devcontainer/ @github/codeql-ci-reviewers

From 5020e36d0a065c66643d4bbec55d725ef28a18a2 Mon Sep 17 00:00:00 2001
From: Michael Nebel <michaelnebel@github.com>
Date: Tue, 26 Nov 2024 13:12:30 +0100
Subject: [PATCH 29/51] C#: Add launch task for debugging the tracing
 extractor.

---
 csharp/.vscode/launch.json | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/csharp/.vscode/launch.json b/csharp/.vscode/launch.json
index 1f598f5e44f7..f5b4aa79cc11 100644
--- a/csharp/.vscode/launch.json
+++ b/csharp/.vscode/launch.json
@@ -61,5 +61,21 @@
             ],
             "env": {}
         },
+        {
+            "name": "C#: Tracing Debug",
+            "type": "coreclr",
+            "request": "launch",
+            "preLaunchTask": "dotnet: build",
+            "program": "${workspaceFolder}/extractor/Semmle.Extraction.CSharp.Driver/bin/Debug/net9.0/Semmle.Extraction.CSharp.Driver.dll",
+            // Set the path to the folder that should be extracted:
+            "cwd": "${workspaceFolder}/ql/test/library-tests/dataflow/local",
+            "args": [
+                "LocalDataFlow.cs"
+            ],
+            "env": {},
+            "stopAtEntry": true,
+            "justMyCode": false,
+            "suppressJITOptimizations": true
+        },
     ]
 }

From 5b6a4e616c2098e2d6a181f04fd4c817bef5bceb Mon Sep 17 00:00:00 2001
From: Simon Friis Vindum <simonfv@gmail.com>
Date: Fri, 29 Nov 2024 13:42:30 +0100
Subject: [PATCH 30/51] Rust: Update stats queries to use shared data flow
 consistency module

---
 rust/ql/src/queries/summary/Stats.qll | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/rust/ql/src/queries/summary/Stats.qll b/rust/ql/src/queries/summary/Stats.qll
index aec671098eb1..c2993b47899f 100644
--- a/rust/ql/src/queries/summary/Stats.qll
+++ b/rust/ql/src/queries/summary/Stats.qll
@@ -7,7 +7,7 @@ private import codeql.rust.dataflow.internal.DataFlowImpl
 private import codeql.rust.dataflow.internal.TaintTrackingImpl
 private import codeql.rust.AstConsistency as AstConsistency
 private import codeql.rust.controlflow.internal.CfgConsistency as CfgConsistency
-private import codeql.dataflow.internal.DataFlowImplConsistency as DataFlowImplConsistency
+private import codeql.rust.dataflow.internal.DataFlowConsistency as DataFlowConsistency
 
 /**
  * Gets a count of the total number of lines of code in the database.
@@ -35,15 +35,9 @@ int getTotalCfgInconsistencies() {
   result = sum(string type | | CfgConsistency::getCfgInconsistencyCounts(type))
 }
 
-private module Input implements DataFlowImplConsistency::InputSig<Location, RustDataFlow> { }
-
 /**
  * Gets a count of the total number of data flow inconsistencies in the database.
  */
 int getTotalDataFlowInconsistencies() {
-  result =
-    sum(string type |
-      |
-      DataFlowImplConsistency::MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>::getInconsistencyCounts(type)
-    )
+  result = sum(string type | | DataFlowConsistency::getInconsistencyCounts(type))
 }

From 986e1cb597ccce863fa238d637623f3dd854b2c9 Mon Sep 17 00:00:00 2001
From: Napalys <napalys@github.com>
Date: Fri, 29 Nov 2024 14:33:40 +0100
Subject: [PATCH 31/51] Add ValidatePredicateGetReturns query and tests

---
 .../style/ValidatePredicateGetReturns.ql      | 40 +++++++++++++++++++
 .../ValidatePredicateGetReturns.expected      |  2 +
 .../ValidatePredicateGetReturns.qlref         |  1 +
 .../ValidatePredicateGetReturns/test.qll      | 28 +++++++++++++
 4 files changed, 71 insertions(+)
 create mode 100644 ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
 create mode 100644 ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
 create mode 100644 ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.qlref
 create mode 100644 ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll

diff --git a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
new file mode 100644
index 000000000000..2a3de1cfb3b5
--- /dev/null
+++ b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
@@ -0,0 +1,40 @@
+/**
+ * @name Predicates starting with "get" should return a value
+ * @description Checks if predicates that start with "get" actually return a value.
+ * @kind problem
+ * @problem.severity warning
+ * @id ql/predicates-get-should-return-value
+ * @tags correctness
+ *       maintainability
+ * @precision high
+ */
+
+import ql
+import codeql_ql.ast.Ast
+
+/**
+ * Identifies predicates whose names start with "get" followed by an uppercase letter.
+ * This ensures that only predicates like "getValue" are matched, excluding names like "getter".
+ */
+predicate isGetPredicate(Predicate pred) { pred.getName().regexpMatch("get[A-Z].*") }
+
+/**
+ * Checks if a predicate has a return type.
+ */
+predicate hasReturnType(Predicate pred) {
+  exists(Type returnType | pred.getReturnType() = returnType)
+}
+
+/**
+ * Checks if a predicate is an alias using getAlias().
+ */
+predicate isAlias(Predicate pred) {
+  pred instanceof ClasslessPredicate and exists(pred.(ClasslessPredicate).getAlias())
+}
+
+from Predicate pred
+where
+  isGetPredicate(pred) and
+  not hasReturnType(pred) and
+  not isAlias(pred)
+select pred, "This predicate starts with 'get' but does not return a value."
diff --git a/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
new file mode 100644
index 000000000000..b78fe32cc75b
--- /dev/null
+++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
@@ -0,0 +1,2 @@
+| test.qll:4:11:4:18 | ClasslessPredicate getValue | This predicate starts with 'get' but does not return a value. |
+| test.qll:25:11:25:28 | ClasslessPredicate getImplementation2 | This predicate starts with 'get' but does not return a value. |
diff --git a/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.qlref b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.qlref
new file mode 100644
index 000000000000..e116f69d6b22
--- /dev/null
+++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.qlref
@@ -0,0 +1 @@
+queries/style/ValidatePredicateGetReturns.ql
diff --git a/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll b/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
new file mode 100644
index 000000000000..b62b0e2dbaf3
--- /dev/null
+++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
@@ -0,0 +1,28 @@
+import ql
+
+// NOT OK -- Predicate starts with "get" but does not return a value
+predicate getValue() { none() }
+
+// OK -- starts with get and returns a value
+string getData() { result = "data" }
+
+// OK -- starts with get but followed by a lowercase letter, probably should be ignored
+predicate getterFunction() { none() }
+
+// OK -- starts with get and returns a value
+string getImplementation() { result = "implementation" }
+
+// OK -- is an alias
+predicate getAlias = getImplementation/0;
+
+// OK -- Starts with "get" but followed by a lowercase letter, probably be ignored
+predicate getvalue() { none() }
+
+// OK -- Does not start with "get", should be ignored
+predicate retrieveValue() { none() }
+
+// NOT OK -- starts with get and does not return value
+predicate getImplementation2() { none() }
+
+// OK -- is an alias
+predicate getAlias2 = getImplementation2/0;

From a763dd7267ea1f88ddf904578b656a12e6d12ee1 Mon Sep 17 00:00:00 2001
From: Napalys <napalys@github.com>
Date: Fri, 29 Nov 2024 14:55:53 +0100
Subject: [PATCH 32/51] Fixed github-advanced-security bot warning

---
 ql/ql/src/queries/style/ValidatePredicateGetReturns.ql | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
index 2a3de1cfb3b5..d6982c757106 100644
--- a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
+++ b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
@@ -21,9 +21,7 @@ predicate isGetPredicate(Predicate pred) { pred.getName().regexpMatch("get[A-Z].
 /**
  * Checks if a predicate has a return type.
  */
-predicate hasReturnType(Predicate pred) {
-  exists(Type returnType | pred.getReturnType() = returnType)
-}
+predicate hasReturnType(Predicate pred) { exists(pred.getReturnType()) }
 
 /**
  * Checks if a predicate is an alias using getAlias().

From 5d79ed6a9e749be2859845e3dbea163195d78112 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 29 Nov 2024 15:08:33 +0100
Subject: [PATCH 33/51] C#: WIP: Fix calls with no target in DB quality query

---
 csharp/ql/src/Telemetry/DatabaseQuality.qll   |  9 +++++++-
 .../DatabaseQuality/IsNotOkayCall.expected    |  4 ++++
 .../DatabaseQuality/NoTarget.expected         |  8 ++++++-
 .../Telemetry/DatabaseQuality/Quality.cs      | 21 +++++++++++++++++++
 4 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/csharp/ql/src/Telemetry/DatabaseQuality.qll b/csharp/ql/src/Telemetry/DatabaseQuality.qll
index 1ca553ffd314..a5288ae5aea2 100644
--- a/csharp/ql/src/Telemetry/DatabaseQuality.qll
+++ b/csharp/ql/src/Telemetry/DatabaseQuality.qll
@@ -77,6 +77,12 @@ module CallTargetStats implements StatsSig {
     )
   }
 
+  private predicate isEventFieldAccess(EventCall c) {
+    exists(Event e | c.getEvent() = e |
+      forall(Accessor a | e.getAnAccessor() = a | a.isCompilerGenerated())
+    )
+  }
+
   additional predicate isNotOkCall(Call c) {
     not exists(c.getTarget()) and
     not c instanceof DelegateCall and
@@ -85,7 +91,8 @@ module CallTargetStats implements StatsSig {
     not isNoSetterPropertyInitialization(c) and
     not isAnonymousObjectMemberDeclaration(c) and
     not isInitializedWithCollectionInitializer(c) and
-    not c.getParent+() instanceof NameOfExpr
+    not c.getParent+() instanceof NameOfExpr and
+    not isEventFieldAccess(c)
   }
 
   int getNumberOfNotOk() { result = count(Call c | isNotOkCall(c)) }
diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected
index e69de29bb2d1..5199f9be9dc4 100644
--- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected
+++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected
@@ -0,0 +1,4 @@
+| Quality.cs:25:19:25:26 | access to indexer | Call without target $@. | Quality.cs:25:19:25:26 | access to indexer | access to indexer |
+| Quality.cs:28:21:28:27 | access to indexer | Call without target $@. | Quality.cs:28:21:28:27 | access to indexer | access to indexer |
+| Quality.cs:31:9:31:21 | access to indexer | Call without target $@. | Quality.cs:31:9:31:21 | access to indexer | access to indexer |
+| Quality.cs:46:20:46:26 | object creation of type T | Call without target $@. | Quality.cs:46:20:46:26 | object creation of type T | object creation of type T |
diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected
index 28f090d3d3d9..917f3c7010ab 100644
--- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected
+++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected
@@ -4,4 +4,10 @@
 | Quality.cs:15:24:15:34 | access to property MyProperty3 | Call without target $@. | Quality.cs:15:24:15:34 | access to property MyProperty3 | access to property MyProperty3 |
 | Quality.cs:15:24:15:46 | access to property MyProperty2 | Call without target $@. | Quality.cs:15:24:15:46 | access to property MyProperty2 | access to property MyProperty2 |
 | Quality.cs:19:13:19:23 | access to property MyProperty4 | Call without target $@. | Quality.cs:19:13:19:23 | access to property MyProperty4 | access to property MyProperty4 |
-| Quality.cs:24:16:24:26 | access to property MyProperty2 | Call without target $@. | Quality.cs:24:16:24:26 | access to property MyProperty2 | access to property MyProperty2 |
+| Quality.cs:22:9:22:14 | access to event Event1 | Call without target $@. | Quality.cs:22:9:22:14 | access to event Event1 | access to event Event1 |
+| Quality.cs:22:9:22:30 | delegate call | Call without target $@. | Quality.cs:22:9:22:30 | delegate call | delegate call |
+| Quality.cs:25:19:25:26 | access to indexer | Call without target $@. | Quality.cs:25:19:25:26 | access to indexer | access to indexer |
+| Quality.cs:28:21:28:27 | access to indexer | Call without target $@. | Quality.cs:28:21:28:27 | access to indexer | access to indexer |
+| Quality.cs:31:9:31:21 | access to indexer | Call without target $@. | Quality.cs:31:9:31:21 | access to indexer | access to indexer |
+| Quality.cs:35:16:35:26 | access to property MyProperty2 | Call without target $@. | Quality.cs:35:16:35:26 | access to property MyProperty2 | access to property MyProperty2 |
+| Quality.cs:46:20:46:26 | object creation of type T | Call without target $@. | Quality.cs:46:20:46:26 | object creation of type T | object creation of type T |
diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs
index 18d0ec678b62..4d1abf13634a 100644
--- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs
+++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs
@@ -18,6 +18,17 @@ public Test()
         {
             MyProperty4 = { 1, 2, 3 }
         };
+
+        Event1.Invoke(this, 5);
+
+        var str = "abcd";
+        var sub = str[..3];
+
+        Span<int> sp = null;
+        var slice = sp[..3];
+
+        Span<byte> guidBytes = stackalloc byte[16];
+        guidBytes[08] = 1;
     }
 
     public int MyProperty1 { get; }
@@ -25,4 +36,14 @@ public Test()
     public Test MyProperty3 { get; set; }
     public List<int> MyProperty4 { get; }
     static int MyProperty5 { get; }
+
+    public event EventHandler<int> Event1;
+
+    class Gen<T> where T : new()
+    {
+        public static T Factory()
+        {
+            return new T();
+        }
+    }
 }

From a5521b90fceecad9e7b38e4af48955b3e45b89c9 Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Fri, 29 Nov 2024 15:13:46 +0100
Subject: [PATCH 34/51] Update
 ql/ql/src/queries/style/ValidatePredicateGetReturns.ql

Co-authored-by: Anders Schack-Mulligen <aschackmull@users.noreply.github.com>
---
 ql/ql/src/queries/style/ValidatePredicateGetReturns.ql | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
index d6982c757106..297d141754e7 100644
--- a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
+++ b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
@@ -26,9 +26,7 @@ predicate hasReturnType(Predicate pred) { exists(pred.getReturnType()) }
 /**
  * Checks if a predicate is an alias using getAlias().
  */
-predicate isAlias(Predicate pred) {
-  pred instanceof ClasslessPredicate and exists(pred.(ClasslessPredicate).getAlias())
-}
+predicate isAlias(Predicate pred) { exists(pred.(ClasslessPredicate).getAlias()) }
 
 from Predicate pred
 where

From 029b567bb722f2416676e8f551e6ef63418ed07d Mon Sep 17 00:00:00 2001
From: Napalys Klicius <napalys@github.com>
Date: Fri, 29 Nov 2024 15:19:19 +0100
Subject: [PATCH 35/51] Update
 ql/ql/src/queries/style/ValidatePredicateGetReturns.ql

Co-authored-by: Anders Schack-Mulligen <aschackmull@users.noreply.github.com>
---
 ql/ql/src/queries/style/ValidatePredicateGetReturns.ql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
index 297d141754e7..eae483d287f4 100644
--- a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
+++ b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
@@ -16,7 +16,7 @@ import codeql_ql.ast.Ast
  * Identifies predicates whose names start with "get" followed by an uppercase letter.
  * This ensures that only predicates like "getValue" are matched, excluding names like "getter".
  */
-predicate isGetPredicate(Predicate pred) { pred.getName().regexpMatch("get[A-Z].*") }
+predicate isGetPredicate(Predicate pred) { pred.getName().regexpMatch("(get|as)[A-Z].*") }
 
 /**
  * Checks if a predicate has a return type.

From e33f7aa1c7f3180670cf580fc9473358e9ae3cf2 Mon Sep 17 00:00:00 2001
From: Napalys <napalys@github.com>
Date: Fri, 29 Nov 2024 15:23:06 +0100
Subject: [PATCH 36/51] Added test cases for 'as' prefix

---
 .../ValidatePredicateGetReturns.expected                 | 1 +
 .../queries/style/ValidatePredicateGetReturns/test.qll   | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
index b78fe32cc75b..a79e572ed1cd 100644
--- a/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
+++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
@@ -1,2 +1,3 @@
 | test.qll:4:11:4:18 | ClasslessPredicate getValue | This predicate starts with 'get' but does not return a value. |
 | test.qll:25:11:25:28 | ClasslessPredicate getImplementation2 | This predicate starts with 'get' but does not return a value. |
+| test.qll:31:11:31:17 | ClasslessPredicate asValue | This predicate starts with 'get' but does not return a value. |
diff --git a/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll b/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
index b62b0e2dbaf3..b18649ea8cc7 100644
--- a/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
+++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
@@ -26,3 +26,12 @@ predicate getImplementation2() { none() }
 
 // OK -- is an alias
 predicate getAlias2 = getImplementation2/0;
+
+// NOT OK -- starts with as and does not return value
+predicate asValue() { none() }
+
+// OK -- starts with as but followed by a lowercase letter, probably should be ignored
+predicate assessment() { none() }
+
+// OK -- starts with as and returns a value
+string asString() { result = "string" }

From 96c1086dfc674ae6e5ddece815d0991e8ae9f309 Mon Sep 17 00:00:00 2001
From: Napalys <napalys@github.com>
Date: Fri, 29 Nov 2024 15:35:18 +0100
Subject: [PATCH 37/51] Modified comments to reflect 'as' changes

---
 ql/ql/src/queries/style/ValidatePredicateGetReturns.ql | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
index eae483d287f4..065e24f9b2e4 100644
--- a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
+++ b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
@@ -1,6 +1,6 @@
 /**
- * @name Predicates starting with "get" should return a value
- * @description Checks if predicates that start with "get" actually return a value.
+ * @name Predicates starting with "get" or "as" should return a value
+ * @description Checks if predicates that start with "get" or "as" actually return a value.
  * @kind problem
  * @problem.severity warning
  * @id ql/predicates-get-should-return-value
@@ -13,7 +13,7 @@ import ql
 import codeql_ql.ast.Ast
 
 /**
- * Identifies predicates whose names start with "get" followed by an uppercase letter.
+ * Identifies predicates whose names start with "get", "as" followed by an uppercase letter.
  * This ensures that only predicates like "getValue" are matched, excluding names like "getter".
  */
 predicate isGetPredicate(Predicate pred) { pred.getName().regexpMatch("(get|as)[A-Z].*") }

From a462ec91f5d6e8e881f370a6823202f8c6f1c400 Mon Sep 17 00:00:00 2001
From: Napalys <napalys@github.com>
Date: Fri, 29 Nov 2024 15:54:58 +0100
Subject: [PATCH 38/51] Now the error message reflects properly the prefix

---
 .../queries/style/ValidatePredicateGetReturns.ql   | 14 +++++++++++++-
 .../ValidatePredicateGetReturns.expected           |  2 +-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
index 065e24f9b2e4..ad7db6e05907 100644
--- a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
+++ b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
@@ -28,9 +28,21 @@ predicate hasReturnType(Predicate pred) { exists(pred.getReturnType()) }
  */
 predicate isAlias(Predicate pred) { exists(pred.(ClasslessPredicate).getAlias()) }
 
+/**
+ * Returns "get" if the predicate name starts with "get", otherwise "as".
+ */
+string getPrefix(Predicate pred) {
+  if pred.getName().matches("get%")
+  then result = "get"
+  else
+    if pred.getName().matches("as%")
+    then result = "as"
+    else result = ""
+}
+
 from Predicate pred
 where
   isGetPredicate(pred) and
   not hasReturnType(pred) and
   not isAlias(pred)
-select pred, "This predicate starts with 'get' but does not return a value."
+select pred, "This predicate starts with '" + getPrefix(pred) + "' but does not return a value."
diff --git a/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
index a79e572ed1cd..57469246ae82 100644
--- a/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
+++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
@@ -1,3 +1,3 @@
 | test.qll:4:11:4:18 | ClasslessPredicate getValue | This predicate starts with 'get' but does not return a value. |
 | test.qll:25:11:25:28 | ClasslessPredicate getImplementation2 | This predicate starts with 'get' but does not return a value. |
-| test.qll:31:11:31:17 | ClasslessPredicate asValue | This predicate starts with 'get' but does not return a value. |
+| test.qll:31:11:31:17 | ClasslessPredicate asValue | This predicate starts with 'as' but does not return a value. |

From b3896df15cf121464afb8a917a0ba56374a7add1 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 29 Nov 2024 16:05:25 +0100
Subject: [PATCH 39/51] Exclude type parameter instantiations from DB quality
 query

---
 csharp/ql/src/Telemetry/DatabaseQuality.qll                | 7 ++++++-
 .../Telemetry/DatabaseQuality/IsNotOkayCall.expected       | 1 -
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/csharp/ql/src/Telemetry/DatabaseQuality.qll b/csharp/ql/src/Telemetry/DatabaseQuality.qll
index a5288ae5aea2..ce29fa19795d 100644
--- a/csharp/ql/src/Telemetry/DatabaseQuality.qll
+++ b/csharp/ql/src/Telemetry/DatabaseQuality.qll
@@ -83,6 +83,10 @@ module CallTargetStats implements StatsSig {
     )
   }
 
+  private predicate isTypeParameterInstantiation(ObjectCreation e) {
+    e.getType() instanceof TypeParameter
+  }
+
   additional predicate isNotOkCall(Call c) {
     not exists(c.getTarget()) and
     not c instanceof DelegateCall and
@@ -92,7 +96,8 @@ module CallTargetStats implements StatsSig {
     not isAnonymousObjectMemberDeclaration(c) and
     not isInitializedWithCollectionInitializer(c) and
     not c.getParent+() instanceof NameOfExpr and
-    not isEventFieldAccess(c)
+    not isEventFieldAccess(c) and
+    not isTypeParameterInstantiation(c)
   }
 
   int getNumberOfNotOk() { result = count(Call c | isNotOkCall(c)) }
diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected
index 5199f9be9dc4..be6dbae45854 100644
--- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected
+++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected
@@ -1,4 +1,3 @@
 | Quality.cs:25:19:25:26 | access to indexer | Call without target $@. | Quality.cs:25:19:25:26 | access to indexer | access to indexer |
 | Quality.cs:28:21:28:27 | access to indexer | Call without target $@. | Quality.cs:28:21:28:27 | access to indexer | access to indexer |
 | Quality.cs:31:9:31:21 | access to indexer | Call without target $@. | Quality.cs:31:9:31:21 | access to indexer | access to indexer |
-| Quality.cs:46:20:46:26 | object creation of type T | Call without target $@. | Quality.cs:46:20:46:26 | object creation of type T | object creation of type T |

From 11dedbef1bbf041d601dfd9698ffcb6a8b5b3a40 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 29 Nov 2024 16:26:40 +0100
Subject: [PATCH 40/51] Exclude property call with object initializer r-value
 from DB quality query

---
 csharp/ql/src/Telemetry/DatabaseQuality.qll       |  6 +++---
 .../DatabaseQuality/IsNotOkayCall.expected        |  6 +++---
 .../Telemetry/DatabaseQuality/NoTarget.expected   | 15 ++++++++-------
 .../Telemetry/DatabaseQuality/Quality.cs          |  4 +++-
 4 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/csharp/ql/src/Telemetry/DatabaseQuality.qll b/csharp/ql/src/Telemetry/DatabaseQuality.qll
index ce29fa19795d..eba5ebb52114 100644
--- a/csharp/ql/src/Telemetry/DatabaseQuality.qll
+++ b/csharp/ql/src/Telemetry/DatabaseQuality.qll
@@ -68,12 +68,12 @@ module CallTargetStats implements StatsSig {
     )
   }
 
-  private predicate isInitializedWithCollectionInitializer(PropertyCall c) {
+  private predicate isInitializedWithObjectOrCollectionInitializer(PropertyCall c) {
     exists(Property p, AssignExpr assign |
       p = c.getProperty() and
       assign = c.getParent() and
       assign.getLValue() = c and
-      assign.getRValue() instanceof CollectionInitializer
+      assign.getRValue() instanceof ObjectOrCollectionInitializer
     )
   }
 
@@ -94,7 +94,7 @@ module CallTargetStats implements StatsSig {
     not isNoSetterPropertyCallInConstructor(c) and
     not isNoSetterPropertyInitialization(c) and
     not isAnonymousObjectMemberDeclaration(c) and
-    not isInitializedWithCollectionInitializer(c) and
+    not isInitializedWithObjectOrCollectionInitializer(c) and
     not c.getParent+() instanceof NameOfExpr and
     not isEventFieldAccess(c) and
     not isTypeParameterInstantiation(c)
diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected
index be6dbae45854..7555a37394b5 100644
--- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected
+++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected
@@ -1,3 +1,3 @@
-| Quality.cs:25:19:25:26 | access to indexer | Call without target $@. | Quality.cs:25:19:25:26 | access to indexer | access to indexer |
-| Quality.cs:28:21:28:27 | access to indexer | Call without target $@. | Quality.cs:28:21:28:27 | access to indexer | access to indexer |
-| Quality.cs:31:9:31:21 | access to indexer | Call without target $@. | Quality.cs:31:9:31:21 | access to indexer | access to indexer |
+| Quality.cs:26:19:26:26 | access to indexer | Call without target $@. | Quality.cs:26:19:26:26 | access to indexer | access to indexer |
+| Quality.cs:29:21:29:27 | access to indexer | Call without target $@. | Quality.cs:29:21:29:27 | access to indexer | access to indexer |
+| Quality.cs:32:9:32:21 | access to indexer | Call without target $@. | Quality.cs:32:9:32:21 | access to indexer | access to indexer |
diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected
index 917f3c7010ab..fae81ab0c6d4 100644
--- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected
+++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected
@@ -4,10 +4,11 @@
 | Quality.cs:15:24:15:34 | access to property MyProperty3 | Call without target $@. | Quality.cs:15:24:15:34 | access to property MyProperty3 | access to property MyProperty3 |
 | Quality.cs:15:24:15:46 | access to property MyProperty2 | Call without target $@. | Quality.cs:15:24:15:46 | access to property MyProperty2 | access to property MyProperty2 |
 | Quality.cs:19:13:19:23 | access to property MyProperty4 | Call without target $@. | Quality.cs:19:13:19:23 | access to property MyProperty4 | access to property MyProperty4 |
-| Quality.cs:22:9:22:14 | access to event Event1 | Call without target $@. | Quality.cs:22:9:22:14 | access to event Event1 | access to event Event1 |
-| Quality.cs:22:9:22:30 | delegate call | Call without target $@. | Quality.cs:22:9:22:30 | delegate call | delegate call |
-| Quality.cs:25:19:25:26 | access to indexer | Call without target $@. | Quality.cs:25:19:25:26 | access to indexer | access to indexer |
-| Quality.cs:28:21:28:27 | access to indexer | Call without target $@. | Quality.cs:28:21:28:27 | access to indexer | access to indexer |
-| Quality.cs:31:9:31:21 | access to indexer | Call without target $@. | Quality.cs:31:9:31:21 | access to indexer | access to indexer |
-| Quality.cs:35:16:35:26 | access to property MyProperty2 | Call without target $@. | Quality.cs:35:16:35:26 | access to property MyProperty2 | access to property MyProperty2 |
-| Quality.cs:46:20:46:26 | object creation of type T | Call without target $@. | Quality.cs:46:20:46:26 | object creation of type T | object creation of type T |
+| Quality.cs:20:13:20:23 | access to property MyProperty6 | Call without target $@. | Quality.cs:20:13:20:23 | access to property MyProperty6 | access to property MyProperty6 |
+| Quality.cs:23:9:23:14 | access to event Event1 | Call without target $@. | Quality.cs:23:9:23:14 | access to event Event1 | access to event Event1 |
+| Quality.cs:23:9:23:30 | delegate call | Call without target $@. | Quality.cs:23:9:23:30 | delegate call | delegate call |
+| Quality.cs:26:19:26:26 | access to indexer | Call without target $@. | Quality.cs:26:19:26:26 | access to indexer | access to indexer |
+| Quality.cs:29:21:29:27 | access to indexer | Call without target $@. | Quality.cs:29:21:29:27 | access to indexer | access to indexer |
+| Quality.cs:32:9:32:21 | access to indexer | Call without target $@. | Quality.cs:32:9:32:21 | access to indexer | access to indexer |
+| Quality.cs:36:16:36:26 | access to property MyProperty2 | Call without target $@. | Quality.cs:36:16:36:26 | access to property MyProperty2 | access to property MyProperty2 |
+| Quality.cs:48:20:48:26 | object creation of type T | Call without target $@. | Quality.cs:48:20:48:26 | object creation of type T | object creation of type T |
diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs
index 4d1abf13634a..2c26c0041312 100644
--- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs
+++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs
@@ -16,7 +16,8 @@ public Test()
 
         new Test()
         {
-            MyProperty4 = { 1, 2, 3 }
+            MyProperty4 = { 1, 2, 3 },
+            MyProperty6 = { [1] = "" }
         };
 
         Event1.Invoke(this, 5);
@@ -36,6 +37,7 @@ public Test()
     public Test MyProperty3 { get; set; }
     public List<int> MyProperty4 { get; }
     static int MyProperty5 { get; }
+    public Dictionary<int, string> MyProperty6 { get; }
 
     public event EventHandler<int> Event1;
 

From 90fa3ec4ed358188c1bbd1aee2d9b5ca503ed314 Mon Sep 17 00:00:00 2001
From: Paolo Tranquilli <redsun82@gihub.com>
Date: Fri, 29 Nov 2024 16:46:41 +0100
Subject: [PATCH 41/51] Rust: test running windows flaky test multiple times

---
 .../integration-tests/hello-workspace/test_workspace.py  | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/rust/ql/integration-tests/hello-workspace/test_workspace.py b/rust/ql/integration-tests/hello-workspace/test_workspace.py
index 6a7e21ebedbc..99c67f6f6247 100644
--- a/rust/ql/integration-tests/hello-workspace/test_workspace.py
+++ b/rust/ql/integration-tests/hello-workspace/test_workspace.py
@@ -1,5 +1,14 @@
+import runs_on
+import pytest
+
+@runs_on.posix
 def test_cargo(codeql, rust, cargo, check_source_archive):
     codeql.database.create()
 
+@runs_on.windows
+@pytest.mark.parametrize("_", range(25))
+def test_cargo_debug(codeql, rust, cargo, check_source_archive, _):
+    codeql.database.create()
+
 def test_rust_project(codeql, rust, rust_project, check_source_archive):
     codeql.database.create()

From 488903280fb4498f164914f2549d18c78d56c4e6 Mon Sep 17 00:00:00 2001
From: Paolo Tranquilli <redsun82@gihub.com>
Date: Fri, 29 Nov 2024 17:15:55 +0100
Subject: [PATCH 42/51] Rust: tentative windows fix

---
 Cargo.lock                                    |  7 ++
 MODULE.bazel                                  |  2 +-
 .../tree_sitter_extractors_deps/BUILD.bazel   |  6 ++
 .../BUILD.dunce-1.0.5.bazel                   | 81 +++++++++++++++++++
 .../tree_sitter_extractors_deps/defs.bzl      | 12 +++
 rust/extractor/Cargo.toml                     |  1 +
 rust/extractor/src/rust_analyzer.rs           | 26 ++++--
 7 files changed, 129 insertions(+), 6 deletions(-)
 create mode 100644 misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.dunce-1.0.5.bazel

diff --git a/Cargo.lock b/Cargo.lock
index 1d5b8824c84a..2b31f24bac4b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -383,6 +383,7 @@ dependencies = [
  "argfile",
  "clap",
  "codeql-extractor",
+ "dunce",
  "figment",
  "glob",
  "itertools 0.13.0",
@@ -540,6 +541,12 @@ version = "0.1.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "9bda8e21c04aca2ae33ffc2fd8c23134f3cac46db123ba97bd9d3f3b8a4a85e1"
 
+[[package]]
+name = "dunce"
+version = "1.0.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813"
+
 [[package]]
 name = "either"
 version = "1.13.0"
diff --git a/MODULE.bazel b/MODULE.bazel
index 13c801520b04..ca09ada47001 100644
--- a/MODULE.bazel
+++ b/MODULE.bazel
@@ -68,7 +68,7 @@ use_repo(py_deps, "vendor__anyhow-1.0.44", "vendor__cc-1.0.70", "vendor__clap-2.
 # deps for ruby+rust
 # keep in sync by running `misc/bazel/3rdparty/update_cargo_deps.sh`
 tree_sitter_extractors_deps = use_extension("//misc/bazel/3rdparty:tree_sitter_extractors_extension.bzl", "r")
-use_repo(tree_sitter_extractors_deps, "vendor__anyhow-1.0.93", "vendor__argfile-0.2.1", "vendor__chrono-0.4.38", "vendor__clap-4.5.20", "vendor__encoding-0.2.33", "vendor__figment-0.10.19", "vendor__flate2-1.0.34", "vendor__glob-0.3.1", "vendor__globset-0.4.15", "vendor__itertools-0.10.5", "vendor__itertools-0.13.0", "vendor__lazy_static-1.5.0", "vendor__log-0.4.22", "vendor__num-traits-0.2.19", "vendor__num_cpus-1.16.0", "vendor__proc-macro2-1.0.89", "vendor__quote-1.0.37", "vendor__ra_ap_base_db-0.0.232", "vendor__ra_ap_cfg-0.0.232", "vendor__ra_ap_hir-0.0.232", "vendor__ra_ap_hir_def-0.0.232", "vendor__ra_ap_hir_expand-0.0.232", "vendor__ra_ap_ide_db-0.0.232", "vendor__ra_ap_intern-0.0.232", "vendor__ra_ap_load-cargo-0.0.232", "vendor__ra_ap_parser-0.0.232", "vendor__ra_ap_paths-0.0.232", "vendor__ra_ap_project_model-0.0.232", "vendor__ra_ap_span-0.0.232", "vendor__ra_ap_syntax-0.0.232", "vendor__ra_ap_vfs-0.0.232", "vendor__rand-0.8.5", "vendor__rayon-1.10.0", "vendor__regex-1.11.1", "vendor__serde-1.0.214", "vendor__serde_json-1.0.132", "vendor__serde_with-3.11.0", "vendor__stderrlog-0.6.0", "vendor__syn-2.0.87", "vendor__tracing-0.1.40", "vendor__tracing-subscriber-0.3.18", "vendor__tree-sitter-0.24.4", "vendor__tree-sitter-embedded-template-0.23.2", "vendor__tree-sitter-json-0.24.8", "vendor__tree-sitter-ql-0.23.1", "vendor__tree-sitter-ruby-0.23.1", "vendor__triomphe-0.1.14", "vendor__ungrammar-1.16.1")
+use_repo(tree_sitter_extractors_deps, "vendor__anyhow-1.0.93", "vendor__argfile-0.2.1", "vendor__chrono-0.4.38", "vendor__clap-4.5.20", "vendor__dunce-1.0.5", "vendor__encoding-0.2.33", "vendor__figment-0.10.19", "vendor__flate2-1.0.34", "vendor__glob-0.3.1", "vendor__globset-0.4.15", "vendor__itertools-0.10.5", "vendor__itertools-0.13.0", "vendor__lazy_static-1.5.0", "vendor__log-0.4.22", "vendor__num-traits-0.2.19", "vendor__num_cpus-1.16.0", "vendor__proc-macro2-1.0.89", "vendor__quote-1.0.37", "vendor__ra_ap_base_db-0.0.232", "vendor__ra_ap_cfg-0.0.232", "vendor__ra_ap_hir-0.0.232", "vendor__ra_ap_hir_def-0.0.232", "vendor__ra_ap_hir_expand-0.0.232", "vendor__ra_ap_ide_db-0.0.232", "vendor__ra_ap_intern-0.0.232", "vendor__ra_ap_load-cargo-0.0.232", "vendor__ra_ap_parser-0.0.232", "vendor__ra_ap_paths-0.0.232", "vendor__ra_ap_project_model-0.0.232", "vendor__ra_ap_span-0.0.232", "vendor__ra_ap_syntax-0.0.232", "vendor__ra_ap_vfs-0.0.232", "vendor__rand-0.8.5", "vendor__rayon-1.10.0", "vendor__regex-1.11.1", "vendor__serde-1.0.214", "vendor__serde_json-1.0.132", "vendor__serde_with-3.11.0", "vendor__stderrlog-0.6.0", "vendor__syn-2.0.87", "vendor__tracing-0.1.40", "vendor__tracing-subscriber-0.3.18", "vendor__tree-sitter-0.24.4", "vendor__tree-sitter-embedded-template-0.23.2", "vendor__tree-sitter-json-0.24.8", "vendor__tree-sitter-ql-0.23.1", "vendor__tree-sitter-ruby-0.23.1", "vendor__triomphe-0.1.14", "vendor__ungrammar-1.16.1")
 
 dotnet = use_extension("@rules_dotnet//dotnet:extensions.bzl", "dotnet")
 dotnet.toolchain(dotnet_version = "9.0.100")
diff --git a/misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.bazel b/misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.bazel
index 844d385f8a41..8eb128bd79d6 100644
--- a/misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.bazel
+++ b/misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.bazel
@@ -55,6 +55,12 @@ alias(
     tags = ["manual"],
 )
 
+alias(
+    name = "dunce",
+    actual = "@vendor__dunce-1.0.5//:dunce",
+    tags = ["manual"],
+)
+
 alias(
     name = "encoding",
     actual = "@vendor__encoding-0.2.33//:encoding",
diff --git a/misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.dunce-1.0.5.bazel b/misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.dunce-1.0.5.bazel
new file mode 100644
index 000000000000..c5341a0cb673
--- /dev/null
+++ b/misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.dunce-1.0.5.bazel
@@ -0,0 +1,81 @@
+###############################################################################
+# @generated
+# DO NOT MODIFY: This file is auto-generated by a crate_universe tool. To
+# regenerate this file, run the following:
+#
+#     bazel run @@//misc/bazel/3rdparty:vendor_tree_sitter_extractors
+###############################################################################
+
+load("@rules_rust//rust:defs.bzl", "rust_library")
+
+package(default_visibility = ["//visibility:public"])
+
+rust_library(
+    name = "dunce",
+    srcs = glob(
+        include = ["**/*.rs"],
+        allow_empty = True,
+    ),
+    compile_data = glob(
+        include = ["**"],
+        allow_empty = True,
+        exclude = [
+            "**/* *",
+            ".tmp_git_root/**/*",
+            "BUILD",
+            "BUILD.bazel",
+            "WORKSPACE",
+            "WORKSPACE.bazel",
+        ],
+    ),
+    crate_root = "src/lib.rs",
+    edition = "2021",
+    rustc_flags = [
+        "--cap-lints=allow",
+    ],
+    tags = [
+        "cargo-bazel",
+        "crate-name=dunce",
+        "manual",
+        "noclippy",
+        "norustfmt",
+    ],
+    target_compatible_with = select({
+        "@rules_rust//rust/platform:aarch64-apple-darwin": [],
+        "@rules_rust//rust/platform:aarch64-apple-ios": [],
+        "@rules_rust//rust/platform:aarch64-apple-ios-sim": [],
+        "@rules_rust//rust/platform:aarch64-fuchsia": [],
+        "@rules_rust//rust/platform:aarch64-linux-android": [],
+        "@rules_rust//rust/platform:aarch64-pc-windows-msvc": [],
+        "@rules_rust//rust/platform:aarch64-unknown-linux-gnu": [],
+        "@rules_rust//rust/platform:aarch64-unknown-nixos-gnu": [],
+        "@rules_rust//rust/platform:aarch64-unknown-nto-qnx710": [],
+        "@rules_rust//rust/platform:arm-unknown-linux-gnueabi": [],
+        "@rules_rust//rust/platform:armv7-linux-androideabi": [],
+        "@rules_rust//rust/platform:armv7-unknown-linux-gnueabi": [],
+        "@rules_rust//rust/platform:i686-apple-darwin": [],
+        "@rules_rust//rust/platform:i686-linux-android": [],
+        "@rules_rust//rust/platform:i686-pc-windows-msvc": [],
+        "@rules_rust//rust/platform:i686-unknown-freebsd": [],
+        "@rules_rust//rust/platform:i686-unknown-linux-gnu": [],
+        "@rules_rust//rust/platform:powerpc-unknown-linux-gnu": [],
+        "@rules_rust//rust/platform:riscv32imc-unknown-none-elf": [],
+        "@rules_rust//rust/platform:riscv64gc-unknown-none-elf": [],
+        "@rules_rust//rust/platform:s390x-unknown-linux-gnu": [],
+        "@rules_rust//rust/platform:thumbv7em-none-eabi": [],
+        "@rules_rust//rust/platform:thumbv8m.main-none-eabi": [],
+        "@rules_rust//rust/platform:wasm32-unknown-unknown": [],
+        "@rules_rust//rust/platform:wasm32-wasi": [],
+        "@rules_rust//rust/platform:x86_64-apple-darwin": [],
+        "@rules_rust//rust/platform:x86_64-apple-ios": [],
+        "@rules_rust//rust/platform:x86_64-fuchsia": [],
+        "@rules_rust//rust/platform:x86_64-linux-android": [],
+        "@rules_rust//rust/platform:x86_64-pc-windows-msvc": [],
+        "@rules_rust//rust/platform:x86_64-unknown-freebsd": [],
+        "@rules_rust//rust/platform:x86_64-unknown-linux-gnu": [],
+        "@rules_rust//rust/platform:x86_64-unknown-nixos-gnu": [],
+        "@rules_rust//rust/platform:x86_64-unknown-none": [],
+        "//conditions:default": ["@platforms//:incompatible"],
+    }),
+    version = "1.0.5",
+)
diff --git a/misc/bazel/3rdparty/tree_sitter_extractors_deps/defs.bzl b/misc/bazel/3rdparty/tree_sitter_extractors_deps/defs.bzl
index 1d0b825c2356..e8ea45d091fe 100644
--- a/misc/bazel/3rdparty/tree_sitter_extractors_deps/defs.bzl
+++ b/misc/bazel/3rdparty/tree_sitter_extractors_deps/defs.bzl
@@ -321,6 +321,7 @@ _NORMAL_DEPENDENCIES = {
             "anyhow": Label("@vendor__anyhow-1.0.93//:anyhow"),
             "argfile": Label("@vendor__argfile-0.2.1//:argfile"),
             "clap": Label("@vendor__clap-4.5.20//:clap"),
+            "dunce": Label("@vendor__dunce-1.0.5//:dunce"),
             "figment": Label("@vendor__figment-0.10.19//:figment"),
             "glob": Label("@vendor__glob-0.3.1//:glob"),
             "itertools": Label("@vendor__itertools-0.13.0//:itertools"),
@@ -1117,6 +1118,16 @@ def crate_repositories():
         build_file = Label("//misc/bazel/3rdparty/tree_sitter_extractors_deps:BUILD.drop_bomb-0.1.5.bazel"),
     )
 
+    maybe(
+        http_archive,
+        name = "vendor__dunce-1.0.5",
+        sha256 = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813",
+        type = "tar.gz",
+        urls = ["https://static.crates.io/crates/dunce/1.0.5/download"],
+        strip_prefix = "dunce-1.0.5",
+        build_file = Label("//misc/bazel/3rdparty/tree_sitter_extractors_deps:BUILD.dunce-1.0.5.bazel"),
+    )
+
     maybe(
         http_archive,
         name = "vendor__either-1.13.0",
@@ -3311,6 +3322,7 @@ def crate_repositories():
         struct(repo = "vendor__argfile-0.2.1", is_dev_dep = False),
         struct(repo = "vendor__chrono-0.4.38", is_dev_dep = False),
         struct(repo = "vendor__clap-4.5.20", is_dev_dep = False),
+        struct(repo = "vendor__dunce-1.0.5", is_dev_dep = False),
         struct(repo = "vendor__encoding-0.2.33", is_dev_dep = False),
         struct(repo = "vendor__figment-0.10.19", is_dev_dep = False),
         struct(repo = "vendor__flate2-1.0.34", is_dev_dep = False),
diff --git a/rust/extractor/Cargo.toml b/rust/extractor/Cargo.toml
index 8b58898d3cf3..971a6f5e74ef 100644
--- a/rust/extractor/Cargo.toml
+++ b/rust/extractor/Cargo.toml
@@ -33,3 +33,4 @@ codeql-extractor = { path = "../../shared/tree-sitter-extractor" }
 rust-extractor-macros = { path = "macros" }
 itertools = "0.13.0"
 glob = "0.3.1"
+dunce = "1.0.5"
diff --git a/rust/extractor/src/rust_analyzer.rs b/rust/extractor/src/rust_analyzer.rs
index 735bacb27c12..468f40a6004d 100644
--- a/rust/extractor/src/rust_analyzer.rs
+++ b/rust/extractor/src/rust_analyzer.rs
@@ -17,6 +17,7 @@ use ra_ap_vfs::Vfs;
 use ra_ap_vfs::VfsPath;
 use ra_ap_vfs::{AbsPathBuf, FileId};
 use std::borrow::Cow;
+use std::iter;
 use std::path::{Path, PathBuf};
 use triomphe::Arc;
 
@@ -188,10 +189,25 @@ fn from_utf8_lossy(v: &[u8]) -> (Cow<'_, str>, Option<SyntaxError>) {
     (Cow::Owned(res), Some(error))
 }
 
+fn canonicalize_if_on_windows(path: &Path) -> Option<PathBuf> {
+    if cfg!(windows) {
+        dunce::canonicalize(path).ok()
+    } else {
+        None
+    }
+}
+
 pub(crate) fn path_to_file_id(path: &Path, vfs: &Vfs) -> Option<FileId> {
-    Utf8PathBuf::from_path_buf(path.to_path_buf())
-        .ok()
-        .and_then(|x| AbsPathBuf::try_from(x).ok())
-        .map(VfsPath::from)
-        .and_then(|x| vfs.file_id(&x))
+    // There seems to be some flaky inconsistencies around UNC paths on Windows, so if we fail to
+    // find the file id for a UNC path like that, we try to canonicalize it using dunce then.
+    iter::once(path.to_path_buf())
+        .chain(canonicalize_if_on_windows(path))
+        .filter_map(|p| {
+            Utf8PathBuf::from_path_buf(p)
+                .ok()
+                .and_then(|x| AbsPathBuf::try_from(x).ok())
+                .map(VfsPath::from)
+                .and_then(|x| vfs.file_id(&x))
+        })
+        .next()
 }

From 7c1aa844594211ae00c934feffb1f77883cd5d40 Mon Sep 17 00:00:00 2001
From: Napalys <napalys@github.com>
Date: Fri, 29 Nov 2024 17:24:58 +0100
Subject: [PATCH 43/51] Fixed bug where some predicates were flagged without
 return type even thought they had

---
 ql/ql/src/queries/style/ValidatePredicateGetReturns.ql       | 2 +-
 .../test/queries/style/ValidatePredicateGetReturns/test.qll  | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
index ad7db6e05907..f65b883a09b0 100644
--- a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
+++ b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
@@ -21,7 +21,7 @@ predicate isGetPredicate(Predicate pred) { pred.getName().regexpMatch("(get|as)[
 /**
  * Checks if a predicate has a return type.
  */
-predicate hasReturnType(Predicate pred) { exists(pred.getReturnType()) }
+predicate hasReturnType(Predicate pred) { exists(pred.getReturnTypeExpr()) }
 
 /**
  * Checks if a predicate is an alias using getAlias().
diff --git a/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll b/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
index b18649ea8cc7..b68aab08fd78 100644
--- a/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
+++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
@@ -35,3 +35,8 @@ predicate assessment() { none() }
 
 // OK -- starts with as and returns a value
 string asString() { result = "string" }
+
+// OK -- starts with get and returns a value
+HiddenType getInjectableCompositeActionNode() {
+  exists(HiddenType hidden | result = hidden.toString())
+}

From 6cb0866d0fc53e4bb16c52c2c2684be40cbe71ed Mon Sep 17 00:00:00 2001
From: Paolo Tranquilli <redsun82@gihub.com>
Date: Fri, 29 Nov 2024 17:41:49 +0100
Subject: [PATCH 44/51] Revert "Rust: test running windows flaky test multiple
 times"

This reverts commit 90fa3ec4ed358188c1bbd1aee2d9b5ca503ed314.
---
 .../integration-tests/hello-workspace/test_workspace.py  | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/rust/ql/integration-tests/hello-workspace/test_workspace.py b/rust/ql/integration-tests/hello-workspace/test_workspace.py
index 99c67f6f6247..6a7e21ebedbc 100644
--- a/rust/ql/integration-tests/hello-workspace/test_workspace.py
+++ b/rust/ql/integration-tests/hello-workspace/test_workspace.py
@@ -1,14 +1,5 @@
-import runs_on
-import pytest
-
-@runs_on.posix
 def test_cargo(codeql, rust, cargo, check_source_archive):
     codeql.database.create()
 
-@runs_on.windows
-@pytest.mark.parametrize("_", range(25))
-def test_cargo_debug(codeql, rust, cargo, check_source_archive, _):
-    codeql.database.create()
-
 def test_rust_project(codeql, rust, rust_project, check_source_archive):
     codeql.database.create()

From f87024c6202a9730e7c3968db24e6087cfa6ac9d Mon Sep 17 00:00:00 2001
From: "github-actions[bot]"
 <41898282+github-actions[bot]@users.noreply.github.com>
Date: Mon, 2 Dec 2024 00:23:11 +0000
Subject: [PATCH 45/51] Add changed framework coverage reports

---
 csharp/documentation/library-coverage/coverage.csv | 4 +++-
 csharp/documentation/library-coverage/coverage.rst | 6 +++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/csharp/documentation/library-coverage/coverage.csv b/csharp/documentation/library-coverage/coverage.csv
index 1c3562e6f885..39646702a8c0 100644
--- a/csharp/documentation/library-coverage/coverage.csv
+++ b/csharp/documentation/library-coverage/coverage.csv
@@ -13,6 +13,8 @@ JsonToItemsTaskFactory,,,11,,,,,,,,,,,,,,,,,,,1,10
 Microsoft.Android.Build,,1,14,,,,,,,,,,,,,1,,,,,,12,2
 Microsoft.Apple.Build,,,7,,,,,,,,,,,,,,,,,,,7,
 Microsoft.ApplicationBlocks.Data,28,,,,,,,,,,,,28,,,,,,,,,,
+Microsoft.AspNetCore.Components,,2,1,,,,,,,,,,,,,,,,2,,,1,
+Microsoft.AspNetCore.WebUtilities,,,2,,,,,,,,,,,,,,,,,,,2,
 Microsoft.CSharp,,,2,,,,,,,,,,,,,,,,,,,2,
 Microsoft.Diagnostics.Tools.Pgo,,,25,,,,,,,,,,,,,,,,,,,2,23
 Microsoft.DotNet.Build.Tasks,,,10,,,,,,,,,,,,,,,,,,,8,2
@@ -44,5 +46,5 @@ MySql.Data.MySqlClient,48,,,,,,,,,,,,48,,,,,,,,,,
 Newtonsoft.Json,,,91,,,,,,,,,,,,,,,,,,,73,18
 ServiceStack,194,,7,27,,,,,75,,,,92,,,,,,,,,7,
 SourceGenerators,,,5,,,,,,,,,,,,,,,,,,,,5
-System,54,47,10818,,6,5,5,,,4,1,,33,2,,6,15,17,4,3,,5511,5307
+System,54,47,10819,,6,5,5,,,4,1,,33,2,,6,15,17,4,3,,5512,5307
 Windows.Security.Cryptography.Core,1,,,,,,,1,,,,,,,,,,,,,,,
diff --git a/csharp/documentation/library-coverage/coverage.rst b/csharp/documentation/library-coverage/coverage.rst
index e913d5edc284..7023de4a356b 100644
--- a/csharp/documentation/library-coverage/coverage.rst
+++ b/csharp/documentation/library-coverage/coverage.rst
@@ -8,7 +8,7 @@ C# framework & library support
 
    Framework / library,Package,Flow sources,Taint & value steps,Sinks (total),`CWE-079` :sub:`Cross-site scripting`
    `ServiceStack <https://servicestack.net/>`_,"``ServiceStack.*``, ``ServiceStack``",,7,194,
-   System,"``System.*``, ``System``",47,10818,54,5
-   Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``JsonToItemsTaskFactory``, ``Microsoft.Android.Build``, ``Microsoft.Apple.Build``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.CSharp``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.DotNet.Build.Tasks``, ``Microsoft.DotNet.PlatformAbstractions``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.JSInterop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.NET.Sdk.WebAssembly``, ``Microsoft.NET.WebAssembly.Webcil``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",57,2068,150,2
-   Totals,,104,12893,398,7
+   System,"``System.*``, ``System``",47,10819,54,5
+   Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``JsonToItemsTaskFactory``, ``Microsoft.Android.Build``, ``Microsoft.Apple.Build``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.AspNetCore.Components``, ``Microsoft.AspNetCore.WebUtilities``, ``Microsoft.CSharp``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.DotNet.Build.Tasks``, ``Microsoft.DotNet.PlatformAbstractions``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.JSInterop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.NET.Sdk.WebAssembly``, ``Microsoft.NET.WebAssembly.Webcil``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",59,2071,150,2
+   Totals,,106,12897,398,7
 

From 9486e8b7345de96dc30af8a2851bf6643aca7baf Mon Sep 17 00:00:00 2001
From: Paolo Tranquilli <redsun82@gihub.com>
Date: Mon, 2 Dec 2024 08:12:57 +0100
Subject: [PATCH 46/51] Rust: elaborate on `path_to_file_id` comment

This is a follow up to https://github.com/github/codeql/pull/18167, addressing a
review comment from @paldepind.
---
 rust/extractor/src/rust_analyzer.rs | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/rust/extractor/src/rust_analyzer.rs b/rust/extractor/src/rust_analyzer.rs
index 468f40a6004d..a896211333dc 100644
--- a/rust/extractor/src/rust_analyzer.rs
+++ b/rust/extractor/src/rust_analyzer.rs
@@ -198,8 +198,11 @@ fn canonicalize_if_on_windows(path: &Path) -> Option<PathBuf> {
 }
 
 pub(crate) fn path_to_file_id(path: &Path, vfs: &Vfs) -> Option<FileId> {
-    // There seems to be some flaky inconsistencies around UNC paths on Windows, so if we fail to
-    // find the file id for a UNC path like that, we try to canonicalize it using dunce then.
+    // There seems to be some flaky inconsistencies around paths on Windows, where sometimes paths
+    // are registered in `vfs` without the `//?/` long path prefix. Then it happens that paths with
+    // that prefix are not found. To work around that, on Windows after failing to find `path` as
+    // is, we then try to canonicalize it using dunce. Dunce will be able to losslessly convert a
+    // `//?/` path into its equivalent one in `vfs` without the prefix, if there is one.
     iter::once(path.to_path_buf())
         .chain(canonicalize_if_on_windows(path))
         .filter_map(|p| {

From 67745e6332564b343537ab63e08330a056eeed4f Mon Sep 17 00:00:00 2001
From: Napalys <napalys@github.com>
Date: Mon, 2 Dec 2024 09:10:54 +0100
Subject: [PATCH 47/51] Reused isGetPredicate to retrieve the prefix of the
 predicate

---
 .../style/ValidatePredicateGetReturns.ql      | 22 +++++--------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
index f65b883a09b0..4d0196509c33 100644
--- a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
+++ b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
@@ -16,7 +16,9 @@ import codeql_ql.ast.Ast
  * Identifies predicates whose names start with "get", "as" followed by an uppercase letter.
  * This ensures that only predicates like "getValue" are matched, excluding names like "getter".
  */
-predicate isGetPredicate(Predicate pred) { pred.getName().regexpMatch("(get|as)[A-Z].*") }
+predicate isGetPredicate(Predicate pred, string prefix) {
+  prefix = pred.getName().regexpCapture("(get|as)[A-Z].*", 1)
+}
 
 /**
  * Checks if a predicate has a return type.
@@ -28,21 +30,9 @@ predicate hasReturnType(Predicate pred) { exists(pred.getReturnTypeExpr()) }
  */
 predicate isAlias(Predicate pred) { exists(pred.(ClasslessPredicate).getAlias()) }
 
-/**
- * Returns "get" if the predicate name starts with "get", otherwise "as".
- */
-string getPrefix(Predicate pred) {
-  if pred.getName().matches("get%")
-  then result = "get"
-  else
-    if pred.getName().matches("as%")
-    then result = "as"
-    else result = ""
-}
-
-from Predicate pred
+from Predicate pred, string prefix
 where
-  isGetPredicate(pred) and
+  isGetPredicate(pred, prefix) and
   not hasReturnType(pred) and
   not isAlias(pred)
-select pred, "This predicate starts with '" + getPrefix(pred) + "' but does not return a value."
+select pred, "This predicate starts with '" + prefix + "' but does not return a value."

From b0d3c118854556f1b47fc135cde5285898207cce Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Mon, 2 Dec 2024 11:03:53 +0100
Subject: [PATCH 48/51] Add a new test case

---
 .../Telemetry/DatabaseQuality/IsNotOkayCall.expected       | 1 +
 .../Telemetry/DatabaseQuality/NoTarget.expected            | 5 +++--
 .../test/query-tests/Telemetry/DatabaseQuality/Quality.cs  | 7 +++++++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected
index 7555a37394b5..205022b71802 100644
--- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected
+++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected
@@ -1,3 +1,4 @@
 | Quality.cs:26:19:26:26 | access to indexer | Call without target $@. | Quality.cs:26:19:26:26 | access to indexer | access to indexer |
 | Quality.cs:29:21:29:27 | access to indexer | Call without target $@. | Quality.cs:29:21:29:27 | access to indexer | access to indexer |
 | Quality.cs:32:9:32:21 | access to indexer | Call without target $@. | Quality.cs:32:9:32:21 | access to indexer | access to indexer |
+| Quality.cs:34:21:34:25 | object creation of type null | Call without target $@. | Quality.cs:34:21:34:25 | object creation of type null | object creation of type null |
diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected
index fae81ab0c6d4..84b6994e033a 100644
--- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected
+++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected
@@ -10,5 +10,6 @@
 | Quality.cs:26:19:26:26 | access to indexer | Call without target $@. | Quality.cs:26:19:26:26 | access to indexer | access to indexer |
 | Quality.cs:29:21:29:27 | access to indexer | Call without target $@. | Quality.cs:29:21:29:27 | access to indexer | access to indexer |
 | Quality.cs:32:9:32:21 | access to indexer | Call without target $@. | Quality.cs:32:9:32:21 | access to indexer | access to indexer |
-| Quality.cs:36:16:36:26 | access to property MyProperty2 | Call without target $@. | Quality.cs:36:16:36:26 | access to property MyProperty2 | access to property MyProperty2 |
-| Quality.cs:48:20:48:26 | object creation of type T | Call without target $@. | Quality.cs:48:20:48:26 | object creation of type T | object creation of type T |
+| Quality.cs:34:21:34:25 | object creation of type null | Call without target $@. | Quality.cs:34:21:34:25 | object creation of type null | object creation of type null |
+| Quality.cs:38:16:38:26 | access to property MyProperty2 | Call without target $@. | Quality.cs:38:16:38:26 | access to property MyProperty2 | access to property MyProperty2 |
+| Quality.cs:50:20:50:26 | object creation of type T | Call without target $@. | Quality.cs:50:20:50:26 | object creation of type T | object creation of type T |
diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs
index 2c26c0041312..648083edad8d 100644
--- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs
+++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs
@@ -30,6 +30,8 @@ public Test()
 
         Span<byte> guidBytes = stackalloc byte[16];
         guidBytes[08] = 1;
+
+        new MyList([new(), new Test()]);
     }
 
     public int MyProperty1 { get; }
@@ -48,4 +50,9 @@ public static T Factory()
             return new T();
         }
     }
+
+    class MyList
+    {
+        public MyList(IEnumerable<Test> init) { }
+    }
 }

From 7f9adbd371296abe7ce232b94745dd01da4c0e0d Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Mon, 2 Dec 2024 11:44:17 +0100
Subject: [PATCH 49/51] Address review comments

---
 .../codeql/dataflow/internal/DataFlowImpl.qll | 20 +++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll
index c67d21fb1ac1..96184a607f0b 100644
--- a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll
+++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll
@@ -459,7 +459,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
     private predicate notExpectsContent(NodeEx n) { not expectsContentSet(n, _) }
 
     pragma[nomagic]
-    private predicate storeExUnrestricted(
+    private predicate storeUnrestricted(
       NodeEx node1, Content c, NodeEx node2, DataFlowType contentType, DataFlowType containerType
     ) {
       storeEx(node1, c, node2, contentType, containerType) and
@@ -470,10 +470,10 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
     private predicate hasReadStep(Content c) { read(_, c, _) }
 
     pragma[nomagic]
-    private predicate storeExRestricted(
+    private predicate store(
       NodeEx node1, Content c, NodeEx node2, DataFlowType contentType, DataFlowType containerType
     ) {
-      storeExUnrestricted(node1, c, node2, contentType, containerType) and
+      storeUnrestricted(node1, c, node2, contentType, containerType) and
       hasReadStep(c)
     }
 
@@ -547,7 +547,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         exists(NodeEx mid |
           useFieldFlow() and
           fwdFlow(mid, cc) and
-          storeExRestricted(mid, _, node, _, _)
+          store(mid, _, node, _, _)
         )
         or
         // read
@@ -642,7 +642,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
           not fullBarrier(node) and
           useFieldFlow() and
           fwdFlow(mid, _) and
-          storeExRestricted(mid, c, node, _, _)
+          store(mid, c, node, _, _)
         )
       }
 
@@ -785,7 +785,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         exists(NodeEx mid |
           revFlow(mid, toReturn) and
           fwdFlowConsCand(c) and
-          storeExRestricted(node, c, mid, _, _)
+          store(node, c, mid, _, _)
         )
       }
 
@@ -882,7 +882,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       ) {
         revFlowIsReadAndStored(c) and
         revFlow(node2) and
-        storeExRestricted(node1, c, node2, contentType, containerType) and
+        store(node1, c, node2, contentType, containerType) and
         exists(ap1)
       }
 
@@ -5352,7 +5352,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
           midNode = mid.getNodeEx() and
           t1 = mid.getType() and
           ap1 = mid.getAp() and
-          storeExUnrestricted(midNode, c, node, contentType, t2) and
+          storeUnrestricted(midNode, c, node, contentType, t2) and
           ap2.getHead() = c and
           ap2.len() = unbindInt(ap1.len() + 1) and
           compatibleTypesFilter(t1, contentType)
@@ -5523,7 +5523,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
           not outBarrier(node, state) and
           // if a node is not the target of a store, we can check `clearsContent` immediately
           (
-            storeExUnrestricted(_, _, node, _, _)
+            storeUnrestricted(_, _, node, _, _)
             or
             not clearsContentEx(node, ap.getHead())
           )
@@ -5664,7 +5664,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         exists(NodeEx midNode |
           midNode = mid.getNodeEx() and
           ap = mid.getAp() and
-          storeExUnrestricted(node, c, midNode, _, _) and
+          storeUnrestricted(node, c, midNode, _, _) and
           ap.getHead() = c
         )
       }

From 7db9b7d758859042797625363fc6c0a2fef9b2c1 Mon Sep 17 00:00:00 2001
From: Napalys <napalys@github.com>
Date: Mon, 2 Dec 2024 12:43:52 +0100
Subject: [PATCH 50/51] Now flag aliases with the 'get' or 'as' prefix that
 resolve to predicates lacking a return type.

Co-authored-by: asgerf <asgerf@github.com>
---
 .../style/ValidatePredicateGetReturns.ql      | 17 ++++++------
 .../ValidatePredicateGetReturns.expected      |  3 +++
 .../ValidatePredicateGetReturns/test.qll      | 27 ++++++++++++++++++-
 3 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
index 4d0196509c33..a79e4f69569e 100644
--- a/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
+++ b/ql/ql/src/queries/style/ValidatePredicateGetReturns.ql
@@ -21,18 +21,17 @@ predicate isGetPredicate(Predicate pred, string prefix) {
 }
 
 /**
- * Checks if a predicate has a return type.
+ * Checks if a predicate has a return type. This is phrased negatively to not flag unresolved aliases.
  */
-predicate hasReturnType(Predicate pred) { exists(pred.getReturnTypeExpr()) }
-
-/**
- * Checks if a predicate is an alias using getAlias().
- */
-predicate isAlias(Predicate pred) { exists(pred.(ClasslessPredicate).getAlias()) }
+predicate hasNoReturnType(Predicate pred) {
+  not exists(pred.getReturnTypeExpr()) and
+  not pred.(ClasslessPredicate).getAlias() instanceof PredicateExpr
+  or
+  hasNoReturnType(pred.(ClasslessPredicate).getAlias().(PredicateExpr).getResolvedPredicate())
+}
 
 from Predicate pred, string prefix
 where
   isGetPredicate(pred, prefix) and
-  not hasReturnType(pred) and
-  not isAlias(pred)
+  hasNoReturnType(pred)
 select pred, "This predicate starts with '" + prefix + "' but does not return a value."
diff --git a/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
index 57469246ae82..4b36d2c5c328 100644
--- a/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
+++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected
@@ -1,3 +1,6 @@
 | test.qll:4:11:4:18 | ClasslessPredicate getValue | This predicate starts with 'get' but does not return a value. |
 | test.qll:25:11:25:28 | ClasslessPredicate getImplementation2 | This predicate starts with 'get' but does not return a value. |
+| test.qll:28:11:28:19 | ClasslessPredicate getAlias2 | This predicate starts with 'get' but does not return a value. |
 | test.qll:31:11:31:17 | ClasslessPredicate asValue | This predicate starts with 'as' but does not return a value. |
+| test.qll:48:11:48:19 | ClasslessPredicate getAlias4 | This predicate starts with 'get' but does not return a value. |
+| test.qll:61:11:61:22 | ClasslessPredicate getDistance2 | This predicate starts with 'get' but does not return a value. |
diff --git a/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll b/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
index b68aab08fd78..2cc4dec64d20 100644
--- a/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
+++ b/ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll
@@ -24,7 +24,7 @@ predicate retrieveValue() { none() }
 // NOT OK -- starts with get and does not return value
 predicate getImplementation2() { none() }
 
-// OK -- is an alias
+// NOT OK -- is an alias for a predicate which does not have a return value
 predicate getAlias2 = getImplementation2/0;
 
 // NOT OK -- starts with as and does not return value
@@ -40,3 +40,28 @@ string asString() { result = "string" }
 HiddenType getInjectableCompositeActionNode() {
   exists(HiddenType hidden | result = hidden.toString())
 }
+
+// OK
+predicate implementation4() { none() }
+
+// NOT OK -- is an alias
+predicate getAlias4 = implementation4/0;
+
+// OK -- is an alias
+predicate alias5 = implementation4/0;
+
+int root() { none() }
+
+predicate edge(int x, int y) { none() }
+
+// OK -- Higher-order predicate
+int getDistance(int x) = shortestDistances(root/0, edge/2)(_, x, result)
+
+// NOT OK -- Higher-order predicate that does not return a value even though has 'get' in the name
+predicate getDistance2(int x, int y) = shortestDistances(root/0, edge/2)(_, x, y)
+
+// OK
+predicate unresolvedAlias = unresolved/0;
+
+// OK -- unresolved alias
+predicate getUnresolvedAlias = unresolved/0;

From b8fd20eb05c069b78cd3d4c5d8dfd61caededa34 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Mon, 2 Dec 2024 13:52:46 +0100
Subject: [PATCH 51/51] Add explanation todo comments in the missing call
 target test file

---
 .../test/query-tests/Telemetry/DatabaseQuality/Quality.cs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs
index 648083edad8d..c3ec759d6879 100644
--- a/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs
+++ b/csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs
@@ -23,15 +23,15 @@ public Test()
         Event1.Invoke(this, 5);
 
         var str = "abcd";
-        var sub = str[..3];
+        var sub = str[..3];         // TODO: this is not an indexer call, but rather a `str.Substring(0, 3)` call.
 
         Span<int> sp = null;
-        var slice = sp[..3];
+        var slice = sp[..3];        // TODO: this is not an indexer call, but rather a `sp.Slice(0, 3)` call.
 
         Span<byte> guidBytes = stackalloc byte[16];
-        guidBytes[08] = 1;
+        guidBytes[08] = 1;          // TODO: this indexer call has no target, because the target is a `ref` returning getter.
 
-        new MyList([new(), new Test()]);
+        new MyList([new(), new Test()]);        // TODO: the `new()` call has no target, which is unexpected, as we know at compile time, that this is a `new Test()` call.
     }
 
     public int MyProperty1 { get; }