Skip to content

Commit 5bff518

Browse files
committed
Python: switch from negative to positive list
This should avoid potentially terrible performance. Also noted the missing syntactic constructs, as I went through the documnetation.
1 parent e865a29 commit 5bff518

File tree

1 file changed

+37
-19
lines changed

1 file changed

+37
-19
lines changed

python/ql/src/semmle/python/functions/ModificationOfParameterWithDefaultCustomizations.qll

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -57,33 +57,51 @@ module ModificationOfParameterWithDefault {
5757
MutableDefaultValue() { nonEmpty = mutableDefaultValue(this.asCfgNode().(NameNode).getNode()) }
5858
}
5959

60-
predicate safe_method(string name) {
61-
name = "count" or
62-
name = "index" or
63-
name = "copy" or
64-
name = "get" or
65-
name = "has_key" or
66-
name = "items" or
67-
name = "keys" or
68-
name = "values" or
69-
name = "iteritems" or
70-
name = "iterkeys" or
71-
name = "itervalues" or
72-
name = "__contains__" or
73-
name = "__getitem__" or
74-
name = "__getattribute__"
60+
/**
61+
* A name of a list function that modifies the list.
62+
* See https://docs.python.org/3/tutorial/datastructures.html#more-on-lists
63+
*/
64+
string list_modifying_method() {
65+
result in ["append", "extend", "insert", "remove", "pop", "clear", "sort", "reverse"]
7566
}
7667

7768
/**
78-
* A mutation is considered a flow sink.
69+
* A name of a dict function that modifies the dict.
70+
* See https://docs.python.org/3/library/stdtypes.html#dict
71+
*/
72+
string dict_modifying_method() { result in ["clear", "pop", "popitem", "setdefault", "update"] }
73+
74+
/**
75+
* A mutation of the default value is a flow sink.
76+
*
77+
* Syntactic constructs that modify a list are:
78+
* - s[i] = x
79+
* - s[i:j] = t
80+
* - del s[i:j]
81+
* - s[i:j:k] = t
82+
* - del s[i:j:k]
83+
* - s += t
84+
* - s *= n
85+
* See https://docs.python.org/3/library/stdtypes.html#mutable-sequence-types
86+
*
87+
* Syntactic constructs that modify a dictionary are:
88+
* - d[key] = value
89+
* - del d[key]
90+
* - d |= other
91+
* See https://docs.python.org/3/library/stdtypes.html#dict
92+
*
93+
* These are all covered by:
94+
* - assignment to a subscript (includes slices)
95+
* - deletion of a subscript
96+
* - augmented assignment to the value
7997
*/
8098
class Mutation extends Sink {
8199
Mutation() {
82100
exists(AugAssign a | a.getTarget().getAFlowNode() = this.asCfgNode())
83101
or
84-
exists(Call c, Attribute a | c.getFunc() = a |
85-
a.getObject().getAFlowNode() = this.asCfgNode() and
86-
not safe_method(a.getName())
102+
exists(DataFlow::CallCfgNode c, DataFlow::AttrRead a | c.getFunction() = a |
103+
a.getObject() = this and
104+
a.getAttributeName() in [list_modifying_method(), dict_modifying_method()]
87105
)
88106
}
89107
}

0 commit comments

Comments
 (0)