hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-01 11:45:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Python: Don't report mutable parameters that are in fact immutable.

Browse Source 
Fixes #1832.

In the taint sink, we add an additional check that the given control-flow node
can indeed point to a value that is mutable. This takes care of the guard on the
type.

If and when we get around to adding configurations for all of the taint
analyses, we may want to implement this as a barrier instead, pruning any steps
that go through a type test where the type is not mutable.
This commit is contained in:
Taus Brock-Nannestad
2019-11-18 16:18:44 +01:00
parent ed4657c201
commit cac261858c
3 changed files with 42 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
python/ql/src/Functions/ModificationOfParameterWithDefault.ql
View File

@@ -73,6 +73,11 @@ class MutableDefaultValue extends TaintSource {
}
}
private ClassValue mutable_class() {
result = Value::named("list") or
result = Value::named("dict")
}
class Mutation extends TaintSink {
Mutation() {
exists(AugAssign a | a.getTarget().getAFlowNode() = this)
@@ -80,7 +85,8 @@ class Mutation extends TaintSink {
exists(Call c, Attribute a |
c.getFunc() = a |
a.getObject().getAFlowNode() = this and
not safe_method(a.getName())
not safe_method(a.getName()) and
this.(ControlFlowNode).pointsTo().getClass() = mutable_class()
)
}

10
python/ql/test/query-tests/Functions/general/ModificationOfParameterWithDefault.expected
View File

@@ -7,8 +7,18 @@ edges
| functions_test.py:157:27:157:27 | empty mutable value | functions_test.py:159:21:159:21 | empty mutable value |
| functions_test.py:158:25:158:25 | empty mutable value | functions_test.py:151:25:151:25 | empty mutable value |
| functions_test.py:159:21:159:21 | empty mutable value | functions_test.py:154:21:154:21 | empty mutable value |
| functions_test.py:175:28:175:28 | non-empty mutable value | functions_test.py:179:9:179:9 | non-empty mutable value |
| functions_test.py:175:28:175:28 | non-empty mutable value | functions_test.py:181:9:181:9 | non-empty mutable value |
| functions_test.py:188:18:188:18 | non-empty mutable value | functions_test.py:189:28:189:28 | non-empty mutable value |
| functions_test.py:189:28:189:28 | non-empty mutable value | functions_test.py:175:28:175:28 | non-empty mutable value |
| functions_test.py:191:18:191:18 | non-empty mutable value | functions_test.py:192:28:192:28 | non-empty mutable value |
| functions_test.py:192:28:192:28 | non-empty mutable value | functions_test.py:175:28:175:28 | non-empty mutable value |
#select
| functions_test.py:40:5:40:5 | x | functions_test.py:39:9:39:9 | empty mutable value | functions_test.py:40:5:40:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:39:9:39:9 | x | Default value |
| functions_test.py:134:5:134:5 | x | functions_test.py:133:15:133:15 | empty mutable value | functions_test.py:134:5:134:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:133:15:133:15 | x | Default value |
| functions_test.py:152:5:152:5 | x | functions_test.py:157:27:157:27 | empty mutable value | functions_test.py:152:5:152:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:157:27:157:27 | y | Default value |
| functions_test.py:155:5:155:5 | x | functions_test.py:157:27:157:27 | empty mutable value | functions_test.py:155:5:155:5 | empty mutable value | $@ flows to here and is mutated. | functions_test.py:157:27:157:27 | y | Default value |
| functions_test.py:179:9:179:9 | x | functions_test.py:188:18:188:18 | non-empty mutable value | functions_test.py:179:9:179:9 | non-empty mutable value | $@ flows to here and is mutated. | functions_test.py:188:18:188:18 | x | Default value |
| functions_test.py:179:9:179:9 | x | functions_test.py:191:18:191:18 | non-empty mutable value | functions_test.py:179:9:179:9 | non-empty mutable value | $@ flows to here and is mutated. | functions_test.py:191:18:191:18 | x | Default value |
| functions_test.py:181:9:181:9 | x | functions_test.py:188:18:188:18 | non-empty mutable value | functions_test.py:181:9:181:9 | non-empty mutable value | $@ flows to here and is mutated. | functions_test.py:188:18:188:18 | x | Default value |
| functions_test.py:181:9:181:9 | x | functions_test.py:191:18:191:18 | non-empty mutable value | functions_test.py:181:9:181:9 | non-empty mutable value | $@ flows to here and is mutated. | functions_test.py:191:18:191:18 | x | Default value |

25
python/ql/test/query-tests/Functions/general/functions_test.py
View File

@@ -168,3 +168,28 @@ def issue1143(expr, param=[]):
return result
for i in param:
param.remove(i) # Mutation here
# Type guarding of modification of parameter with default:
def do_stuff_based_on_type(x):
if isinstance(x, str):
x = x.split()
elif isinstance(x, dict):
x.setdefault('foo', 'bar')
elif isinstance(x, list):
x.append(5)
elif isinstance(x, tuple):
x = x.unknown_method()
def str_default(x="hello world"):
do_stuff_based_on_type(x)
def dict_default(x={'baz':'quux'}):
do_stuff_based_on_type(x)
def list_default(x=[1,2,3,4]):
do_stuff_based_on_type(x)
def tuple_default(x=(1,2)):
do_stuff_based_on_type(x)