fix(security): check for an escaped identifier before consuming the escape

My previous commit introduced this. Handling `\` outside strings before
readIdentifier meant a LEADING escape was eaten before the url check
saw it: `\75` is the CSS escape for `u`, so `.a{background:\75rl(...)}`
is url() to a browser and passed through untouched, with no warning —
a bypass the base version did not have. An escape mid-identifier
(`u\72l`) was unaffected, which is why the first tests missed it.

The escape branch now runs AFTER readIdentifier, which already decodes
leading escapes itself. What is left for it is the case it was added
for: `\'`, which must not be read as opening a string.

Both spellings are pinned, along with the legitimate escaped selector
and data: URI that must survive untouched.

Refs #1264
This commit is contained in:
Paul Nothaft
2026-09-05 14:26:51 +02:00
parent 1cf82746b7
commit b6dc0991ce
2 changed files with 24 additions and 12 deletions
+13 -12
View File
@@ -140,18 +140,6 @@ function stripDisallowedUrls(css) {
continue;
}
// --- escape OUTSIDE a string -----------------------------------------
// `\'` is an escaped identifier character, not the start of a string.
// Without this the scanner stepped onto the apostrophe, entered string
// mode, and copied the rest of the stylesheet unscanned — so
// `.hero{--marker:\';background:url(https://evil.example/p.gif)}` kept a
// live remote URL. Consume the escape and its escaped character together.
if (input[i] === '\\' && i + 1 < input.length) {
out += input.slice(i, i + 2);
i += 2;
continue;
}
// --- string ----------------------------------------------------------
// Escape-aware: `\"` inside a double-quoted string does NOT close it.
// Decoding escapes up front (an earlier attempt) turned that into a real
@@ -213,6 +201,19 @@ function stripDisallowedUrls(css) {
continue;
}
// --- escape that does NOT begin an identifier -------------------------
// Ordered AFTER readIdentifier deliberately. `\75` is the escape for
// `u`, so `\75rl(...)` is url() to a browser — consuming the escape
// first would hide it from the check above, which is a bypass this
// branch introduced when it ran earlier. What is left for it is the
// `\'` case: an escaped quote that must not be read as opening a
// string, since that swallowed the rest of the stylesheet unscanned.
if (input[i] === '\\' && i + 1 < input.length) {
out += input.slice(i, i + 2);
i += 2;
continue;
}
out += input[i];
i += 1;
}