Skip to content

Commit 31ff652

Browse files
committed
Python: Make Sanitizer available for urlsplit taint
It isn't used by default, it has to *actively* be enabled.
1 parent fd270cc commit 31ff652

File tree

4 files changed

+83
-4
lines changed

4 files changed

+83
-4
lines changed

python/ql/src/semmle/python/security/strings/External.qll

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,3 +195,62 @@ class ExternalFileObject extends TaintKind {
195195
name = "read" and result = this.getValue()
196196
}
197197
}
198+
199+
/**
200+
* Temporary sanitizer for the tainted result from `urlsplit` and `urlparse`. Can be used to reduce FPs until
201+
* we have better support for namedtuples.
202+
*
203+
* Will clear **all** taint on a test of the kind. That is, on the true edge of any matching test,
204+
* all fields/indexes will be cleared of taint.
205+
*
206+
* Handles:
207+
* - `if splitres.netloc == "KNOWN_VALUE"`
208+
* - `if splitres[0] == "KNOWN_VALUE"`
209+
*/
210+
class UrlsplitUrlparseTempSanitizer extends Sanitizer {
211+
// TODO: remove this once we have better support for named tuples
212+
213+
UrlsplitUrlparseTempSanitizer() { this = "UrlsplitUrlparseTempSanitizer" }
214+
215+
override predicate sanitizingEdge(TaintKind taint, PyEdgeRefinement test) {
216+
(
217+
taint instanceof ExternalUrlSplitResult
218+
or
219+
taint instanceof ExternalUrlParseResult
220+
) and
221+
exists(ControlFlowNode foobar |
222+
foobar.(SubscriptNode).getObject() = test.getInput().getAUse()
223+
or
224+
foobar.(AttrNode).getObject() = test.getInput().getAUse()
225+
|
226+
clears_taint(_, foobar, test.getTest(), test.getSense())
227+
)
228+
}
229+
230+
private predicate clears_taint(ControlFlowNode final_test, ControlFlowNode tainted, ControlFlowNode test, boolean sense) {
231+
test_equality_with_const(final_test, tainted, sense)
232+
or
233+
test.(UnaryExprNode).getNode().getOp() instanceof Not and
234+
exists(ControlFlowNode nested_test |
235+
nested_test = test.(UnaryExprNode).getOperand() and
236+
clears_taint(final_test, tainted, nested_test, sense.booleanNot())
237+
)
238+
}
239+
240+
/** holds for `== "KNOWN_VALUE"` on `true` edge, and `!= "KNOWN_VALUE"` on `false` edge */
241+
private predicate test_equality_with_const(CompareNode cmp, ControlFlowNode operand, boolean sense) {
242+
exists(ControlFlowNode const, Cmpop op |
243+
const.getNode() instanceof StrConst
244+
|
245+
(
246+
cmp.operands(const, op, operand)
247+
or
248+
cmp.operands(operand, op, const)
249+
) and (
250+
op instanceof Eq and sense = true
251+
or
252+
op instanceof NotEq and sense = false
253+
)
254+
)
255+
}
256+
}

python/ql/test/library-tests/taint/namedtuple/Taint.qll

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,23 @@ class DictSource extends TaintSource {
2525

2626
override string toString() { result = "dict taint source" }
2727
}
28+
29+
class TestConfig extends TaintTracking::Configuration {
30+
TestConfig() { this = "TestConfig" }
31+
32+
override predicate isSanitizer(Sanitizer sanitizer) {
33+
sanitizer instanceof UrlsplitUrlparseTempSanitizer
34+
}
35+
36+
override predicate isSource(TaintTracking::Source source) {
37+
source instanceof SimpleSource
38+
or
39+
source instanceof ListSource
40+
or
41+
source instanceof DictSource
42+
}
43+
44+
override predicate isSink(TaintTracking::Sink sink) {
45+
none()
46+
}
47+
}

python/ql/test/library-tests/taint/namedtuple/TestTaint.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
| test.py:13 | test_basic | c | externally controlled string |
44
| test.py:13 | test_basic | d | externally controlled string |
55
| test.py:13 | test_basic | urlsplit_res | [externally controlled string] |
6-
| test.py:20 | test_sanitizer | Attribute | externally controlled string |
7-
| test.py:23 | test_sanitizer | Subscript | externally controlled string |
6+
| test.py:20 | test_sanitizer | Attribute | NO TAINT |
7+
| test.py:23 | test_sanitizer | Subscript | NO TAINT |
88
| test.py:33 | test_namedtuple | a | NO TAINT |
99
| test.py:33 | test_namedtuple | b | NO TAINT |
1010
| test.py:33 | test_namedtuple | c | NO TAINT |

python/ql/test/library-tests/taint/namedtuple/test.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ def test_sanitizer():
1717
urlsplit_res = urlsplit(tainted_string)
1818

1919
if urlsplit_res.netloc == "OK":
20-
test(urlsplit_res.netloc) # TODO: FP, should not be tainted here
20+
test(urlsplit_res.netloc)
2121

2222
if urlsplit_res[2] == "OK":
23-
test(urlsplit_res[0]) # TODO: FP, should not be tainted here
23+
test(urlsplit_res[0])
2424

2525
def test_namedtuple():
2626
tainted_string = TAINTED_STRING

0 commit comments

Comments
 (0)