Skip to content

Creative CSS and srcset rewriters mishandle malformed values #1114

Description

@prk-Jr

Description

Two parsers in crates/trusted-server-core/src/creative.rs handle malformed input in ways that diverge from how a browser reads the same markup.

rewrite_style_urls decides whether a url() value is quoted by looking at its first character, then steps back one byte from the closing paren on the assumption that a matching quote is there. Nothing verifies it. When the preceding character is multi-byte, the resulting slice is not on a character boundary. When the quotes simply do not match, the closing delimiter is silently rewritten to match the opening one.

It also ends the value at the first ), but a quoted CSS string may legally contain one — so url("https://cdn.example/a)b.png") is truncated, and the intended URL is left unrewritten.

Values it cannot faithfully resolve are still rewritten: CSS escapes are not resolved here, and a raw newline (which preprocessing also produces from a carriage return or form feed) makes the value a bad string the browser discards. In both cases a proxy URL is emitted for a resource the page never requests.

split_srcset_candidates re-derives per-candidate facts — whether the candidate uses the data: scheme, and whether whitespace has appeared — from the whole candidate prefix at every comma, allocating a fresh lowercased copy each time. Those facts belong to the candidate, not to each comma, so a candidate containing many commas rescans and reallocates repeatedly. The creative module's test suite took 64s on one such input; it now takes 0.46s.

Expected behavior

  • A url() value is treated as quoted only when a matching closing quote is present, and slicing never lands mid-character.
  • A quoted value ends at its closing quote, so an inner ) is part of the URL.
  • A value whose bytes cannot be resolved to the URL the browser will request is passed through untouched.
  • Candidate state in srcset is tracked as the scan advances rather than re-derived per comma.

Affected area

HTML processing / JS injection

Done when

  • A malformed url() value round-trips unchanged instead of being mangled or slicing mid-character
  • A quoted value containing ) is rewritten in full
  • Values carrying an escape or a raw newline are left alone
  • srcset scanning is linear in the candidate length
  • Each behaviour has a test that fails when its fix is removed

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions