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: Improve rust/ctor-initialization #18176

Open
wants to merge 7 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
48 changes: 33 additions & 15 deletions rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import rust

/**
* A `#[ctor]` or `#[dtor]` attribute.
* A `#[ctor]` or `#[dtor]` attribute, that is, a source for this query.
*/
class CtorAttr extends Attr {
string whichAttr;
Expand All @@ -28,7 +28,7 @@ class CtorAttr extends Attr {
}

/**
* A call into the Rust standard library.
* A call into the Rust standard library, that is, a sink for this query.
*/
class StdCall extends Expr {
StdCall() {
Expand All @@ -39,20 +39,38 @@ class StdCall extends Expr {

class PathElement = AstNode;

/**
* Holds if (`pred`, `succ`) represents a candidate edge for the query that is
* reachable from a source.
*/
predicate edgesFwd(PathElement pred, PathElement succ) {
// attribute (source) -> callable
pred.(CtorAttr) = succ.(Callable).getAnAttr()
or
// [forwards reachable] callable -> enclosed call
edgesFwd(_, pred) and
pred = succ.(CallExprBase).getEnclosingCallable()
or
// [forwards reachable] call -> target callable
edgesFwd(_, pred) and
pred.(CallExprBase).getStaticTarget() = succ
}

/**
* Holds if (`pred`, `succ`) represents an edge for the query that is reachable
* from a source and backwards reachable from a sink (adding the backwards
* reachability constraint reduces the amount of output data produced).
*/
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
edgesFwd(pred, succ) and
(
succ instanceof StdCall // sink
or
edges(succ, _) // backwards reachable from a 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."
from CtorAttr source, StdCall sink
where edges+(source, sink)
select sink, source, sink,
"Call to " + sink.toString() + " in a function with the " + source.getWhichAttr() + " attribute."
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,37 @@
| 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. |
| test.rs:126:9:126:16 | stderr(...) | test.rs:129:1:129:7 | Attr | test.rs:126:9:126:16 | stderr(...) | Call to stderr(...) in a function with the ctor attribute. |
| test.rs:126:9:126:16 | stderr(...) | test.rs:145:1:145:7 | Attr | test.rs:126:9:126:16 | stderr(...) | Call to stderr(...) in a function with the ctor attribute. |
| test.rs:126:9:126:44 | ... .write_all(...) | test.rs:129:1:129:7 | Attr | test.rs:126:9:126:44 | ... .write_all(...) | Call to ... .write_all(...) in a function with the ctor attribute. |
| test.rs:126:9:126:44 | ... .write_all(...) | test.rs:145:1:145:7 | Attr | test.rs:126:9:126:44 | ... .write_all(...) | Call to ... .write_all(...) in a function with the ctor attribute. |
| test.rs:171:5:171:15 | ...::stdout(...) | test.rs:169:1:169:7 | Attr | test.rs:171:5:171: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(...) |
| test.rs:29:1:29:13 | Attr | test.rs:29:1:32:1 | fn bad1_1 |
| test.rs:29:1:32:1 | fn bad1_1 | test.rs:31:9:31:25 | ...::stdout(...) |
| test.rs:34:1:34:13 | Attr | test.rs:34:1:37:1 | fn bad1_2 |
| test.rs:34:1:37:1 | fn bad1_2 | test.rs:36:9:36:25 | ...::stdout(...) |
| test.rs:39:1:44:1 | fn bad1_3 | test.rs:43:9:43:25 | ...::stdout(...) |
| test.rs:40:1:40:13 | Attr | test.rs:39:1:44:1 | fn bad1_3 |
| test.rs:51:1:51:7 | Attr | test.rs:51:1:54:1 | fn bad2_1 |
| test.rs:51:1:54:1 | fn bad2_1 | test.rs:53:9:53:16 | stdout(...) |
| test.rs:56:1:56:7 | Attr | test.rs:56:1:59:1 | fn bad2_2 |
| test.rs:56:1:59:1 | fn bad2_2 | test.rs:58:9:58:16 | stderr(...) |
| test.rs:61:1:61:7 | Attr | test.rs:61:1:64:1 | fn bad2_3 |
| test.rs:61:1:64:1 | fn bad2_3 | test.rs:63:14:63:28 | ...::_print(...) |
| test.rs:66:1:66:7 | Attr | test.rs:66:1:70:1 | fn bad2_4 |
| test.rs:66:1:70:1 | fn bad2_4 | test.rs:69:9:69:24 | ...::stdin(...) |
| test.rs:88:1:88:7 | Attr | test.rs:88:1:91:1 | fn bad2_7 |
| test.rs:88:1:91:1 | fn bad2_7 | test.rs:90:5:90:35 | ...::sleep(...) |
| test.rs:95:1:95:7 | Attr | test.rs:95:1:98:1 | fn bad2_8 |
| test.rs:95:1:98:1 | fn bad2_8 | test.rs:97:5:97:23 | ...::exit(...) |
| test.rs:125:1:127:1 | fn call_target3_1 | test.rs:126:9:126:16 | stderr(...) |
| test.rs:125:1:127:1 | fn call_target3_1 | test.rs:126:9:126:44 | ... .write_all(...) |
| test.rs:129:1:129:7 | Attr | test.rs:129:1:132:1 | fn bad3_1 |
| test.rs:129:1:132:1 | fn bad3_1 | test.rs:131:5:131:20 | call_target3_1(...) |
| test.rs:131:5:131:20 | call_target3_1(...) | test.rs:125:1:127:1 | fn call_target3_1 |
| test.rs:145:1:145:7 | Attr | test.rs:145:1:149:1 | fn bad3_3 |
| test.rs:145:1:149:1 | fn bad3_3 | test.rs:147:5:147:20 | call_target3_1(...) |
| test.rs:147:5:147:20 | call_target3_1(...) | test.rs:125:1:127:1 | fn call_target3_1 |
| test.rs:169:1:169:7 | Attr | test.rs:169:1:172:1 | fn bad4_1 |
| test.rs:169:1:172:1 | fn bad4_1 | test.rs:171:5:171:15 | ...::stdout(...) |
13 changes: 9 additions & 4 deletions rust/ql/test/query-tests/security/CWE-696/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,10 @@ unsafe fn harmless2_11() {
// --- transitive cases ---

fn call_target3_1() {
_ = stderr().write_all(b"Hello, world!"); // $ MISSING: Alert=source3_1 Alert=source3_3 Alert=source3_4
_ = stderr().write_all(b"Hello, world!"); // $ Alert=source3_1 Alert=source3_3 MISSING: Alert=source3_4
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source3_4 result is still missing because the call to bad3_3() has no getStaticTarget().

}

#[ctor] // $ MISSING: Source=source3_1
#[ctor] // $ Source=source3_1
fn bad3_1() {
call_target3_1();
}
Expand All @@ -137,12 +137,12 @@ fn call_target3_2() {
}
}

#[ctor] // $ MISSING: Source=source3_2
#[ctor]
fn harmless3_2() {
call_target3_2();
}

#[ctor]
#[ctor] // $ Source=source3_3
fn bad3_3() {
call_target3_1();
call_target3_2();
Expand All @@ -153,6 +153,11 @@ fn bad3_4() {
bad3_3();
}

fn harmless3_5() {
call_target3_1();
call_target3_2();
}

// --- macros ---

macro_rules! macro4_1 {
Expand Down