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 }