Skip to content

Commit 1897a0d

Browse files
committed
Python: Move PathInjection configuration to own file
This one required a bit more thought, but ended up pretty nicely. Had to write some QLDoc, but I think it turned out OK.
1 parent 0c6bd84 commit 1897a0d

File tree

3 files changed

+113
-94
lines changed

3 files changed

+113
-94
lines changed

python/ql/src/Security/CWE-022/PathInjection.ql

Lines changed: 2 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -14,103 +14,11 @@
1414
* external/cwe/cwe-036
1515
* external/cwe/cwe-073
1616
* external/cwe/cwe-099
17-
*
18-
* The query detects cases where a user-controlled path is used in an unsafe manner,
19-
* meaning it is not both normalized and _afterwards_ checked.
20-
*
21-
* It does so by dividing the problematic situation into two cases:
22-
* 1. The file path is never normalized.
23-
* This is easily detected by using normalization as a sanitizer.
24-
*
25-
* 2. The file path is normalized at least once, but never checked afterwards.
26-
* This is detected by finding the earliest normalization and then ensuring that
27-
* no checks happen later. Since we start from the earliest normalization,
28-
* we know that the absence of checks means that no normalization has a
29-
* check after it. (No checks after a second normalization would be ok if
30-
* there was a check between the first and the second.)
31-
*
32-
* Note that one could make the dual split on whether the file path is ever checked. This does
33-
* not work as nicely, however, since checking is modelled as a `BarrierGuard` rather than
34-
* as a `Sanitizer`. That means that only some dataflow paths out of a check will be removed,
35-
* and so identifying the last check is not possible simply by finding a dataflow path from it
36-
* to a sink.
3717
*/
3818

3919
import python
40-
import semmle.python.dataflow.new.DataFlow
41-
import semmle.python.dataflow.new.DataFlow2
42-
import semmle.python.dataflow.new.TaintTracking
43-
import semmle.python.dataflow.new.TaintTracking2
44-
import semmle.python.Concepts
45-
import semmle.python.dataflow.new.RemoteFlowSources
46-
import ChainedConfigs12
20+
import semmle.python.security.dataflow.PathInjection
4721

48-
// ---------------------------------------------------------------------------
49-
// Case 1. The path is never normalized.
50-
// ---------------------------------------------------------------------------
51-
/** Configuration to find paths from sources to sinks that contain no normalization. */
52-
class PathNotNormalizedConfiguration extends TaintTracking::Configuration {
53-
PathNotNormalizedConfiguration() { this = "PathNotNormalizedConfiguration" }
54-
55-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
56-
57-
override predicate isSink(DataFlow::Node sink) {
58-
sink = any(FileSystemAccess e).getAPathArgument()
59-
}
60-
61-
override predicate isSanitizer(DataFlow::Node node) { node instanceof Path::PathNormalization }
62-
}
63-
64-
predicate pathNotNormalized(CustomPathNode source, CustomPathNode sink) {
65-
any(PathNotNormalizedConfiguration config).hasFlowPath(source.asNode1(), sink.asNode1())
66-
}
67-
68-
// ---------------------------------------------------------------------------
69-
// Case 2. The path is normalized at least once, but never checked afterwards.
70-
// ---------------------------------------------------------------------------
71-
/** Configuration to find paths from sources to normalizations that contain no prior normalizations. */
72-
class FirstNormalizationConfiguration extends TaintTracking::Configuration {
73-
FirstNormalizationConfiguration() { this = "FirstNormalizationConfiguration" }
74-
75-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
76-
77-
override predicate isSink(DataFlow::Node sink) { sink instanceof Path::PathNormalization }
78-
79-
override predicate isSanitizerOut(DataFlow::Node node) { node instanceof Path::PathNormalization }
80-
}
81-
82-
/** Configuration to find paths from normalizations to sinks that do not go through a check. */
83-
class NormalizedPathNotCheckedConfiguration extends TaintTracking2::Configuration {
84-
NormalizedPathNotCheckedConfiguration() { this = "NormalizedPathNotCheckedConfiguration" }
85-
86-
override predicate isSource(DataFlow::Node source) { source instanceof Path::PathNormalization }
87-
88-
override predicate isSink(DataFlow::Node sink) {
89-
sink = any(FileSystemAccess e).getAPathArgument()
90-
}
91-
92-
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
93-
guard instanceof Path::SafeAccessCheck
94-
}
95-
}
96-
97-
predicate pathNotCheckedAfterNormalization(CustomPathNode source, CustomPathNode sink) {
98-
exists(
99-
FirstNormalizationConfiguration config, DataFlow::PathNode mid1, DataFlow2::PathNode mid2,
100-
NormalizedPathNotCheckedConfiguration config2
101-
|
102-
config.hasFlowPath(source.asNode1(), mid1) and
103-
config2.hasFlowPath(mid2, sink.asNode2()) and
104-
mid1.getNode().asCfgNode() = mid2.getNode().asCfgNode()
105-
)
106-
}
107-
108-
// ---------------------------------------------------------------------------
109-
// Query: Either case 1 or case 2.
110-
// ---------------------------------------------------------------------------
11122
from CustomPathNode source, CustomPathNode sink
112-
where
113-
pathNotNormalized(source, sink)
114-
or
115-
pathNotCheckedAfterNormalization(source, sink)
23+
where pathInjection(source, sink)
11624
select sink, source, sink, "This path depends on $@.", source, "a user-provided value"

python/ql/src/Security/CWE-022/ChainedConfigs12.qll renamed to python/ql/src/semmle/python/security/dataflow/ChainedConfigs12.qll

File renamed without changes.
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about path injection
3+
* vulnerabilities.
4+
*
5+
* We detect cases where a user-controlled path is used in an unsafe manner,
6+
* meaning it is not both normalized and _afterwards_ checked.
7+
*
8+
* It does so by dividing the problematic situation into two cases:
9+
* 1. The file path is never normalized.
10+
* This is easily detected by using normalization as a sanitizer.
11+
*
12+
* 2. The file path is normalized at least once, but never checked afterwards.
13+
* This is detected by finding the earliest normalization and then ensuring that
14+
* no checks happen later. Since we start from the earliest normalization,
15+
* we know that the absence of checks means that no normalization has a
16+
* check after it. (No checks after a second normalization would be ok if
17+
* there was a check between the first and the second.)
18+
*
19+
* Note that one could make the dual split on whether the file path is ever checked. This does
20+
* not work as nicely, however, since checking is modelled as a `BarrierGuard` rather than
21+
* as a `Sanitizer`. That means that only some dataflow paths out of a check will be removed,
22+
* and so identifying the last check is not possible simply by finding a dataflow path from it
23+
* to a sink.
24+
*/
25+
26+
import python
27+
import semmle.python.dataflow.new.DataFlow
28+
import semmle.python.dataflow.new.DataFlow2
29+
import semmle.python.dataflow.new.TaintTracking
30+
import semmle.python.dataflow.new.TaintTracking2
31+
import semmle.python.Concepts
32+
import semmle.python.dataflow.new.RemoteFlowSources
33+
import ChainedConfigs12
34+
35+
// ---------------------------------------------------------------------------
36+
// Case 1. The path is never normalized.
37+
// ---------------------------------------------------------------------------
38+
/** Configuration to find paths from sources to sinks that contain no normalization. */
39+
class PathNotNormalizedConfiguration extends TaintTracking::Configuration {
40+
PathNotNormalizedConfiguration() { this = "PathNotNormalizedConfiguration" }
41+
42+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
43+
44+
override predicate isSink(DataFlow::Node sink) {
45+
sink = any(FileSystemAccess e).getAPathArgument()
46+
}
47+
48+
override predicate isSanitizer(DataFlow::Node node) { node instanceof Path::PathNormalization }
49+
}
50+
51+
/**
52+
* Holds if there is a path injection from source to sink, where the (python) path is
53+
* not normalized.
54+
*/
55+
predicate pathNotNormalized(CustomPathNode source, CustomPathNode sink) {
56+
any(PathNotNormalizedConfiguration config).hasFlowPath(source.asNode1(), sink.asNode1())
57+
}
58+
59+
// ---------------------------------------------------------------------------
60+
// Case 2. The path is normalized at least once, but never checked afterwards.
61+
// ---------------------------------------------------------------------------
62+
/** Configuration to find paths from sources to normalizations that contain no prior normalizations. */
63+
class FirstNormalizationConfiguration extends TaintTracking::Configuration {
64+
FirstNormalizationConfiguration() { this = "FirstNormalizationConfiguration" }
65+
66+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
67+
68+
override predicate isSink(DataFlow::Node sink) { sink instanceof Path::PathNormalization }
69+
70+
override predicate isSanitizerOut(DataFlow::Node node) { node instanceof Path::PathNormalization }
71+
}
72+
73+
/** Configuration to find paths from normalizations to sinks that do not go through a check. */
74+
class NormalizedPathNotCheckedConfiguration extends TaintTracking2::Configuration {
75+
NormalizedPathNotCheckedConfiguration() { this = "NormalizedPathNotCheckedConfiguration" }
76+
77+
override predicate isSource(DataFlow::Node source) { source instanceof Path::PathNormalization }
78+
79+
override predicate isSink(DataFlow::Node sink) {
80+
sink = any(FileSystemAccess e).getAPathArgument()
81+
}
82+
83+
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
84+
guard instanceof Path::SafeAccessCheck
85+
}
86+
}
87+
88+
/**
89+
* Holds if there is a path injection from source to sink, where the (python) path is
90+
* normalized at least once, but never checked afterwards.
91+
*/
92+
predicate pathNotCheckedAfterNormalization(CustomPathNode source, CustomPathNode sink) {
93+
exists(
94+
FirstNormalizationConfiguration config, DataFlow::PathNode mid1, DataFlow2::PathNode mid2,
95+
NormalizedPathNotCheckedConfiguration config2
96+
|
97+
config.hasFlowPath(source.asNode1(), mid1) and
98+
config2.hasFlowPath(mid2, sink.asNode2()) and
99+
mid1.getNode().asCfgNode() = mid2.getNode().asCfgNode()
100+
)
101+
}
102+
103+
// ---------------------------------------------------------------------------
104+
// Query: Either case 1 or case 2.
105+
// ---------------------------------------------------------------------------
106+
/** Holds if there is a path injection from source to sink */
107+
predicate pathInjection(CustomPathNode source, CustomPathNode sink) {
108+
pathNotNormalized(source, sink)
109+
or
110+
pathNotCheckedAfterNormalization(source, sink)
111+
}

0 commit comments

Comments
 (0)