From b6dc0991ce04b574a03aff9cd579f0333b11048e Mon Sep 17 00:00:00 2001
From: Paul Nothaft <paul@MacStudio-von-Paul.local>
Date: Sat, 5 Sep 2026 14:26:51 +0200
Subject: [PATCH] fix(security): check for an escaped identifier before
 consuming the escape
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 .../utils/cssSanitizer.remoteUrls.test.js     | 11 ++++++++
 backend/src/utils/cssSanitizer.js             | 25 ++++++++++---------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js b/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js
index ba30a667..76c1011b 100644
--- a/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js
+++ b/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js
@@ -249,6 +249,17 @@ describe('sanitizeCSS', () => {
     expect(asParsed(sanitized)).not.toContain('evil.example');
   });
 
+  it.each([
+    ['a leading escape', '.a{background:\\75rl(https://evil.example/p.gif)}'],
+    ['an escape mid-identifier', '.a{background:u\\72l(https://evil.example/p.gif)}'],
+  ])('blocks a url() spelled with %s', (_label, css) => {
+    // `\75` is the CSS escape for `u`, so a browser reads `\75rl(` as
+    // url(). The escape-outside-a-string handling has to run AFTER the
+    // identifier check, or it eats the escape and hides the token.
+    const { sanitized } = sanitizeCSS(css);
+    expect(asParsed(sanitized)).not.toContain('evil.example');
+  });
+
   it('does not let an unterminated quote hide everything after it', () => {
     // An unclosed quote is a parse error. Trusting it meant one stray
     // apostrophe disabled scanning for the remainder of the stylesheet, so
diff --git a/backend/src/utils/cssSanitizer.js b/backend/src/utils/cssSanitizer.js
index 152e51c9..ed3623f5 100644
--- a/backend/src/utils/cssSanitizer.js
+++ b/backend/src/utils/cssSanitizer.js
@@ -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;
   }