Skip to content

SDG: parameter passed as an argument is wired from CFG ENTRY instead of formal_in, breaking all interprocedural value flow #220

Description

@rahlk

Summary

At -a 4, a value passed from a parameter into a call is not linked across the call
boundary. Every interprocedural value query therefore returns a negative answer, and
taint() reports a clean exhausted for a flow that plainly exists. A caller who
trusts exhausted as a refutation will close a live finding.

Reproducer

queries.py:

def sanitize(raw):
    return raw.strip()


def build_query(user_input, limit):
    name = sanitize(user_input)
    if limit > 100:
        limit = 100
    return f"SELECT * FROM users WHERE name = '{name}' LIMIT {limit}"


def handle(request):
    return build_query(request, 500)
from cldk import CLDK
from cldk.analysis import AnalysisLevel

a = CLDK.python(project_path=".", analysis_level=AnalysisLevel.system_dependency_graph)

a.slice_forward("request", within="handle").total   # 4   (expected: crosses into callees)
a.flows_to_call("request", "build_query", within="handle")   # False  (expected True)
a.paths_between("request", "raw", src_within="handle", dst_within="sanitize")   # []

r = a.taint(sources=[("request", "handle")], sinks=[("raw", "sanitize")])
r.paths        # []
r.exhausted    # [('request', 'raw')]   <-- a refutation for a real flow
r.complete     # True
r.unresolved   # []

The identical fixture in TypeScript (codeanalyzer-typescript 1.6.0) answers correctly:
flows_to_call is True and paths_between returns one path with hops
('data','userInput',['reaching-defs']) then ('argument','raw',[]).

Versions: codeanalyzer-python 1.5.2 and 1.5.3, cldk 2.0.0rc7.

Root cause

The SDG edges themselves are correct and complete. application.param_in holds
BQ@6:4/actual_in:0 -> SAN@formal_in:0 and H@13:4/actual_in:0 -> BQ@formal_in:0,
and the summary edges are present. The break is intraprocedural, in the DDG.

codeanalyzer/dataflow/sdg.py:

  • build_formals (the # Wiring: formal_in -> first uses block) remaps DDG edges whose
    source is the synthetic CFG ENTRY node onto the matching formal_in vertex. Reaching-def
    analysis names ENTRY as the definition site of every parameter, capture and read global,
    so this remap is what puts a parameter's definition on its formal_in port.
  • build_actuals runs after build_formals, and allocates the actual_in ports there.
    It wires each port from _defs_reaching_call_matching(...), which returns the raw
    reaching-def sources. For a parameter that source is ENTRY, and it is never remapped.

So the graph gets ENTRY -> actual_in:N where it needs formal_in:N -> actual_in:N:

handle:
  H@formal_in:0 -> H@13:4              var=request    <- parameter reaches the statement
  H@entry       -> H@13:4/actual_in:0  var=request    <- but the ARGUMENT PORT hangs off ENTRY
  H@13:4/actual_out:1 -> H@13:4        var=user_input <- the return direction is wired correctly

resolve_value("request", within="handle") returns formal_in:0, so a forward walk starts
at a vertex with no path to actual_in:0, never crosses PARAM_IN, and stops inside the
caller. The asymmetry with the correctly-wired return direction is the tell.

Fix

Extract the ENTRY-to-formal_in lookup from build_formals into a formal_for(var) helper
and apply it in build_actuals when the reaching-def source is the CFG entry id, for both
the argument ports and the global-read ports. Fall back to the original source when the
variable is not one of the callable's formals, so nothing is lost.

After the fix

slice_forward("request", within="handle").total   # 21
flows_to_call("request", "build_query", ...)      # True
flows_to_argument("request", "build_query", "user_input", ...)  # True
paths_between("request", "raw", ...)              # one path, two call boundaries:
#   ('data','request',['reaching-defs']) -> ('argument','user_input',[])
#   -> ('data','user_input',['reaching-defs']) -> ('argument','raw',[])
taint(...).paths                                   # the same path
taint(...).exhausted                               # []

Full suite: 517 passed, 6 skipped. A regression test is added to
test/test_dataflow_sdg.py that asserts an actual_in port carrying a parameter is fed
from formal_in, never from ENTRY. It fails on 1.5.3 and passes with the fix.

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

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions