Skip to content
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

Rust: New query for bad 'ctor' initialization #18097

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp
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.
</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>
58 changes: 58 additions & 0 deletions rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
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."
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
}
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
}
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(...) |
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
4 changes: 4 additions & 0 deletions rust/ql/test/query-tests/security/CWE-696/options.yml
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" }
167 changes: 167 additions & 0 deletions rust/ql/test/query-tests/security/CWE-696/test.rs
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
}