fix: count a guard only when it examines what the sink receives

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Precision
 
+- A dominating guard makes a flow a hotspot only when it examines what the sink receives. `if request.method == 'POST':` before `request.POST['cmd']` no longer lowers a vulnerability to a hotspot. A test of the value itself, of the same attribute, of a method called on it (`form.is_valid()`), or of any part of an object the flow passes whole still does. The regression snapshots record the verdict of each taint finding (#135).
 - Django's `csrf.get_token` returns a token of letters and digits whatever the request holds, and `reverse`/`reverse_lazy` build a local path that attacker data in their arguments or query cannot turn into another host: the first is no longer an injection, the second no longer an open redirect. `reverse` still carries other kinds, since it leaves `'` unquoted (#132).
 - The SSRF sinks of Requests and httpx read the destination only: the URL (the first argument, the second after the method for `request` and `stream`, or `url=`) or the prepared request `send` takes. Attacker data in the body, the JSON, the headers or the query parameters is no longer a server-side request forgery. `Sink` gains `keywords` next to `positions`, and function summaries keep the name of each keyword argument, so a project function forwarding `url=` still reaches the sink; the cache format is bumped (#128).
 - `render_to_string` escapes what it renders where the analyzer can establish it: on a directory check, it reads the project's Django templates, and a template escapes when neither it nor what it extends or includes marks output safe, uses a tag, filter or library beyond Django's own, or prints a variable where HTML escaping does not protect it, and no settings turn autoescaping off. Data reaching a response through such a template is no longer `xss`; any other template keeps the flow reported. `TemplateRender` declares such rendering calls, and the escaped templates are part of the cache key (#130).

docs/usage.md

@@ -118,9 +118,19 @@ a sanitizer for that kind of sink.
 Each flow is judged before it is reported. A dominating guard that proves the value
 safe (`isdigit()`, membership in a constant collection, equality with a constant, a
 numeric value proven by `int()`, `len()` or bounded arithmetic, a validator such as
-`re.fullmatch`) refutes it and nothing is reported. A guard that only mentions the value,
-or an authorization decorator such as `login_required`, makes it a hotspot reported at
-medium confidence. The verdict and its evidence are in the finding's metadata.
+`re.fullmatch`) refutes it and nothing is reported. A hotspot is reported at medium
+confidence. Two things make a flow a hotspot:
+
+- A guard that examines the data without proving it safe. That covers a test of the value
+  itself (`if cmd:`, `len(cmd) < 10`), of the same attribute, or of a method called on
+  it (`form.is_valid()`).
+- An authorization decorator such as `login_required`.
+
+A guard that reads another attribute of the object the data comes from is no guard of
+that data. `if request.method == 'POST':` before `os.system(request.POST['cmd'])` leaves
+a vulnerability. When the flow passes the object whole, to the sink or to a function,
+a guard on any of its attributes still counts. The verdict and its evidence are in the
+finding's metadata.
 
 Django's `render_to_string(name, context)` returns HTML in which autoescaping escaped
 every variable, so data reaching a response through it is no `xss`, if the template really

src/coretrace_python/findings/refutation.py

@@ -8,10 +8,13 @@
 one of its arguments, a numeric value (``abstract.ranges``) cannot inject, and anything
 else that mentions the value is a guard that does not prove it. A proof counts for an
 origin when every dependence path from that origin to the sink argument goes through a
-proven value. A flow is refuted when every tainted origin is proven safe or the sink is
-unreachable, a hotspot when it sits behind an ``AuthorizationGuard`` (a decorator or a
-dominating condition) or when an unproven guard mentions it, and a vulnerability
-otherwise.
+proven value. A guard mentions an origin only when it examines what the sink receives:
+reading another attribute or item of an object the flow reads (``request.method`` next
+to ``request.POST``) examines nothing of it, while the same attribute, or a method called
+on the object (``form.is_valid()``), does. A flow is refuted when every tainted origin is
+proven safe or the sink is unreachable, a hotspot when it sits behind an
+``AuthorizationGuard`` (a decorator or a dominating condition) or when an unproven guard
+mentions it, and a vulnerability otherwise.
 """
 
 from __future__ import annotations
@@ -37,6 +40,7 @@
     Constant,
     FunctionIR,
     GetAttr,
+    GetItem,
     Instruction,
     UnaryOp,
     Value,
@@ -267,6 +271,66 @@ def recognise(self, definition: Instruction | None) -> tuple[Value, str, bool] |
                         return tested, "equals a constant", definition.operator == "eq"
         return None
 
+    def examined(
+        self, condition: Value, mentioned: set[Value], argument: Value, chain: frozenset[Value]
+    ) -> set[Value]:
+        """What the guard examines of the values it mentions: everything its condition is
+        computed from, except an object it only reads another attribute or item of than
+        the flow reads (``request`` in ``request.method`` when the sink receives
+        ``request.POST['cmd']``). A method called on the object (``form.is_valid()``), the
+        same attribute or item, or any part of an object the flow takes whole (passes to
+        the sink or to a call), examines it."""
+
+        reads = {self.access(value) for value in chain} - {None}
+        whole = {argument} | {
+            operand
+            for value in chain
+            if (definition := self.defs.get(value)) is not None
+            for operand in definition.operands()
+            if not (isinstance(definition, GetAttr | GetItem) and definition.object == operand)
+        }
+        found: set[Value] = set()
+        called: set[Value] = set()
+        pending = [condition]
+        while pending:
+            value = pending.pop()
+            if value in found:
+                continue
+            found.add(value)
+            definition = self.defs.get(value)
+            if definition is None:
+                continue
+            if isinstance(definition, Call):
+                called.add(definition.callee)
+            access = None if value in called else self.access(value)
+            for operand in definition.operands():
+                if access is not None and access[0] == operand and operand not in whole and self.aside(access, reads):
+                    continue
+                pending.append(operand)
+        return found & mentioned
+
+    @staticmethod
+    def aside(access: tuple[Value, str, object], reads: set[tuple[Value, str, object] | None]) -> bool:
+        """Whether reading ``access`` reads beside what the flow reads of the same object;
+        an item under a key unknown on either side may be the same one."""
+
+        owner, kind, name = access
+        if kind == "item" and (name is None or (owner, "item", None) in reads):
+            return False
+        return access not in reads
+
+    def access(self, value: Value) -> tuple[Value, str, object] | None:
+        """``(object, "attribute" or "item", name or constant key)`` for a read of an
+        attribute or an item; ``None`` for any other value, and as the key when unknown."""
+
+        definition = self.defs.get(value)
+        if isinstance(definition, GetAttr):
+            return definition.object, "attribute", definition.attribute
+        if isinstance(definition, GetItem):
+            key = self.defs.get(definition.key)
+            return definition.object, "item", key.value if isinstance(key, Constant) else None
+        return None
+
     def is_constant_collection(self, value: Value) -> bool:
         definition = self.defs.get(value)
         if isinstance(definition, Constant):
@@ -326,8 +390,9 @@ def judge(self, flow: TaintFlow, reachable: bool) -> Verdict:
             for value, reason in found.items():
                 if value in chain:
                     proven.setdefault(value, reason)
+            examined = self.examined(guard.condition, mentioned, flow.argument, chain)
             for origin in origins:
-                if origin not in mentions and mentioned & (
+                if origin not in mentions and examined & (
                     {origin} | {w for w in chain if origin in self.closure(w)}
                 ):
                     mentions[origin] = guard.line

tests/regression/expected/Chatbot.json

@@ -16,7 +16,8 @@
       "file": "setup.py",
       "line": 48,
       "rule": "command-injection",
-      "function": "install_ollama"
+      "function": "install_ollama",
+      "verdict": "vulnerability"
     }
   ]
 }

tests/regression/expected/DjangoGoat.json

@@ -10,7 +10,8 @@
       "file": "authentication/views.py",
       "line": 57,
       "rule": "sql-injection",
-      "function": "log_in"
+      "function": "log_in",
+      "verdict": "vulnerability"
     },
     {
       "file": "djangogoat/settings.py",

tests/regression/expected/Flask-Image-Editing-Website.json

@@ -22,7 +22,8 @@
       "file": "main.py",
       "line": 79,
       "rule": "exploitable-vulnerability",
-      "function": "edit"
+      "function": "edit",
+      "verdict": "hotspot"
     },
     {
       "file": "main.py",

tests/regression/expected/InsecureGram.json

@@ -10,13 +10,15 @@
       "file": "app/admin.py",
       "line": 45,
       "rule": "insecure-deserialization",
-      "function": "insecure_deserialize"
+      "function": "insecure_deserialize",
+      "verdict": "vulnerability"
     },
     {
       "file": "app/command.py",
       "line": 12,
       "rule": "command-injection",
-      "function": "execute"
+      "function": "execute",
+      "verdict": "vulnerability"
     },
     {
       "file": "app/config.py",
@@ -34,25 +36,29 @@
       "file": "app/redirect.py",
       "line": 11,
       "rule": "open-redirect",
-      "function": "redirect"
+      "function": "redirect",
+      "verdict": "vulnerability"
     },
     {
       "file": "app/ssrf.py",
       "line": 12,
       "rule": "ssrf",
-      "function": "proxy"
+      "function": "proxy",
+      "verdict": "vulnerability"
     },
     {
       "file": "app/upload.py",
       "line": 36,
       "rule": "path-traversal",
-      "function": "upload"
+      "function": "upload",
+      "verdict": "hotspot"
     },
     {
       "file": "app/users.py",
       "line": 36,
       "rule": "sql-injection",
-      "function": "search_users"
+      "function": "search_users",
+      "verdict": "hotspot"
     }
   ]
 }

tests/regression/expected/MiniBookApiServer.json

@@ -10,13 +10,15 @@
       "file": "app.py",
       "line": 69,
       "rule": "path-traversal",
-      "function": "get_img"
+      "function": "get_img",
+      "verdict": "vulnerability"
     },
     {
       "file": "app.py",
       "line": 100,
       "rule": "exploitable-vulnerability",
-      "function": "new_book"
+      "function": "new_book",
+      "verdict": "hotspot"
     },
     {
       "file": "config-example.json",

tests/regression/expected/Vulnerable_fastapi.json

@@ -10,13 +10,15 @@
       "file": "app/routers/auth.py",
       "line": 37,
       "rule": "sql-injection",
-      "function": "login"
+      "function": "login",
+      "verdict": "vulnerability"
     },
     {
       "file": "app/routers/files.py",
       "line": 27,
       "rule": "path-traversal",
-      "function": "download"
+      "function": "download",
+      "verdict": "hotspot"
     },
     {
       "file": "exploit_scripts/brute_force_login.py",
@@ -118,7 +120,8 @@
       "file": "exploit_scripts/stored_xss_profile.py",
       "line": 24,
       "rule": "ssrf",
-      "function": "<module>"
+      "function": "<module>",
+      "verdict": "vulnerability"
     },
     {
       "file": "exploit_scripts/stored_xss_profile.py",
@@ -130,7 +133,8 @@
       "file": "exploit_scripts/stored_xss_profile.py",
       "line": 28,
       "rule": "ssrf",
-      "function": "<module>"
+      "function": "<module>",
+      "verdict": "vulnerability"
     }
   ]
 }

tests/regression/expected/dvpwa.json

@@ -34,7 +34,8 @@
       "file": "sqli/views.py",
       "line": 57,
       "rule": "sql-injection",
-      "function": "students"
+      "function": "students",
+      "verdict": "vulnerability"
     }
   ]
 }

tests/regression/expected/full-stack-fastapi-template.json

@@ -40,7 +40,8 @@
       "file": "backend/app/api/routes/login.py",
       "line": 121,
       "rule": "xss",
-      "function": "recover_password_html_content"
+      "function": "recover_password_html_content",
+      "verdict": "hotspot"
     },
     {
       "file": "backend/tests/api/routes/test_login.py",