Skip to content

Commit 6941e7f

Browse files
committed
Rust: Add tags to intermediate steps in the test.
1 parent ecf0e08 commit 6941e7f

File tree

2 files changed

+48
-9
lines changed

2 files changed

+48
-9
lines changed

rust/ql/test/query-tests/security/CWE-022/TaintedPathSinks.ql

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
11
import rust
22
import codeql.rust.security.TaintedPathExtensions
33
import utils.test.InlineExpectationsTest
4+
import codeql.rust.dataflow.DataFlow
5+
import codeql.rust.dataflow.internal.DataFlowImpl as DataflowImpl
6+
import codeql.rust.Concepts
47

58
module TaintedPathSinksTest implements TestSig {
6-
string getARelevantTag() { result = "path-injection-sink" }
9+
string getARelevantTag() {
10+
result =
11+
[
12+
"path-injection-sink", "path-injection-barrier", "path-injection-normalize",
13+
"path-injection-checked"
14+
]
15+
}
716

817
predicate hasActualResult(Location location, string element, string tag, string value) {
918
exists(TaintedPath::Sink sink |
@@ -13,6 +22,36 @@ module TaintedPathSinksTest implements TestSig {
1322
tag = "path-injection-sink" and
1423
value = ""
1524
)
25+
or
26+
exists(DataFlow::Node node |
27+
(
28+
node instanceof TaintedPath::Barrier or
29+
node instanceof TaintedPath::SanitizerGuard // tends to label the node *after* the check
30+
) and
31+
location = node.getLocation() and
32+
location.getFile().getBaseName() != "" and
33+
element = node.toString() and
34+
tag = "path-injection-barrier" and
35+
value = ""
36+
)
37+
or
38+
exists(DataFlow::Node node |
39+
DataflowImpl::optionalBarrier(node, "normalize-path") and
40+
location = node.getLocation() and
41+
location.getFile().getBaseName() != "" and
42+
element = node.toString() and
43+
tag = "path-injection-normalize" and
44+
value = ""
45+
)
46+
or
47+
exists(DataFlow::Node node |
48+
node instanceof Path::SafeAccessCheck and // tends to label the node *after* the check
49+
location = node.getLocation() and
50+
location.getFile().getBaseName() != "" and
51+
element = node.toString() and
52+
tag = "path-injection-checked" and
53+
value = ""
54+
)
1655
}
1756
}
1857

rust/ql/test/query-tests/security/CWE-022/src/main.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ fn tainted_path_handler_bad(
1313
//#[handler]
1414
fn tainted_path_handler_good(Query(file_name): Query<String>) -> Result<String> {
1515
// GOOD: ensure that the filename has no path separators or parent directory references
16-
if file_name.contains("..") || file_name.contains("/") || file_name.contains("\\") {
16+
if file_name.contains("..") || file_name.contains("/") || file_name.contains("\\") { // $ path-injection-barrier
1717
return Err(Error::from_status(StatusCode::BAD_REQUEST));
1818
}
19-
let file_path = PathBuf::from(file_name);
19+
let file_path = PathBuf::from(file_name); // $ path-injection-barrier (following the last `.contains` check)
2020
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink
2121
}
2222

@@ -29,7 +29,7 @@ fn tainted_path_handler_folder_good(Query(file_path): Query<String>) -> Result<S
2929
if !file_path.starts_with(public_path) {
3030
return Err(Error::from_status(StatusCode::BAD_REQUEST));
3131
}
32-
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink
32+
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-checked path-injection-sink
3333
}
3434

3535
//#[handler]
@@ -42,7 +42,7 @@ fn tainted_path_handler_folder_almost_good1(
4242
if !file_path.starts_with(public_path) {
4343
return Err(Error::from_status(StatusCode::BAD_REQUEST));
4444
}
45-
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink MISSING: Alert[rust/path-injection]=remote2 -- we cannot resolve the `join` call above, because it needs a `PathBuf -> Path` `Deref`
45+
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-checked path-injection-sink MISSING: Alert[rust/path-injection]=remote2 -- we cannot resolve the `join` call above, because it needs a `PathBuf -> Path` `Deref`
4646
}
4747

4848
//#[handler]
@@ -54,7 +54,7 @@ fn tainted_path_handler_folder_good_simpler(Query(file_path): Query<String>) ->
5454
if !file_path.starts_with(public_path) {
5555
return Err(Error::from_status(StatusCode::BAD_REQUEST));
5656
}
57-
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink
57+
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-checked path-injection-sink
5858
}
5959

6060
//#[handler]
@@ -67,7 +67,7 @@ fn tainted_path_handler_folder_almost_good1_simpler(
6767
if !file_path.starts_with(public_path) {
6868
return Err(Error::from_status(StatusCode::BAD_REQUEST));
6969
}
70-
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink MISSING: Alert[rust/path-injection]=remote3
70+
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-checked path-injection-sink MISSING: Alert[rust/path-injection]=remote3
7171
}
7272

7373
//#[handler]
@@ -81,7 +81,7 @@ fn tainted_path_handler_folder_almost_good2(
8181
if file_path.starts_with(public_path) {
8282
return Err(Error::from_status(StatusCode::BAD_REQUEST));
8383
}
84-
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink MISSING: Alert[rust/path-injection]=remote4 -- we cannot resolve the `join` call above, because it needs a `PathBuf -> Path` `Deref`
84+
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink MISSING: path-injection-checked Alert[rust/path-injection]=remote4 -- we cannot resolve the `join` call above, because it needs a `PathBuf -> Path` `Deref`
8585
}
8686

8787
//#[handler]
@@ -94,7 +94,7 @@ fn tainted_path_handler_folder_almost_good3(
9494
if !file_path.starts_with(public_path) {
9595
return Err(Error::from_status(StatusCode::BAD_REQUEST));
9696
}
97-
let file_path = file_path.canonicalize().unwrap();
97+
let file_path = file_path.canonicalize().unwrap(); // $ path-injection-checked
9898
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink MISSING: Alert[rust/path-injection]=remote5
9999
}
100100

0 commit comments

Comments
 (0)