Skip to content

Python: isType treats any object in a module named *typing as a type, corrupting points-to for unrelated containers #22621

Description

@mauvilsa

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:

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-all 4.1.0, codeql/python-queries 1.6.8
  • Also present on main as of 7a367c9

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions