Merge pull request github#18097 from geoffw0/ctor

Rust: New query for bad 'ctor' initialization

Author: geoffw0

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 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>
+<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 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 used 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>

rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql

@@ -0,0 +1,58 @@
+/**
+ * @name Bad 'ctor' initialization
+ * @description Calling functions in the Rust std library from a ctor or dtor function is not safe.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id rust/ctor-initialization
+ * @tags reliability
+ *       correctness
+ *       external/cwe/cwe-696
+ *       external/cwe/cwe-665
+ */
+
+import rust
+
+/**
+ * A `#[ctor]` or `#[dtor]` attribute.
+ */
+class CtorAttr extends Attr {
+  string whichAttr;
+
+  CtorAttr() {
+    whichAttr = this.getMeta().getPath().getPart().getNameRef().getText() and
+    whichAttr = ["ctor", "dtor"]
+  }
+
+  string getWhichAttr() { result = whichAttr }
+}
+
+/**
+ * A call into the Rust standard library.
+ */
+class StdCall extends Expr {
+  StdCall() {
+    this.(CallExpr).getFunction().(PathExpr).getPath().getResolvedCrateOrigin() = "lang:std" or
+    this.(MethodCallExpr).getResolvedCrateOrigin() = "lang:std"
+  }
+}
+
+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 " + call.toString() + " in a function with the " + ctor.getWhichAttr() + " attribute."

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
+}

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
+}

rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected

@@ -0,0 +1,22 @@
+#select
+| 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(...) |
+| 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(...) |

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

rust/ql/test/query-tests/security/CWE-696/options.yml

@@ -0,0 +1,4 @@
+qltest_cargo_check: true
+qltest_dependencies:
+    - ctor = { version = "0.2.9" }
+    - libc-print = { version = "0.1.23" }

rust/ql/test/query-tests/security/CWE-696/test.rs

@@ -0,0 +1,167 @@
+
+// --- 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] // $ Source=source1_1
+fn bad1_1() {
+    _ = std::io::stdout().write(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source1_1
+}
+
+#[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] // $ Source=source1_3
+#[rustfmt::skip]
+fn bad1_3() {
+    _ = std::io::stdout().write(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source1_3
+}
+
+// --- code variants ---
+
+use ctor::ctor;
+use std::io::*;
+
+#[ctor] // $ Source=source2_1
+fn bad2_1() {
+    _ = stdout().write(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source2_1
+}
+
+#[ctor] // $ Source=source2_2
+fn bad2_2() {
+    _ = stderr().write_all(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source2_2
+}
+
+#[ctor] // $ Source=source2_3
+fn bad2_3() {
+    println!("Hello, world!"); // $ Alert[rust/ctor-initialization]=source2_3
+}
+
+#[ctor] // $ Source=source2_4
+fn bad2_4() {
+    let mut buff = String::new();
+    _ = std::io::stdin().read_line(&mut buff); // $ Alert[rust/ctor-initialization]=source2_4
+}
+
+use std::fs;
+
+#[ctor] // $ MISSING: Source=source2_5
+fn bad2_5() {
+    let _buff = fs::File::create("hello.txt").unwrap(); // $ MISSING: Alert[rust/ctor-initialization]=source2_5
+}
+
+#[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] // $ Source=source2_7
+fn bad2_7() {
+    std::thread::sleep(DURATION2_7); // $ Alert[rust/ctor-initialization]=source2_7
+}
+
+use std::process;
+
+#[ctor] // $ Source=source2_8
+fn bad2_8() {
+    process::exit(1234); // $ Alert[rust/ctor-initialization]=source2_8
+}
+
+#[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() {
+    _ = stderr().write_all(b"Hello, world!"); // $ MISSING: Alert=source3_1 Alert=source3_3 Alert=source3_4
+}
+
+#[ctor] // $ MISSING: Source=source3_1
+fn bad3_1() {
+    call_target3_1();
+}
+
+fn call_target3_2() {
+    for _x in 0..10 {
+        // ...
+    }
+}
+
+#[ctor] // $ MISSING: Source=source3_2
+fn harmless3_2() {
+    call_target3_2();
+}
+
+#[ctor]
+fn bad3_3() {
+    call_target3_1();
+    call_target3_2();
+}
+
+#[ctor] // $ MISSING: Source=source3_4
+fn bad3_4() {
+    bad3_3();
+}
+
+// --- macros ---
+
+macro_rules! macro4_1 {
+    () => {
+        _ = std::io::stdout().write(b"Hello, world!");
+    };
+}
+
+#[ctor] // $ Source=source4_1
+fn bad4_1() {
+    macro4_1!(); // $ Alert[rust/ctor-initialization]=source4_1
+}