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..8dd16d6f6b13 --- /dev/null +++ b/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp @@ -0,0 +1,44 @@ + + + + +

+Calling functions and methods in the Rust std library from a #[ctor] or #[dtor] function is not safe. This is because the std library only guarantees stability and portability between the beginning and the end of main, whereas #[ctor] functions are called before main, and #[dtor] functions are called after it. +

+ + + + +

+Do not call any part of the std library from a #[ctor] or #[dtor] function. Instead either: +

+ + + + + +

+In the following example, a #[ctor] function uses the println! macro which calls std library functions. This may cause unexpected behavior at runtime. +

+ + + +

+The issue can be fixed by replacing println! with something that does not rely on the std library. In the fixed code below, we used the libc_println! macro from the libc-print library: +

+ + + + + + +
  • GitHub: rust-ctor - Warnings.
    • +
  • Rust Programming Language: Crate std - Use before and after main().
    • + +     +     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..22ea6514e02e --- /dev/null +++ b/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." 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 +} 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..508a359b0c0b --- /dev/null +++ b/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(...) | 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..cc3dc0f35196 --- /dev/null +++ b/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" } 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..87f544be85c1 --- /dev/null +++ b/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::(); + 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 +}