Skip to content

fix(security) - parse meta-refresh directives and commit Head() meta … - #58

Merged
hexplus merged 3 commits into
mainfrom
chore/57-framework-hardening
Aug 28, 2026
Merged

fix(security) - parse meta-refresh directives and commit Head() meta …#58
hexplus merged 3 commits into
mainfrom
chore/57-framework-hardening

Conversation

@hexplus

@hexplus hexplus commented Aug 27, 2026

Copy link
Copy Markdown
Owner

Description

Four defects in <meta http-equiv="refresh"> handling, found after #57.

Detection substring-matched a grammar. Dangerous destinations were found by asking whether the lower-cased content contained url=javascript: (plus three sibling schemes). That recognises exactly one spelling of a directive the browser's refresh parser accepts in many — every one of these was a live redirect the check never saw:

0; url = javascript:alert(1)        whitespace around the separator and `=`
0;URL=JAVASCRIPT:alert(1)           mixed-case key and scheme
0;url='javascript:alert(1)'         quoted destination
0;\turl\t=\tjavascript:alert(1)     tabs

Pattern-matching a grammar is a losing position: each new spelling needs a new pattern, and the attacker picks the spelling. The destination is now extracted structurally and handed to sanitizeUrl() — the same protocol authority every other URL sink uses — so adding a scheme to that allowlist fixes this sink automatically.

Reactive attributes were validated independently. Head() created one effect per reactive attribute, so every write was judged alone. That lets the combination become dangerous while no individual write ever looks wrong:

{ "http-equiv": () => equiv(), content: "0;url=javascript:alert(1)" }

With equiv() initially "x-custom" the entry is not a refresh, so the static content is accepted on that basis. setEquiv("refresh") then re-runs only the http-equiv effect — the content is never revalidated, and the element is a live redirect nothing ever approved. Security decisions are properties of the whole entry, so the whole entry is now the unit of work.

Duplicate case-insensitive names split verdict from commit. HTML attribute names are case-insensitive; JavaScript object keys are not, so { "http-equiv": "x-custom", "HTTP-EQUIV": "refresh", … } is legal. A first-match lookup validated x-custom while the DOM loop wrote both and the later HTTP-EQUIV became effective.

Client and SSR each carried the rule. head.ts and ssr.ts held separate copies of the same four checks, so a fix to either would have silently diverged from the other.

What changed

src/utils/metaRefresh.ts is the single policy, called by Head(), renderToDocument, and router SSR:

content := WS* delay WS*
         | WS* delay WS* ";" WS* "url" WS* "=" WS* dest WS*
delay   := DIGIT+
dest    := "'"…"'" | '"'…'"' | unquoted-run

Anything it cannot read unambiguously is dropped — unterminated quotes, competing url= assignments, non-numeric delays, trailing junk, empty destinations, non-url keys. This is deliberately stricter than a browser and makes no claim of WHATWG parity: real engines recover aggressively from malformed input and differ at the edges, and reproducing that would mean matching recovery behaviour I cannot verify across the whole support floor. The cost is a few odd-but-harmless directives dropped; the benefit is that nothing unreadable is ever emitted on the strength of "no forbidden substring was found".

The verdict is a discriminated union (not-refresh / delay-only / allowed / forbidden) because callers act differently on each — a boolean forced "ignore me, I'm a description tag" and "drop this element" into one answer.

Duplicate casings are rejected, not resolved by precedence. Last-write-wins would also be sound if the validated value were provably the committed one, but rejection removes the class of bug rather than re-parameterising it.

Head() now runs one effect per entry: resolve every attribute → fold duplicate casings → validate the assembled snapshot → reconcile. A snapshot that fails validation detaches the element rather than blanking an attribute, since a partially-cleared element is still live markup and a detached one must never count as active.

Related Issue

Closes #

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Breaking: a dangerous or ambiguous refresh entry is now removed entirely, where previously its content attribute was blanked while http-equiv="refresh" stayed in the head. Applications relying on odd-but-harmless refresh spellings the strict parser rejects will see those entries dropped.

Checklist

  • I have read CONTRIBUTING.md
  • My code builds without errors
  • I have tested my changes
  • I have updated documentation if needed
Full suite      428 files / 5659 passed / 1 skipped / 0 failed   (vitest exit 0)
New tests       parser 48 · Head 15 · SSR/router SSR 44 · browser 4 × 3 engines
Browser matrix  chromium 92/92 · firefox 92/92 · webkit 92/92 — 276 total
Node            22.3.0 PASS · 22.14.0 PASS · 24.19.0 PASS  (5659 each)
certify:rc      14 PASS / 0 FAIL — ALL REQUIRED GATES PASSED
Typecheck       src 0 errors · tests+entries 0 errors
Lint            638 files, 0 errors

The browser test synchronises on behaviour, not a delay. A benign control performs a real same-origin refresh and the test waits on waitForURL() — proving the probe can observe a refresh at all, and bounding how long the engine takes to honour one. The negative cases are then backed by a route interceptor counting requests to a protected same-origin path plus an unchanged page.url(). No external host is contacted.

Two judgement calls worth review:

  • One existing test was strengthened, not preserved. head.coverage2 asserted the old mitigation (blank content, keep the element). That expectation is superseded by removal, so it now asserts detachment and restoration once the value is safe again.
  • HeadProps was deliberately not widened. Reactive getters are typed to return string, so "return null to remove an attribute" is not expressible today. Reconciliation does remove attributes that disappear from the resolved snapshot; I did not broaden the public type just to write that test.

@hexplus
hexplus merged commit 1e02935 into main Aug 28, 2026
5 checks passed
@hexplus
hexplus deleted the chore/57-framework-hardening branch August 28, 2026 10:11
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