Python: `isType` treats any object in a module named `*typing` as a type, corrupting points-to for unrelated containers
Personne n'a encore pris cette issue.
Évaluation
- Difficulté
- 3/5
- Temps estimé
- 1-2 jours
- Accessibilité débutants
- 68/100
Piste de recherche
Start in python/ql/lib/semmle/python/objects/TObject.qll at isType and review how it feeds TSubscriptedType, then run NonCallableCalled.ql and HashedButNoHash.ql against the four-file reproduction. Compare results for pkg/typing.py and pkg/other.py, and verify that the false positives disappear without breaking handling of typing and typing_extensions modules.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Description
Disclaimer
The analysis below was done by a coding agent. I tried to verify it up to my knowledge, but there could be inaccuracies. Nevertheless, the false positive is real and it has been affecting jsonargparse for a long time. Just decided that since I have access to a good coding agent I could analyze the issue and report it here.
Description of the false positive
py/call-to-non-callable and py/hash-unhashable-value fire on correct code purely because of the filename of the module the code lives in.
The cause is the second disjunct of isType in python/ql/lib/semmle/python/objects/TObject.qll#L215-L220:
predicate isType(ObjectInternal t) {
t.isClass() = true
or
t.getOrigin().getEnclosingModule().getName().matches("%typing")
}
isType gates the construction of TSubscriptedType (TObject.qll#L200-L205), the model of a subscripted generic such as List[int]. Because the disjunct matches on module name rather than on anything about the object, every object defined in a module whose dotted name ends in typing is classified as a type. Subscripting an ordinary module-level dict in such a module therefore produces a SubscriptedTypeInternal, whose getClass() returns the generic's class (Classes.qll#L332):
override ObjectInternal getClass() { result = this.getGeneric().getClass() }
So my_registry[key] is inferred to be an instance of dict. Any function that returns such a lookup is then inferred to return a dict, and every downstream use of its result is flagged.
Note the alert message is quite misleading in this situation. On a class returned from a registry lookup, the user sees "Call to a non-callable of builtin-class dict" — the dict is the registry, not anything related to the value, and nothing in the message text says so. The origin in relatedLocations is the only clue, and it points at the registry lookup, often in a different file from the alert.
Code samples or links to source code
Self-contained reproduction. Four files; pkg/other.py is byte-identical to pkg/typing.py apart from the identifier names, and is there purely to isolate the filename as the trigger.
pkg/__init__.py (empty)
pkg/typing.py:
registry = {}
def make_type(name):
if name in registry:
return registry[name]
class Created(str):
pass
registry[name] = Created
return Created
InTyping = make_type("InTyping")
pkg/other.py: identical, with InTyping renamed to InOther.
main.py:
from typing import Optional
from pkg.other import InOther
from pkg.typing import InTyping
InTyping("a") # py/call-to-non-callable <- false positive
Optional[InTyping] # py/hash-unhashable-value <- false positive
InOther("b") # no alert
Optional[InOther] # no alert
Running NonCallableCalled.ql and HashedButNoHash.ql from codeql/python-queries 1.6.8 (CLI 2.23.5) gives:
main.py:6 Non-callable called Call to a non-callable of builtin-class dict.
related location: pkg/typing.py:6:16:6:29
main.py:7 Unhashable object hashed This instance of dict is unhashable.
related location: pkg/typing.py:6:16:6:29
Both alerts are on uses of InTyping; the identical InOther uses produce nothing. The relatedLocation on both is pkg/typing.py:6, the registry[name] lookup.
This is not hypothetical. It is the cause of a long-standing batch of dismissed alerts in jsonargparse, which has a public jsonargparse.typing module:
- The registry:
jsonargparse/typing.py#L70 - The lookup that leaks:
jsonargparse/typing.py#L414-L415
Every py/call-to-non-callable and py/hash-unhashable-value alert in that repository traces, via relatedLocations, to that single line — including alerts in test files that merely call a path type such as Path_fr(...) or write Optional[Path_fr].
One further observation that may be useful when triaging severity: whether the corruption surfaces as an alert depends on unrelated downstream details. The same project has a second lookup of the same registry at typing.py#L248-L252 that produces the same bogus Dict[...] value, but it never escapes the function: the next statement reads registered_type.__name__, and since SubscriptedTypeInternal declares both attribute() { none() } and attributesUnknown() { none() }, the attribute lookup definitively fails and the following return becomes unreachable. Probing that return yields no points-to tuples at all. So the bad inference is present at both sites and happens to be masked at one of them.
Suggested fix
Restricting the disjunct to the actual typing modules removes the false positives:
predicate isType(ObjectInternal t) {
t.isClass() = true
or
exists(string name | name = t.getOrigin().getEnclosingModule().getName() |
name = "typing" or name = "typing_extensions" or name.matches("typing.%")
)
}
With that change, on the reproduction above both alerts go to zero. On a database built from jsonargparse at the commit linked above, py/call-to-non-callable goes from 9 results to 0 and py/hash-unhashable-value from 11 to 0. Those before-counts are exactly what code scanning itself reported for that commit on main (9 and 11), so the narrowing clears the full set of false positives in that project.
Open question
I could not construct a case where the current disjunct actually helps, so I may be missing its intended purpose. In every database I built the stdlib typing module is not extracted at all — querying for modules matching %typing returned only the project's own jsonargparse.typing and jsonargparse_tests.test_typing. If the disjunct exists to model typing.List and friends via extracted stdlib source, a maintainer will know whether the narrowing above preserves that; if it only ever matches user modules that happen to be named *typing, it may be removable outright.
URL to the alert on GitHub code scanning (optional)
The affected alerts are dismissed as false positives in a public repository, e.g. https://github.com/mauvilsa/jsonargparse/security/code-scanning/617 — raised on path = Path_fr(file_r), where Path_fr is a class. (Code scanning alerts are only visible to users with write access even on public repositories, so that link may not be usable from outside the project; the reproduction above stands on its own.)
Environment
codeql-bundle-v2.23.5— CodeQL CLI 2.23.5,codeql/python-all4.1.0,codeql/python-queries1.6.8- Also present on
mainas of 7a367c9b70e215e4e6373bbfd3164d4f44bcc11d
- Langage dominant
- CodeQL
- Étoiles
- 10.1k
- Forks
- 2.1k
- Merge moyen
- 2 j 11 h
- PR mergées (30 j)
- 129
Guide de contribution
Ouvrir le guide de contribution
Par où commencer
- Lisez l'issue en entier, puis le guide de contribution du projet.
- Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
- Forkez le dépôt et travaillez sur une branche.
- Ouvrez une pull request qui référence le numéro de l'issue.
Autres issues de github/codeql
-
Difficulté 2/5 1-3 heures Accessibilité débutants 84/100
-
C#: cs/simplifiable-boolean-expression false positive on Nullable<bool> compared with a literal Ouverte
Difficulté 2/5 1-3 heures Accessibilité débutants 82/100
-
Difficulté 2/5 1-3 heures Accessibilité débutants 78/100
-
false-positive
Difficulté 2/5 1-3 heures Accessibilité débutants 70/100
-
False positive Ouvertefalse-positive
Difficulté 4/5 3-5 jours Accessibilité débutants 15/100
Toutes les issues de github/codeql
Issues similaires
-
스택 PR 머지 시 하위 PR base 재지정 단계 부재 Ouverte
Difficulté 2/5 1-3 heures Accessibilité débutants 78/100
idean3885/claude-ops-agent#521 ·
-
Difficulté 2/5 1-3 heures Accessibilité débutants 72/100
0xMiden/bridge-portal#132 ·
-
bug
Difficulté 2/5 1-3 heures Accessibilité débutants 84/100
newrelic-experimental/preflight#793 · 1 commentaire ·
-
enhancement
Difficulté 2/5 1-3 heures Accessibilité débutants 70/100
babalae/bettergi-scripts-list#3674 ·
-
Difficulté 2/5 1-3 heures Accessibilité débutants 88/100
caddyserver/caddy#8046 ·