Skip to content

Commit b4fbfa0

Browse files
tausbntausbn
authored
Merge pull request #3132 from RasmusWL/python-fix-iterable-unpacking-taint-CP
Python: Fix iterable-unpacking taint CP
2 parents b2769b4 + d55acc3 commit b4fbfa0

File tree

7 files changed

+111
-57
lines changed

7 files changed

+111
-57
lines changed

‎python/ql/src/semmle/python/dataflow/Implementation.qll‎

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -712,32 +712,16 @@ private class EssaTaintTracking extends string {
712712
TaintTrackingNode src, MultiAssignmentDefinition defn, TaintTrackingContext context,
713713
AttributePath path, TaintKind kind
714714
) {
715-
exists(DataFlow::Node srcnode, TaintKind srckind, Assign assign |
715+
exists(DataFlow::Node srcnode, TaintKind srckind, Assign assign, int depth |
716716
src = TTaintTrackingNode_(srcnode, context, path, srckind, this) and
717717
path.noAttribute()
718718
|
719719
assign.getValue().getAFlowNode() = srcnode.asCfgNode() and
720-
kind = iterable_unpacking_descent(assign.getATarget().getAFlowNode(), defn.getDefiningNode(),
721-
srckind)
720+
depth = iterable_unpacking_descent(assign.getATarget().getAFlowNode(), defn.getDefiningNode()) and
721+
kind = taint_at_depth(srckind, depth)
722722
)
723723
}
724724

725-
/** `((x,y), ...) = value` with any nesting on LHS */
726-
private TaintKind iterable_unpacking_descent(
727-
SequenceNode left_parent, ControlFlowNode left_defn, CollectionKind parent_kind
728-
) {
729-
//TODO: Fix the cartesian product in this predicate
730-
none() and
731-
left_parent.getAnElement() = left_defn and
732-
// Handle `a, *b = some_iterable`
733-
if left_defn instanceof StarredNode
734-
then result = parent_kind
735-
else result = parent_kind.getMember()
736-
or
737-
result = iterable_unpacking_descent(left_parent.getAnElement(), left_defn,
738-
parent_kind.getMember())
739-
}
740-
741725
pragma[noinline]
742726
private predicate taintedAttributeAssignment(
743727
TaintTrackingNode src, AttributeAssignment defn, TaintTrackingContext context,
@@ -967,6 +951,46 @@ private predicate piNodeTestAndUse(PyEdgeRefinement defn, ControlFlowNode test,
967951
test = defn.getTest() and use = defn.getInput().getASourceUse() and test.getAChild*() = use
968952
}
969953

954+
/** Helper predicate for taintedMultiAssignment */
955+
private TaintKind taint_at_depth(SequenceKind parent_kind, int depth) {
956+
depth >= 0 and
957+
(
958+
// base-case #0
959+
depth = 0 and
960+
result = parent_kind
961+
or
962+
// base-case #1
963+
depth = 1 and
964+
result = parent_kind.getMember()
965+
or
966+
// recursive case
967+
depth > 1 and
968+
result = taint_at_depth(parent_kind.getMember(), depth-1)
969+
)
970+
}
971+
972+
/** Helper predicate for taintedMultiAssignment
973+
*
974+
* Returns the `depth` the elements that are assigned to `left_defn` with iterable unpacking has,
975+
* compared to `left_parent`. Special care is taken for `StarredNode` that is assigned a sequence of items.
976+
*
977+
* For example, `((x, *y), ...) = value` with any nesting on LHS
978+
* - with `left_defn` = `x`, `left_parent` = `(x, *y)`, result = 1
979+
* - with `left_defn` = `x`, `left_parent` = `((x, *y), ...)`, result = 2
980+
* - with `left_defn` = `*y`, `left_parent` = `(x, *y)`, result = 0
981+
* - with `left_defn` = `*y`, `left_parent` = `((x, *y), ...)`, result = 1
982+
*/
983+
int iterable_unpacking_descent(SequenceNode left_parent, ControlFlowNode left_defn) {
984+
exists(Assign a | a.getATarget().getASubExpression*().getAFlowNode() = left_parent) and
985+
left_parent.getAnElement() = left_defn and
986+
// Handle `a, *b = some_iterable`
987+
if left_defn instanceof StarredNode
988+
then result = 0
989+
else result = 1
990+
or
991+
result = 1 + iterable_unpacking_descent(left_parent.getAnElement(), left_defn)
992+
}
993+
970994
module Implementation {
971995
/* A call that returns a copy (or similar) of the argument */
972996
predicate copyCall(ControlFlowNode fromnode, CallNode tonode) {

‎python/ql/test/3/library-tests/taint/unpacking/TestTaint.expected‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
| test.py:11 | extended_unpacking | first | NO TAINT |
2-
| test.py:11 | extended_unpacking | last | NO TAINT |
3-
| test.py:11 | extended_unpacking | rest | NO TAINT |
4-
| test.py:16 | also_allowed | a | NO TAINT |
1+
| test.py:11 | extended_unpacking | first | externally controlled string |
2+
| test.py:11 | extended_unpacking | last | externally controlled string |
3+
| test.py:11 | extended_unpacking | rest | [externally controlled string] |
4+
| test.py:16 | also_allowed | a | [externally controlled string] |
55
| test.py:24 | also_allowed | b | NO TAINT |
66
| test.py:24 | also_allowed | c | NO TAINT |
7-
| test.py:31 | nested | x | NO TAINT |
8-
| test.py:31 | nested | xs | NO TAINT |
9-
| test.py:31 | nested | ys | NO TAINT |
7+
| test.py:31 | nested | x | externally controlled string |
8+
| test.py:31 | nested | xs | [externally controlled string] |
9+
| test.py:31 | nested | ys | [externally controlled string] |

‎python/ql/test/library-tests/taint/collections/TestStep.expected‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626
| Taint [externally controlled string] | test.py:29 | test.py:29:9:29:25 | Subscript | | --> | Taint [externally controlled string] | test.py:32 | test.py:32:16:32:16 | c | |
2727
| Taint [externally controlled string] | test.py:30 | test.py:30:9:30:20 | tainted_list | | --> | Taint [externally controlled string] | test.py:30 | test.py:30:9:30:27 | Attribute() | |
2828
| Taint [externally controlled string] | test.py:30 | test.py:30:9:30:27 | Attribute() | | --> | Taint [externally controlled string] | test.py:32 | test.py:32:19:32:19 | d | |
29+
| Taint [externally controlled string] | test.py:31 | test.py:31:15:31:26 | tainted_list | | --> | Taint externally controlled string | test.py:32 | test.py:32:22:32:22 | e | |
30+
| Taint [externally controlled string] | test.py:31 | test.py:31:15:31:26 | tainted_list | | --> | Taint externally controlled string | test.py:32 | test.py:32:25:32:25 | f | |
31+
| Taint [externally controlled string] | test.py:31 | test.py:31:15:31:26 | tainted_list | | --> | Taint externally controlled string | test.py:32 | test.py:32:28:32:28 | g | |
2932
| Taint [externally controlled string] | test.py:33 | test.py:33:14:33:25 | tainted_list | | --> | Taint externally controlled string | test.py:33 | test.py:33:5:33:26 | For | |
3033
| Taint [externally controlled string] | test.py:35 | test.py:35:14:35:35 | reversed() | | --> | Taint externally controlled string | test.py:35 | test.py:35:5:35:36 | For | |
3134
| Taint [externally controlled string] | test.py:35 | test.py:35:23:35:34 | tainted_list | | --> | Taint [externally controlled string] | test.py:35 | test.py:35:14:35:35 | reversed() | |

‎python/ql/test/library-tests/taint/collections/TestTaint.expected‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@
1010
| test.py:32 | test_access | b | externally controlled string |
1111
| test.py:32 | test_access | c | [externally controlled string] |
1212
| test.py:32 | test_access | d | [externally controlled string] |
13-
| test.py:32 | test_access | e | NO TAINT |
14-
| test.py:32 | test_access | f | NO TAINT |
15-
| test.py:32 | test_access | g | NO TAINT |
13+
| test.py:32 | test_access | e | externally controlled string |
14+
| test.py:32 | test_access | f | externally controlled string |
15+
| test.py:32 | test_access | g | externally controlled string |
1616
| test.py:34 | test_access | h | externally controlled string |
1717
| test.py:36 | test_access | i | externally controlled string |
1818
| test.py:43 | test_dict_access | a | externally controlled string |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
| test.py:13 | test_basic | a | externally controlled string |
22
| test.py:13 | test_basic | b | externally controlled string |
33
| test.py:13 | test_basic | c | externally controlled string |
4-
| test.py:13 | test_basic | d | NO TAINT |
4+
| test.py:13 | test_basic | d | externally controlled string |
55
| test.py:13 | test_basic | urlsplit_res | [externally controlled string] |
66
| test.py:19 | test_sanitizer | Attribute | externally controlled string |
77
| test.py:22 | test_sanitizer | Attribute | NO TAINT |

‎python/ql/test/library-tests/taint/unpacking/TestStep.expected‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,35 @@
11
| Taint [[externally controlled string]] | test.py:19 | test.py:19:10:19:18 | List | | --> | Taint [[externally controlled string]] | test.py:22 | test.py:22:28:22:29 | ll | |
22
| Taint [[externally controlled string]] | test.py:19 | test.py:19:10:19:18 | List | | --> | Taint [[externally controlled string]] | test.py:26 | test.py:26:28:26:29 | ll | |
33
| Taint [[externally controlled string]] | test.py:19 | test.py:19:10:19:18 | List | | --> | Taint [[externally controlled string]] | test.py:30 | test.py:30:28:30:29 | ll | |
4+
| Taint [[externally controlled string]] | test.py:22 | test.py:22:28:22:29 | ll | | --> | Taint [externally controlled string] | test.py:23 | test.py:23:22:23:22 | b | |
5+
| Taint [[externally controlled string]] | test.py:22 | test.py:22:28:22:29 | ll | | --> | Taint [externally controlled string] | test.py:23 | test.py:23:25:23:25 | c | |
6+
| Taint [[externally controlled string]] | test.py:22 | test.py:22:28:22:29 | ll | | --> | Taint externally controlled string | test.py:23 | test.py:23:10:23:11 | a1 | |
7+
| Taint [[externally controlled string]] | test.py:22 | test.py:22:28:22:29 | ll | | --> | Taint externally controlled string | test.py:23 | test.py:23:14:23:15 | a2 | |
8+
| Taint [[externally controlled string]] | test.py:22 | test.py:22:28:22:29 | ll | | --> | Taint externally controlled string | test.py:23 | test.py:23:18:23:19 | a3 | |
9+
| Taint [[externally controlled string]] | test.py:26 | test.py:26:28:26:29 | ll | | --> | Taint [externally controlled string] | test.py:27 | test.py:27:22:27:22 | b | |
10+
| Taint [[externally controlled string]] | test.py:26 | test.py:26:28:26:29 | ll | | --> | Taint [externally controlled string] | test.py:27 | test.py:27:25:27:25 | c | |
11+
| Taint [[externally controlled string]] | test.py:26 | test.py:26:28:26:29 | ll | | --> | Taint externally controlled string | test.py:27 | test.py:27:10:27:11 | a1 | |
12+
| Taint [[externally controlled string]] | test.py:26 | test.py:26:28:26:29 | ll | | --> | Taint externally controlled string | test.py:27 | test.py:27:14:27:15 | a2 | |
13+
| Taint [[externally controlled string]] | test.py:26 | test.py:26:28:26:29 | ll | | --> | Taint externally controlled string | test.py:27 | test.py:27:18:27:19 | a3 | |
14+
| Taint [[externally controlled string]] | test.py:30 | test.py:30:28:30:29 | ll | | --> | Taint [externally controlled string] | test.py:31 | test.py:31:22:31:22 | b | |
15+
| Taint [[externally controlled string]] | test.py:30 | test.py:30:28:30:29 | ll | | --> | Taint [externally controlled string] | test.py:31 | test.py:31:25:31:25 | c | |
16+
| Taint [[externally controlled string]] | test.py:30 | test.py:30:28:30:29 | ll | | --> | Taint externally controlled string | test.py:31 | test.py:31:10:31:11 | a1 | |
17+
| Taint [[externally controlled string]] | test.py:30 | test.py:30:28:30:29 | ll | | --> | Taint externally controlled string | test.py:31 | test.py:31:14:31:15 | a2 | |
18+
| Taint [[externally controlled string]] | test.py:30 | test.py:30:28:30:29 | ll | | --> | Taint externally controlled string | test.py:31 | test.py:31:18:31:19 | a3 | |
19+
| Taint [[externally controlled string]] | test.py:47 | test.py:47:28:47:54 | Tuple | | --> | Taint externally controlled string | test.py:48 | test.py:48:10:48:10 | a | |
20+
| Taint [[externally controlled string]] | test.py:47 | test.py:47:28:47:54 | Tuple | | --> | Taint externally controlled string | test.py:48 | test.py:48:13:48:13 | b | |
21+
| Taint [[externally controlled string]] | test.py:47 | test.py:47:28:47:54 | Tuple | | --> | Taint externally controlled string | test.py:48 | test.py:48:16:48:16 | c | |
22+
| Taint [[externally controlled string]] | test.py:47 | test.py:47:28:47:54 | Tuple | | --> | Taint externally controlled string | test.py:48 | test.py:48:19:48:19 | d | |
23+
| Taint [[externally controlled string]] | test.py:47 | test.py:47:28:47:54 | Tuple | | --> | Taint externally controlled string | test.py:48 | test.py:48:22:48:22 | e | |
24+
| Taint [[externally controlled string]] | test.py:47 | test.py:47:28:47:54 | Tuple | | --> | Taint externally controlled string | test.py:48 | test.py:48:25:48:25 | f | |
425
| Taint [externally controlled string] | test.py:6 | test.py:6:9:6:20 | TAINTED_LIST | | --> | Taint [externally controlled string] | test.py:7 | test.py:7:15:7:15 | l | |
26+
| Taint [externally controlled string] | test.py:7 | test.py:7:15:7:15 | l | | --> | Taint externally controlled string | test.py:8 | test.py:8:10:8:10 | a | |
27+
| Taint [externally controlled string] | test.py:7 | test.py:7:15:7:15 | l | | --> | Taint externally controlled string | test.py:8 | test.py:8:13:8:13 | b | |
28+
| Taint [externally controlled string] | test.py:7 | test.py:7:15:7:15 | l | | --> | Taint externally controlled string | test.py:8 | test.py:8:16:8:16 | c | |
529
| Taint [externally controlled string] | test.py:12 | test.py:12:9:12:20 | TAINTED_LIST | | --> | Taint [externally controlled string] | test.py:13 | test.py:13:17:13:17 | l | |
30+
| Taint [externally controlled string] | test.py:13 | test.py:13:17:13:17 | l | | --> | Taint externally controlled string | test.py:14 | test.py:14:10:14:10 | a | |
31+
| Taint [externally controlled string] | test.py:13 | test.py:13:17:13:17 | l | | --> | Taint externally controlled string | test.py:14 | test.py:14:13:14:13 | b | |
32+
| Taint [externally controlled string] | test.py:13 | test.py:13:17:13:17 | l | | --> | Taint externally controlled string | test.py:14 | test.py:14:16:14:16 | c | |
633
| Taint [externally controlled string] | test.py:18 | test.py:18:9:18:20 | TAINTED_LIST | | --> | Taint [externally controlled string] | test.py:19 | test.py:19:11:19:11 | l | |
734
| Taint [externally controlled string] | test.py:18 | test.py:18:9:18:20 | TAINTED_LIST | | --> | Taint [externally controlled string] | test.py:19 | test.py:19:14:19:14 | l | |
835
| Taint [externally controlled string] | test.py:18 | test.py:18:9:18:20 | TAINTED_LIST | | --> | Taint [externally controlled string] | test.py:19 | test.py:19:17:19:17 | l | |

‎python/ql/test/library-tests/taint/unpacking/TestTaint.expected‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,33 @@
1-
| test.py:8 | unpacking | a | NO TAINT |
2-
| test.py:8 | unpacking | b | NO TAINT |
3-
| test.py:8 | unpacking | c | NO TAINT |
4-
| test.py:14 | unpacking_to_list | a | NO TAINT |
5-
| test.py:14 | unpacking_to_list | b | NO TAINT |
6-
| test.py:14 | unpacking_to_list | c | NO TAINT |
7-
| test.py:23 | nested | a1 | NO TAINT |
8-
| test.py:23 | nested | a2 | NO TAINT |
9-
| test.py:23 | nested | a3 | NO TAINT |
10-
| test.py:23 | nested | b | NO TAINT |
11-
| test.py:23 | nested | c | NO TAINT |
12-
| test.py:27 | nested | a1 | NO TAINT |
13-
| test.py:27 | nested | a2 | NO TAINT |
14-
| test.py:27 | nested | a3 | NO TAINT |
15-
| test.py:27 | nested | b | NO TAINT |
16-
| test.py:27 | nested | c | NO TAINT |
17-
| test.py:31 | nested | a1 | NO TAINT |
18-
| test.py:31 | nested | a2 | NO TAINT |
19-
| test.py:31 | nested | a3 | NO TAINT |
20-
| test.py:31 | nested | b | NO TAINT |
21-
| test.py:31 | nested | c | NO TAINT |
1+
| test.py:8 | unpacking | a | externally controlled string |
2+
| test.py:8 | unpacking | b | externally controlled string |
3+
| test.py:8 | unpacking | c | externally controlled string |
4+
| test.py:14 | unpacking_to_list | a | externally controlled string |
5+
| test.py:14 | unpacking_to_list | b | externally controlled string |
6+
| test.py:14 | unpacking_to_list | c | externally controlled string |
7+
| test.py:23 | nested | a1 | externally controlled string |
8+
| test.py:23 | nested | a2 | externally controlled string |
9+
| test.py:23 | nested | a3 | externally controlled string |
10+
| test.py:23 | nested | b | [externally controlled string] |
11+
| test.py:23 | nested | c | [externally controlled string] |
12+
| test.py:27 | nested | a1 | externally controlled string |
13+
| test.py:27 | nested | a2 | externally controlled string |
14+
| test.py:27 | nested | a3 | externally controlled string |
15+
| test.py:27 | nested | b | [externally controlled string] |
16+
| test.py:27 | nested | c | [externally controlled string] |
17+
| test.py:31 | nested | a1 | externally controlled string |
18+
| test.py:31 | nested | a2 | externally controlled string |
19+
| test.py:31 | nested | a3 | externally controlled string |
20+
| test.py:31 | nested | b | [externally controlled string] |
21+
| test.py:31 | nested | c | [externally controlled string] |
2222
| test.py:38 | unpack_from_set | a | NO TAINT |
2323
| test.py:38 | unpack_from_set | b | NO TAINT |
2424
| test.py:38 | unpack_from_set | c | NO TAINT |
25-
| test.py:48 | contrived_1 | a | NO TAINT |
26-
| test.py:48 | contrived_1 | b | NO TAINT |
27-
| test.py:48 | contrived_1 | c | NO TAINT |
28-
| test.py:48 | contrived_1 | d | NO TAINT |
29-
| test.py:48 | contrived_1 | e | NO TAINT |
30-
| test.py:48 | contrived_1 | f | NO TAINT |
25+
| test.py:48 | contrived_1 | a | externally controlled string |
26+
| test.py:48 | contrived_1 | b | externally controlled string |
27+
| test.py:48 | contrived_1 | c | externally controlled string |
28+
| test.py:48 | contrived_1 | d | externally controlled string |
29+
| test.py:48 | contrived_1 | e | externally controlled string |
30+
| test.py:48 | contrived_1 | f | externally controlled string |
3131
| test.py:56 | contrived_2 | a | NO TAINT |
3232
| test.py:56 | contrived_2 | b | NO TAINT |
3333
| test.py:56 | contrived_2 | c | NO TAINT |

0 commit comments

Comments
 (0)