Skip to content

Commit 96d1fc8

Browse files
committed
Python: Fix iterable-unpacking taint CP
When running ql/python/ql/src/Security/CWE-079/ReflectedXss.ql against the database for flask. Iitially there were 10 million result-tuples for iterable_unpacking_descent. With this change, we're down to roughly 2100,
1 parent 0f70da2 commit 96d1fc8

File tree

1 file changed

+42
-19
lines changed

1 file changed

+42
-19
lines changed

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

Lines changed: 42 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,45 @@ 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+
result = taint_at_depth(parent_kind.getMember(), depth-1)
968+
)
969+
}
970+
971+
/** Helper predicate for taintedMultiAssignment
972+
*
973+
* Returns the `depth` the elements that are assigned to `left_defn` with iterable unpacking has,
974+
* compared to `left_parent`. Special care is taken for `StarredNode` that is assigned a sequence of items.
975+
*
976+
* For example, `((x, *y), ...) = value` with any nesting on LHS
977+
* - with `left_defn` = `x`, `left_parent` = `(x, *y)`, result = 1
978+
* - with `left_defn` = `x`, `left_parent` = `((x, *y), ...)`, result = 2
979+
* - with `left_defn` = `*y`, `left_parent` = `(x, *y)`, result = 0
980+
* - with `left_defn` = `*y`, `left_parent` = `((x, *y), ...)`, result = 1
981+
*/
982+
int iterable_unpacking_descent(SequenceNode left_parent, ControlFlowNode left_defn) {
983+
exists(Assign a | a.getATarget().getASubExpression*().getAFlowNode() = left_parent) and
984+
left_parent.getAnElement() = left_defn and
985+
// Handle `a, *b = some_iterable`
986+
if left_defn instanceof StarredNode
987+
then result = 0
988+
else result = 1
989+
or
990+
result = 1 + iterable_unpacking_descent(left_parent.getAnElement(), left_defn)
991+
}
992+
970993
module Implementation {
971994
/* A call that returns a copy (or similar) of the argument */
972995
predicate copyCall(ControlFlowNode fromnode, CallNode tonode) {

0 commit comments

Comments
 (0)