-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: New query for bad 'ctor' initialization #18097
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
68a4ea3
Rust: New query rust/ctor-initialization (placeholder).
geoffw0 9ead2dc
Rust: Add a query test.
geoffw0 88fc7be
Rust: Implement the query.
geoffw0 82f2c60
Rust: Add qhelp + examples.
geoffw0 be5bd1d
Rust: Also add the good example and a couple of other cited good case…
geoffw0 77f5168
Rust: Query metadata and path edges.
geoffw0 e8981a5
Rust: Fix qhelp.
geoffw0 e6302ca
Rust: Address CI and ql-for-ql issues.
geoffw0 28c0e89
Rust: Autoformat.
geoffw0 a6f20a6
Apply suggestions from code review
geoffw0 0f34693
Merge branch 'main' into ctor
geoffw0 49b569c
Rust: Update for changes on main.
geoffw0 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
44 changes: 44 additions & 0 deletions
44
rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. | ||
geoffw0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| </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: | ||
geoffw0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| </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> | ||
58 changes: 58 additions & 0 deletions
58
rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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).getExpr().(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." |
5 changes: 5 additions & 0 deletions
5
rust/ql/src/queries/security/CWE-696/BadCtorInitializationBad.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
|
|
||
| #[ctor::ctor] | ||
| fn bad_example() { | ||
| println!("Hello, world!"); // BAD: the println! macro calls std library functions | ||
| } |
5 changes: 5 additions & 0 deletions
5
rust/ql/src/queries/security/CWE-696/BadCtorInitializationGood.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
|
|
||
| #[ctor::ctor] | ||
| fn good_example() { | ||
| libc_print::libc_println!("Hello, world!"); // GOOD: libc-print does not use the std library | ||
| } |
22 changes: 22 additions & 0 deletions
22
rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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(...) | |
2 changes: 2 additions & 0 deletions
2
rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.qlref
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| query: queries/security/CWE-696/BadCtorInitialization.ql | ||
| postprocess: utils/InlineExpectationsTestQuery.ql |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| qltest_cargo_check: true | ||
| qltest_dependencies: | ||
| - ctor = { version = "0.2.9" } | ||
| - libc-print = { version = "0.1.23" } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.