Skip to content

Commit 81af324

Browse files
aibaarsaibaars
committed
Rust: tainted path implement basic sanitizers
1 parent cb615f2 commit 81af324

File tree

9 files changed

+248
-43
lines changed

9 files changed

+248
-43
lines changed

‎rust/ql/lib/codeql/rust/Concepts.qll‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ private import codeql.rust.dataflow.DataFlow
88
private import codeql.threatmodels.ThreatModels
99
private import codeql.rust.Frameworks
1010
private import codeql.rust.dataflow.FlowSource
11+
private import codeql.rust.controlflow.ControlFlowGraph as Cfg
12+
private import codeql.rust.controlflow.CfgNodes as CfgNodes
1113

1214
/**
1315
* A data flow source for a specific threat-model.
@@ -264,3 +266,44 @@ module Cryptography {
264266

265267
class CryptographicAlgorithm = SC::CryptographicAlgorithm;
266268
}
269+
270+
/** Provides classes for modeling path-related APIs. */
271+
module Path {
272+
/**
273+
* A data-flow node that performs path normalization. This is often needed in order
274+
* to safely access paths.
275+
*/
276+
class PathNormalization extends DataFlow::Node instanceof PathNormalization::Range {
277+
/** Gets an argument to this path normalization that is interpreted as a path. */
278+
DataFlow::Node getPathArg() { result = super.getPathArg() }
279+
}
280+
281+
/** Provides a class for modeling new path normalization APIs. */
282+
module PathNormalization {
283+
/**
284+
* A data-flow node that performs path normalization. This is often needed in order
285+
* to safely access paths.
286+
*/
287+
abstract class Range extends DataFlow::Node {
288+
/** Gets an argument to this path normalization that is interpreted as a path. */
289+
abstract DataFlow::Node getPathArg();
290+
}
291+
}
292+
293+
class SafeAccessCheck extends DataFlow::ExprNode {
294+
SafeAccessCheck() { this = DataFlow::BarrierGuard<safeAccessCheck/3>::getABarrierNode() }
295+
}
296+
297+
private predicate safeAccessCheck(CfgNodes::AstCfgNode g, Cfg::CfgNode node, boolean branch) {
298+
g.(SafeAccessCheck::Range).checks(node, branch)
299+
}
300+
301+
/** Provides a class for modeling new path safety checks. */
302+
module SafeAccessCheck {
303+
/** A data-flow node that checks that a path is safe to access. */
304+
abstract class Range extends CfgNodes::AstCfgNode {
305+
/** Holds if this guard validates `node` upon evaluating to `branch`. */
306+
abstract predicate checks(Cfg::CfgNode node, boolean branch);
307+
}
308+
}
309+
}

‎rust/ql/lib/codeql/rust/Frameworks.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ private import codeql.rust.frameworks.rustcrypto.RustCrypto
66
private import codeql.rust.frameworks.Poem
77
private import codeql.rust.frameworks.Sqlx
88
private import codeql.rust.frameworks.stdlib.Clone
9+
private import codeql.rust.frameworks.stdlib.Stdlib

‎rust/ql/lib/codeql/rust/frameworks/stdlib/Stdlib.qll‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
private import rust
2+
private import codeql.rust.Concepts
3+
private import codeql.rust.controlflow.ControlFlowGraph as Cfg
4+
private import codeql.rust.controlflow.CfgNodes as CfgNodes
5+
private import codeql.rust.dataflow.DataFlow
6+
7+
/**
8+
* A call to the `starts_with` method on a `Path`.
9+
*/
10+
private class StartswithCall extends Path::SafeAccessCheck::Range, CfgNodes::MethodCallExprCfgNode {
11+
StartswithCall() {
12+
this.getAstNode().(Resolvable).getResolvedPath() = "<crate::path::Path>::starts_with"
13+
}
14+
15+
override predicate checks(Cfg::CfgNode e, boolean branch) {
16+
e = this.getReceiver() and
17+
branch = true
18+
}
19+
}
20+
21+
/**
22+
* A call to `Path.canonicalize`.
23+
* See https://doc.rust-lang.org/std/path/struct.Path.html#method.canonicalize
24+
*/
25+
private class PathCanonicalizeCall extends Path::PathNormalization::Range {
26+
CfgNodes::MethodCallExprCfgNode call;
27+
28+
PathCanonicalizeCall() {
29+
call = this.asExpr() and
30+
call.getAstNode().(Resolvable).getResolvedPath() = "<crate::path::Path>::canonicalize"
31+
}
32+
33+
override DataFlow::Node getPathArg() { result.asExpr() = call.getReceiver() }
34+
}

‎rust/ql/lib/codeql/rust/frameworks/stdlib/fs.model.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,4 @@ extensions:
1616
- ["lang:std", "<crate::path::PathBuf as crate::convert::From>::from", "Argument[0]", "ReturnValue", "taint", "manual"]
1717
- ["lang:std", "<crate::path::Path>::join", "Argument[self]", "ReturnValue", "taint", "manual"]
1818
- ["lang:std", "<crate::path::Path>::join", "Argument[0]", "ReturnValue", "taint", "manual"]
19+
- ["lang:std", "<crate::path::Path>::canonicalize", "Argument[self]", "ReturnValue.Field[crate::result::Result::Ok(0)]", "taint", "manual"]

‎rust/ql/lib/codeql/rust/frameworks/stdlib/lang-core.model.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,6 @@ extensions:
3232
- ["lang:alloc", "<crate::string::String>::as_str", "Argument[self]", "ReturnValue", "taint", "manual"]
3333
- ["lang:alloc", "<crate::string::String>::as_bytes", "Argument[self]", "ReturnValue", "taint", "manual"]
3434
- ["lang:alloc", "<_ as crate::string::ToString>::to_string", "Argument[self]", "ReturnValue", "taint", "manual"]
35+
# Result
36+
- ["lang:core", "<crate::result::Result>::unwrap", "Argument[self]", "ReturnValue", "taint", "manual"]
37+

‎rust/ql/lib/codeql/rust/security/TaintedPathExtensions.qll‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ private import codeql.rust.dataflow.DataFlow
77
private import codeql.rust.dataflow.TaintTracking
88
private import codeql.rust.Concepts
99
private import codeql.rust.dataflow.internal.DataFlowImpl
10+
private import codeql.rust.controlflow.ControlFlowGraph as Cfg
11+
private import codeql.rust.controlflow.CfgNodes as CfgNodes
1012

1113
/**
1214
* Provides default sources, sinks and barriers for detecting path injection
@@ -28,6 +30,10 @@ module TaintedPath {
2830
*/
2931
abstract class Barrier extends DataFlow::Node { }
3032

33+
class SanitizerGuard extends DataFlow::Node {
34+
SanitizerGuard() { this = DataFlow::BarrierGuard<sanitizerGuard/3>::getABarrierNode() }
35+
}
36+
3137
/**
3238
* An active threat-model source, considered as a flow source.
3339
*/
@@ -38,3 +44,33 @@ module TaintedPath {
3844
ModelsAsDataSinks() { sinkNode(this, "path-injection") }
3945
}
4046
}
47+
48+
private predicate sanitizerGuard(CfgNodes::AstCfgNode g, Cfg::CfgNode node, boolean branch) {
49+
g.(SanitizerGuard::Range).checks(node, branch)
50+
}
51+
52+
/** Provides a class for modeling new path safety checks. */
53+
module SanitizerGuard {
54+
/** A data-flow node that checks that a path is safe to access. */
55+
abstract class Range extends CfgNodes::AstCfgNode {
56+
/** Holds if this guard validates `node` upon evaluating to `branch`. */
57+
abstract predicate checks(Cfg::CfgNode node, boolean branch);
58+
}
59+
}
60+
61+
/**
62+
* A check of the form `!strings.Contains(nd, "..")`, considered as a sanitizer guard for
63+
* path traversal.
64+
*/
65+
private class DotDotCheck extends SanitizerGuard::Range, CfgNodes::MethodCallExprCfgNode {
66+
DotDotCheck() {
67+
this.getAstNode().(Resolvable).getResolvedPath() = "<str>::contains" and
68+
this.getArgument(0).getAstNode().(LiteralExpr).getTextValue() =
69+
["\"..\"", "\"../\"", "\"..\\\""]
70+
}
71+
72+
override predicate checks(Cfg::CfgNode e, boolean branch) {
73+
e = this.getReceiver() and
74+
branch = false
75+
}
76+
}

‎rust/ql/src/queries/security/CWE-022/TaintedPath.ql‎

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,72 @@ import codeql.rust.dataflow.DataFlow
1919
import codeql.rust.dataflow.TaintTracking
2020
import codeql.rust.security.TaintedPathExtensions
2121
import TaintedPathFlow::PathGraph
22+
private import codeql.rust.Concepts
23+
24+
abstract private class NormalizationState extends string {
25+
bindingset[this]
26+
NormalizationState() { any() }
27+
}
28+
29+
/** A state signifying that the file path has not been normalized. */
30+
class NotNormalized extends NormalizationState {
31+
NotNormalized() { this = "NotNormalized" }
32+
}
33+
34+
/** A state signifying that the file path has been normalized, but not checked. */
35+
class NormalizedUnchecked extends NormalizationState {
36+
NormalizedUnchecked() { this = "NormalizedUnchecked" }
37+
}
2238

2339
/**
24-
* A taint configuration for tainted data that reaches a file access sink.
40+
* This configuration uses two flow states, `NotNormalized` and `NormalizedUnchecked`,
41+
* to track the requirement that a file path must be first normalized and then checked
42+
* before it is safe to use.
43+
*
44+
* At sources, paths are assumed not normalized. At normalization points, they change
45+
* state to `NormalizedUnchecked` after which they can be made safe by an appropriate
46+
* check of the prefix.
47+
*
48+
* Such checks are ineffective in the `NotNormalized` state.
2549
*/
26-
module TaintedPathConfig implements DataFlow::ConfigSig {
27-
predicate isSource(DataFlow::Node node) { node instanceof TaintedPath::Source }
50+
module TaintedPathConfig implements DataFlow::StateConfigSig {
51+
class FlowState = NormalizationState;
52+
53+
predicate isSource(DataFlow::Node source, FlowState state) {
54+
source instanceof TaintedPath::Source and state instanceof NotNormalized
55+
}
56+
57+
predicate isSink(DataFlow::Node sink, FlowState state) {
58+
sink instanceof TaintedPath::Sink and
59+
(
60+
state instanceof NotNormalized or
61+
state instanceof NormalizedUnchecked
62+
)
63+
}
64+
65+
predicate isBarrier(DataFlow::Node node) {
66+
node instanceof TaintedPath::Barrier or node instanceof TaintedPath::SanitizerGuard
67+
}
2868

29-
predicate isSink(DataFlow::Node node) { node instanceof TaintedPath::Sink }
69+
predicate isBarrier(DataFlow::Node node, FlowState state) {
70+
// Block `NotNormalized` paths here, since they change state to `NormalizedUnchecked`
71+
node instanceof Path::PathNormalization and
72+
state instanceof NotNormalized
73+
or
74+
node instanceof Path::SafeAccessCheck and
75+
state instanceof NormalizedUnchecked
76+
}
3077

31-
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof TaintedPath::Barrier }
78+
predicate isAdditionalFlowStep(
79+
DataFlow::Node nodeFrom, FlowState stateFrom, DataFlow::Node nodeTo, FlowState stateTo
80+
) {
81+
nodeFrom = nodeTo.(Path::PathNormalization).getPathArg() and
82+
stateFrom instanceof NotNormalized and
83+
stateTo instanceof NormalizedUnchecked
84+
}
3285
}
3386

34-
module TaintedPathFlow = TaintTracking::Global<TaintedPathConfig>;
87+
module TaintedPathFlow = TaintTracking::GlobalWithState<TaintedPathConfig>;
3588

3689
from TaintedPathFlow::PathNode source, TaintedPathFlow::PathNode sink
3790
where TaintedPathFlow::flowPath(source, sink)

‎rust/ql/test/query-tests/security/CWE-022/TaintedPath.expected‎

Lines changed: 40 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,57 @@
11
#select
22
| src/main.rs:10:5:10:22 | ...::read_to_string | src/main.rs:6:11:6:19 | file_name | src/main.rs:10:5:10:22 | ...::read_to_string | This path depends on a $@. | src/main.rs:6:11:6:19 | file_name | user-provided value |
3-
| src/main.rs:22:5:22:22 | ...::read_to_string | src/main.rs:15:11:15:19 | file_name | src/main.rs:22:5:22:22 | ...::read_to_string | This path depends on a $@. | src/main.rs:15:11:15:19 | file_name | user-provided value |
4-
| src/main.rs:35:5:35:22 | ...::read_to_string | src/main.rs:27:11:27:19 | file_path | src/main.rs:35:5:35:22 | ...::read_to_string | This path depends on a $@. | src/main.rs:27:11:27:19 | file_path | user-provided value |
3+
| src/main.rs:45:5:45:22 | ...::read_to_string | src/main.rs:37:11:37:19 | file_path | src/main.rs:45:5:45:22 | ...::read_to_string | This path depends on a $@. | src/main.rs:37:11:37:19 | file_path | user-provided value |
4+
| src/main.rs:59:5:59:22 | ...::read_to_string | src/main.rs:50:11:50:19 | file_path | src/main.rs:59:5:59:22 | ...::read_to_string | This path depends on a $@. | src/main.rs:50:11:50:19 | file_path | user-provided value |
55
edges
66
| src/main.rs:6:11:6:19 | file_name | src/main.rs:8:35:8:43 | file_name | provenance | |
77
| src/main.rs:8:9:8:17 | file_path | src/main.rs:10:24:10:32 | file_path | provenance | |
88
| src/main.rs:8:21:8:44 | ...::from(...) | src/main.rs:8:9:8:17 | file_path | provenance | |
9-
| src/main.rs:8:35:8:43 | file_name | src/main.rs:8:21:8:44 | ...::from(...) | provenance | MaD:3 |
9+
| src/main.rs:8:35:8:43 | file_name | src/main.rs:8:21:8:44 | ...::from(...) | provenance | MaD:4 |
1010
| src/main.rs:10:24:10:32 | file_path | src/main.rs:10:5:10:22 | ...::read_to_string | provenance | MaD:1 Sink:MaD:1 |
11-
| src/main.rs:15:11:15:19 | file_name | src/main.rs:21:35:21:43 | file_name | provenance | |
12-
| src/main.rs:21:9:21:17 | file_path | src/main.rs:22:24:22:32 | file_path | provenance | |
13-
| src/main.rs:21:21:21:44 | ...::from(...) | src/main.rs:21:9:21:17 | file_path | provenance | |
14-
| src/main.rs:21:35:21:43 | file_name | src/main.rs:21:21:21:44 | ...::from(...) | provenance | MaD:3 |
15-
| src/main.rs:22:24:22:32 | file_path | src/main.rs:22:5:22:22 | ...::read_to_string | provenance | MaD:1 Sink:MaD:1 |
16-
| src/main.rs:27:11:27:19 | file_path | src/main.rs:30:52:30:60 | file_path | provenance | |
17-
| src/main.rs:30:9:30:17 | file_path | src/main.rs:35:24:35:32 | file_path | provenance | |
18-
| src/main.rs:30:21:30:62 | public_path.join(...) | src/main.rs:30:9:30:17 | file_path | provenance | |
19-
| src/main.rs:30:38:30:61 | ...::from(...) | src/main.rs:30:21:30:62 | public_path.join(...) | provenance | MaD:2 |
20-
| src/main.rs:30:52:30:60 | file_path | src/main.rs:30:38:30:61 | ...::from(...) | provenance | MaD:3 |
21-
| src/main.rs:35:24:35:32 | file_path | src/main.rs:35:5:35:22 | ...::read_to_string | provenance | MaD:1 Sink:MaD:1 |
11+
| src/main.rs:37:11:37:19 | file_path | src/main.rs:40:52:40:60 | file_path | provenance | |
12+
| src/main.rs:40:9:40:17 | file_path | src/main.rs:45:24:45:32 | file_path | provenance | |
13+
| src/main.rs:40:21:40:62 | public_path.join(...) | src/main.rs:40:9:40:17 | file_path | provenance | |
14+
| src/main.rs:40:38:40:61 | ...::from(...) | src/main.rs:40:21:40:62 | public_path.join(...) | provenance | MaD:3 |
15+
| src/main.rs:40:52:40:60 | file_path | src/main.rs:40:38:40:61 | ...::from(...) | provenance | MaD:4 |
16+
| src/main.rs:45:24:45:32 | file_path | src/main.rs:45:5:45:22 | ...::read_to_string | provenance | MaD:1 Sink:MaD:1 |
17+
| src/main.rs:50:11:50:19 | file_path | src/main.rs:53:52:53:60 | file_path | provenance | |
18+
| src/main.rs:53:9:53:17 | file_path | src/main.rs:54:21:54:29 | file_path | provenance | |
19+
| src/main.rs:53:21:53:62 | public_path.join(...) | src/main.rs:53:9:53:17 | file_path | provenance | |
20+
| src/main.rs:53:38:53:61 | ...::from(...) | src/main.rs:53:21:53:62 | public_path.join(...) | provenance | MaD:3 |
21+
| src/main.rs:53:52:53:60 | file_path | src/main.rs:53:38:53:61 | ...::from(...) | provenance | MaD:4 |
22+
| src/main.rs:54:9:54:17 | file_path | src/main.rs:59:24:59:32 | file_path | provenance | |
23+
| src/main.rs:54:21:54:29 | file_path | src/main.rs:54:21:54:44 | file_path.canonicalize(...) | provenance | Config |
24+
| src/main.rs:54:21:54:44 | file_path.canonicalize(...) | src/main.rs:54:21:54:53 | ... .unwrap(...) | provenance | MaD:2 |
25+
| src/main.rs:54:21:54:53 | ... .unwrap(...) | src/main.rs:54:9:54:17 | file_path | provenance | |
26+
| src/main.rs:59:24:59:32 | file_path | src/main.rs:59:5:59:22 | ...::read_to_string | provenance | MaD:1 Sink:MaD:1 |
2227
models
2328
| 1 | Sink: lang:std; crate::fs::read_to_string; path-injection; Argument[0] |
24-
| 2 | Summary: lang:std; <crate::path::Path>::join; Argument[0]; ReturnValue; taint |
25-
| 3 | Summary: lang:std; <crate::path::PathBuf as crate::convert::From>::from; Argument[0]; ReturnValue; taint |
29+
| 2 | Summary: lang:core; <crate::result::Result>::unwrap; Argument[self]; ReturnValue; taint |
30+
| 3 | Summary: lang:std; <crate::path::Path>::join; Argument[0]; ReturnValue; taint |
31+
| 4 | Summary: lang:std; <crate::path::PathBuf as crate::convert::From>::from; Argument[0]; ReturnValue; taint |
2632
nodes
2733
| src/main.rs:6:11:6:19 | file_name | semmle.label | file_name |
2834
| src/main.rs:8:9:8:17 | file_path | semmle.label | file_path |
2935
| src/main.rs:8:21:8:44 | ...::from(...) | semmle.label | ...::from(...) |
3036
| src/main.rs:8:35:8:43 | file_name | semmle.label | file_name |
3137
| src/main.rs:10:5:10:22 | ...::read_to_string | semmle.label | ...::read_to_string |
3238
| src/main.rs:10:24:10:32 | file_path | semmle.label | file_path |
33-
| src/main.rs:15:11:15:19 | file_name | semmle.label | file_name |
34-
| src/main.rs:21:9:21:17 | file_path | semmle.label | file_path |
35-
| src/main.rs:21:21:21:44 | ...::from(...) | semmle.label | ...::from(...) |
36-
| src/main.rs:21:35:21:43 | file_name | semmle.label | file_name |
37-
| src/main.rs:22:5:22:22 | ...::read_to_string | semmle.label | ...::read_to_string |
38-
| src/main.rs:22:24:22:32 | file_path | semmle.label | file_path |
39-
| src/main.rs:27:11:27:19 | file_path | semmle.label | file_path |
40-
| src/main.rs:30:9:30:17 | file_path | semmle.label | file_path |
41-
| src/main.rs:30:21:30:62 | public_path.join(...) | semmle.label | public_path.join(...) |
42-
| src/main.rs:30:38:30:61 | ...::from(...) | semmle.label | ...::from(...) |
43-
| src/main.rs:30:52:30:60 | file_path | semmle.label | file_path |
44-
| src/main.rs:35:5:35:22 | ...::read_to_string | semmle.label | ...::read_to_string |
45-
| src/main.rs:35:24:35:32 | file_path | semmle.label | file_path |
39+
| src/main.rs:37:11:37:19 | file_path | semmle.label | file_path |
40+
| src/main.rs:40:9:40:17 | file_path | semmle.label | file_path |
41+
| src/main.rs:40:21:40:62 | public_path.join(...) | semmle.label | public_path.join(...) |
42+
| src/main.rs:40:38:40:61 | ...::from(...) | semmle.label | ...::from(...) |
43+
| src/main.rs:40:52:40:60 | file_path | semmle.label | file_path |
44+
| src/main.rs:45:5:45:22 | ...::read_to_string | semmle.label | ...::read_to_string |
45+
| src/main.rs:45:24:45:32 | file_path | semmle.label | file_path |
46+
| src/main.rs:50:11:50:19 | file_path | semmle.label | file_path |
47+
| src/main.rs:53:9:53:17 | file_path | semmle.label | file_path |
48+
| src/main.rs:53:21:53:62 | public_path.join(...) | semmle.label | public_path.join(...) |
49+
| src/main.rs:53:38:53:61 | ...::from(...) | semmle.label | ...::from(...) |
50+
| src/main.rs:53:52:53:60 | file_path | semmle.label | file_path |
51+
| src/main.rs:54:9:54:17 | file_path | semmle.label | file_path |
52+
| src/main.rs:54:21:54:29 | file_path | semmle.label | file_path |
53+
| src/main.rs:54:21:54:44 | file_path.canonicalize(...) | semmle.label | file_path.canonicalize(...) |
54+
| src/main.rs:54:21:54:53 | ... .unwrap(...) | semmle.label | ... .unwrap(...) |
55+
| src/main.rs:59:5:59:22 | ...::read_to_string | semmle.label | ...::read_to_string |
56+
| src/main.rs:59:24:59:32 | file_path | semmle.label | file_path |
4657
subpaths

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

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,29 +11,52 @@ fn tainted_path_handler_bad(
1111
}
1212

1313
//#[handler]
14-
fn tainted_path_handler_good(
15-
Query(file_name): Query<String>, // $ Source=remote2
16-
) -> Result<String> {
14+
fn tainted_path_handler_good(Query(file_name): Query<String>) -> Result<String> {
1715
// GOOD: ensure that the filename has no path separators or parent directory references
1816
if file_name.contains("..") || file_name.contains("/") || file_name.contains("\\") {
1917
return Err(Error::from_status(StatusCode::BAD_REQUEST));
2018
}
2119
let file_path = PathBuf::from(file_name);
22-
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink SPURIOUS: Alert[rust/path-injection]=remote2
20+
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink
2321
}
2422

2523
//#[handler]
26-
fn tainted_path_handler_folder_good(
27-
Query(file_path): Query<String>, // $ Source=remote3
28-
) -> Result<String> {
24+
fn tainted_path_handler_folder_good(Query(file_path): Query<String>) -> Result<String> {
2925
let public_path = home_dir().unwrap().join("public");
3026
let file_path = public_path.join(PathBuf::from(file_path));
3127
let file_path = file_path.canonicalize().unwrap();
3228
// GOOD: ensure that the path stays within the public folder
3329
if !file_path.starts_with(public_path) {
3430
return Err(Error::from_status(StatusCode::BAD_REQUEST));
3531
}
36-
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink SPURIOUS: Alert[rust/path-injection]=remote3
32+
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink
33+
}
34+
35+
//#[handler]
36+
fn tainted_path_handler_folder_almost_good1(
37+
Query(file_path): Query<String>, // $ Source=remote4
38+
) -> Result<String> {
39+
let public_path = home_dir().unwrap().join("public");
40+
let file_path = public_path.join(PathBuf::from(file_path));
41+
// BAD: the path could still contain `..` and escape the public folder
42+
if !file_path.starts_with(public_path) {
43+
return Err(Error::from_status(StatusCode::BAD_REQUEST));
44+
}
45+
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink Alert[rust/path-injection]=remote4
46+
}
47+
48+
//#[handler]
49+
fn tainted_path_handler_folder_almost_good2(
50+
Query(file_path): Query<String>, // $ Source=remote5
51+
) -> Result<String> {
52+
let public_path = home_dir().unwrap().join("public");
53+
let file_path = public_path.join(PathBuf::from(file_path));
54+
let file_path = file_path.canonicalize().unwrap();
55+
// BAD: thecheck to ensure that the path stays within the public folder is wrong
56+
if file_path.starts_with(public_path) {
57+
return Err(Error::from_status(StatusCode::BAD_REQUEST));
58+
}
59+
fs::read_to_string(file_path).map_err(InternalServerError) // $ path-injection-sink Alert[rust/path-injection]=remote5
3760
}
3861

3962
fn main() {}

0 commit comments

Comments
 (0)