Skip to content

fix: keep what sanitizers clear in function summaries - #138

Merged
SizzleUnrlsd merged 2 commits into
mainfrom
feat/summaries-keep-sanitizers
Sep 24, 2026
Merged

SizzleUnrlsd merged 2 commits into
mainfrom
feat/summaries-keep-sanitizers

Conversation

@SizzleUnrlsd

Copy link
Copy Markdown
Contributor

Part of #134, the sanitizer half. The validator half, for project validators, follows in its own PR.

Problem

A function summary recorded which parameters reach which external calls and the return value, but not what the calls on the way did to the data. When a caller passed request data to a helper that escapes it, or to a view that redirects to reverse(...), the taint engine mapped the full taint through the summary and reported the sink anyway. Only a sanitizer called directly in the flow's own function counted.

Change

  • What summaries record. Summaries keep, for each parameter, the taint kind bits cleared on every path from it:
    • to each external call's arguments (ExternalCall.argument_cleared, keyword_cleared);
    • to the return value (FunctionSummary.return_cleared).
      When two paths join, only what both cleared is kept, so a path around the sanitizer still reaches the sink. Summaries compose: a sanitizer two calls deep still protects.
  • Where the bits come from. A Clearing says what each call clears: a sanitizer clears its kinds, and a TemplateRender of an escaped template clears HTML. It is derived by ModelTable.clearing(escaped), the single place that rule lives. The taint engine uses it for direct calls, replacing its two separate checks. The engine provides it to the summaries (ClearingAnalysis), which stay free of the taint package. Without it, a summary clears nothing, which is conservative.
  • The taint engine removes the cleared kinds when it maps a callee's summary at a call site.
  • Cache format 9 → 10.
  • Not covered yet: stores into objects and nonlocal writes still clear nothing, which is conservative. This is documented.

Snapshot changes, reviewed (healthchecks only, 8 findings removed)

Finding Why it goes
hc/accounts/views.py:170 (login), :281 ×2 (check_token) open-redirect Both reach _check_2fa, which redirects to reverse(route, query=...); next only lands in the query string.
hc/accounts/views.py:637 ×2 (change_email_verify) open-redirect check_token(...) leads to the same reverse, one more call deep.
hc/api/views.py:778 (badge), :811 (check_badge) xss HttpResponse(get_badge_svg(...)): the helper renders badge.svg, which prints tag/status only in text and quoted attributes, so it is established as escaped.
hc/integrations/po/views.py:59 (add) open-redirect Redirects to settings.PUSHOVER_SUBSCRIPTION_URL + "?" + urlencode(...). The POST data only enters through absolute_reverse → reverse, in the query.

Verification

  • pytest: 1073 passed. ruff check src tests and mypy are clean.
  • pytest -m regression under Python 3.12: 19 passed. Only healthchecks.json changed, by the 8 findings above.
  • Tests. The new tests were red for the right reasons: XSS and open-redirect reported through helpers, and Dep had no cleared. The two guard tests, a path around the sanitizer and a sanitizer clearing only its own kinds, were green before and after.
  • Mutation checks. Each of these changes fails at least one test: summaries ignoring sanitizers, a join keeping any path, no composition across calls, the taint engine ignoring cleared return values or arguments, and a project clearing built without the escaped templates.

@SizzleUnrlsd SizzleUnrlsd self-assigned this Sep 24, 2026
@SizzleUnrlsd
SizzleUnrlsd merged commit 2db93a0 into main Sep 24, 2026
10 checks passed
@SizzleUnrlsd
SizzleUnrlsd deleted the feat/summaries-keep-sanitizers branch September 24, 2026 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant