Python: `isType` treats any object in a module named `*typing` as a type, corrupting points-to for unrelated containers
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 3/5
- Tiempo estimado
- 1-2 días
- Aptitud para principiantes
- 68/100
Línea de trabajo
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.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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
- Lenguaje dominante
- CodeQL
- Estrellas
- 10.1k
- Forks
- 2.1k
- Merge medio
- 2 d 11 h
- PR fusionados (30 d)
- 129
Guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de github/codeql
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 84/100
-
C#: cs/simplifiable-boolean-expression false positive on Nullable<bool> compared with a literal Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
-
false-positive
Dificultad 2/5 1-3 horas Aptitud para principiantes 70/100
-
Dificultad 4/5 3-5 días Aptitud para principiantes 55/100
Todos los issues de github/codeql
Issues similares
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
use-agent-os/agent-os#3314 ·
-
Implement GitHub Issue templates Abiertodocumentation low-priority templates
Dificultad 2/5 1-3 horas Aptitud para principiantes 85/100
jesseray718/openroot#87 ·
-
factory-active factory-automatic task-bug-reproduction-cannot-reproduce task-identify-harness-labels-done task-identify-issue-type-done
Dificultad 2/5 1-3 horas Aptitud para principiantes 84/100
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 90/100
danielmiessler/LifeOS#2218 ·
-
in-progress proposal
Dificultad 1/5 Menos de una hora Aptitud para principiantes 88/100
uibcdf/molsyssuite#36 ·