Skip to content

Commit 261cd36

Browse files
authored
Merge pull request #781 from kevinbackhouse/HashedButNoHash
Python: fix false positive result.
2 parents bcc65db + 9e79e1b commit 261cd36

File tree

2 files changed

+44
-5
lines changed

2 files changed

+44
-5
lines changed

python/ql/src/Expressions/HashedButNoHash.ql

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import python
1616
* For sequences, the index must be an int, which are hashable, so we don't need to treat them specially.
1717
* For numpy arrays, the index may be a list, which are not hashable and needs to be treated specially.
1818
*/
19-
19+
2020
predicate numpy_array_type(ClassObject na) {
2121
exists(ModuleObject np | np.getName() = "numpy" or np.getName() = "numpy.core" |
2222
na.getAnImproperSuperType() = np.getAttribute("ndarray")
@@ -51,9 +51,30 @@ predicate is_unhashable(ControlFlowNode f, ClassObject cls, ControlFlowNode orig
5151
)
5252
}
5353

54+
/**
55+
* Holds if `f` is inside a `try` that catches `TypeError`. For example:
56+
*
57+
* try:
58+
* ... f ...
59+
* except TypeError:
60+
* ...
61+
*
62+
* This predicate is used to eliminate false positive results. If `hash`
63+
* is called on an unhashable object then a `TypeError` will be thrown.
64+
* But this is not a bug if the code catches the `TypeError` and handles
65+
* it.
66+
*/
67+
predicate typeerror_is_caught(ControlFlowNode f) {
68+
exists (Try try |
69+
try.getBody().contains(f.getNode()) and
70+
try.getAHandler().getType().refersTo(theTypeErrorType()))
71+
}
72+
5473
from ControlFlowNode f, ClassObject c, ControlFlowNode origin
55-
where
56-
explicitly_hashed(f) and is_unhashable(f, c, origin)
57-
or
58-
unhashable_subscript(f, c, origin)
74+
where
75+
not typeerror_is_caught(f)
76+
and
77+
(explicitly_hashed(f) and is_unhashable(f, c, origin)
78+
or
79+
unhashable_subscript(f, c, origin))
5980
select f.getNode(), "This $@ of $@ is unhashable.", origin, "instance", c, c.getQualifiedName()

python/ql/test/query-tests/Expressions/general/expressions_test.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,21 @@ def not_dup_key():
216216
u"😆" : 3
217217
}
218218

219+
# Lookup of unhashable object triggers TypeError, but the
220+
# exception is caught, so it's not a bug. This used to be
221+
# a false positive of the HashedButNoHash query.
222+
def func():
223+
unhash = list()
224+
try:
225+
hash(unhash)
226+
except TypeError:
227+
return 1
228+
return 0
229+
230+
def func():
231+
mapping = dict(); unhash = list()
232+
try:
233+
mapping[unhash]
234+
except TypeError:
235+
return 1
236+
return 0

0 commit comments

Comments
 (0)