fix(security): detect image-set() and comment-hidden CSS remote references

scripts/lib/svg-active-content.mjs

@@ -359,6 +359,124 @@ const CSS_URL_PATTERN =
  */
 const CSS_IMPORT_PATTERN = /@import\s+(?:"([^"]*)"|'([^']*)')/gi;
 
+/**
+ * The opening of an `image-set()` argument list, `-webkit-` alias included.
+ *
+ * CSS Images 4 gives `image-set()` a second spelling for its image reference:
+ * `<image-set-option> = [ <image> | <string> ] ...`, and "each `<string>`
+ * inside `image-set()` represents a `<url>`". So
+ * `image-set("https://evil.example/x.png" 1x)` is a live remote fetch that
+ * carries no `url(` token for CSS_URL_PATTERN to find and no `@import` for
+ * CSS_IMPORT_PATTERN -- the same shape of gap CSS_IMPORT_PATTERN exists to
+ * close. `image-set()` is accepted wherever an `<image>` is (`cursor`,
+ * `background-image`, `mask-image`, `list-style-image`), and `cursor` and
+ * `mask-image` are not `img-src` fetch directives, so the meta CSP does not
+ * stand in for this check.
+ *
+ * Only the opening is a pattern. The argument list is walked by
+ * {@link imageSetTargets} rather than matched, because a nested
+ * `url(...)`/`type(...)` means the closing parenthesis cannot be found by a
+ * regex without either stopping early or backtracking.
+ */
+const IMAGE_SET_OPEN = /(?:-webkit-)?image-set\(/gi;
+
+/** A quoted CSS string, matched as a unit so its contents stay opaque. */
+const CSS_STRING = /"([^"]*)"|'([^']*)'/g;
+
+/**
+ * The bare-string arguments of every `image-set()` in a fragment of CSS.
+ *
+ * The argument list is walked with a parenthesis depth counter, skipping
+ * quoted strings so a parenthesis inside one cannot close the list early. An
+ * `image-set(` that never closes is read to the end of the fragment, which is
+ * the fail-closed reading: a target hidden behind a missing parenthesis is
+ * still reported. Strings belonging to a nested `url("...")` are collected
+ * too; that only duplicates what CSS_URL_PATTERN already found, and
+ * findRemoteReferences collects into a Set. A `type("image/png")` string is
+ * collected and then discarded by remoteTarget(), which no media type
+ * satisfies.
+ *
+ * @param {string} css
+ * @returns {string[]} Raw string contents, still CSS-escaped.
+ */
+function imageSetArguments(css) {
+  const values = [];
+  IMAGE_SET_OPEN.lastIndex = 0;
+  let open;
+  while ((open = IMAGE_SET_OPEN.exec(css))) {
+    let cursor = open.index + open[0].length;
+    let depth = 1;
+    const start = cursor;
+    while (cursor < css.length && depth > 0) {
+      const char = css[cursor];
+      if (char === '"' || char === "'") {
+        const close = css.indexOf(char, cursor + 1);
+        cursor = close === -1 ? css.length : close + 1;
+        continue;
+      }
+      if (char === '\\') {
+        cursor += 2;
+        continue;
+      }
+      if (char === '(') depth += 1;
+      else if (char === ')') depth -= 1;
+      cursor += 1;
+    }
+    const end = depth === 0 ? cursor - 1 : css.length;
+    const args = css.slice(start, end);
+    CSS_STRING.lastIndex = 0;
+    for (const string of args.matchAll(CSS_STRING)) {
+      values.push(string[1] ?? string[2]);
+    }
+    IMAGE_SET_OPEN.lastIndex = end;
+  }
+  return values;
+}
+
+/**
+ * Remove CSS comments, which a browser's tokenizer discards before any value
+ * is read. A comment placed between `url(` and its quoted argument, or
+ * between `@import` and its string, still fetches, while the raw text matches
+ * neither CSS_URL_PATTERN nor CSS_IMPORT_PATTERN.
+ *
+ * Quoted strings are skipped: a comment opener inside a CSS string is two
+ * literal characters, not the start of a comment, and treating it as one
+ * would swallow the rest of the stylesheet -- hiding every later `url()` from
+ * the scan, which is the opposite of what this gate is for. An unterminated
+ * comment runs to the end of the fragment, as the tokenizer does.
+ *
+ * Each comment is replaced by a single space rather than deleted. A comment
+ * separates tokens, so deleting it would fuse the text either side and
+ * manufacture a token the browser never sees: a comment inserted into the
+ * middle of the letters of `url(` does not leave a `url(` behind.
+ *
+ * @param {string} css
+ * @returns {string}
+ */
+function stripCssComments(css) {
+  let output = '';
+  let cursor = 0;
+  while (cursor < css.length) {
+    const char = css[cursor];
+    if (char === '"' || char === "'") {
+      const close = css.indexOf(char, cursor + 1);
+      const end = close === -1 ? css.length : close + 1;
+      output += css.slice(cursor, end);
+      cursor = end;
+      continue;
+    }
+    if (char === '/' && css[cursor + 1] === '*') {
+      const close = css.indexOf('*/', cursor + 2);
+      cursor = close === -1 ? css.length : close + 2;
+      output += ' ';
+      continue;
+    }
+    output += char;
+    cursor += 1;
+  }
+  return output;
+}
+
 /**
  * Report whether a value points at a resource on another host.
  *
@@ -464,20 +582,28 @@ export function findRemoteReferences(source) {
 }
 
 /**
- * Collect the remote targets in a fragment of CSS, from both `url(...)` and
- * the bare-string `@import` form.
+ * Collect the remote targets in a fragment of CSS, from the `url(...)` token,
+ * the bare-string `@import` form and the bare-string `image-set()` form.
+ *
+ * Comments are removed first, because a browser's tokenizer discards them
+ * before any of these three constructs is read.
  * @param {string} css
  * @returns {string[]}
  */
 function cssTargets(css) {
   const targets = [];
+  const normalized = stripCssComments(String(css));
   for (const pattern of [CSS_URL_PATTERN, CSS_IMPORT_PATTERN]) {
-    for (const match of String(css).matchAll(pattern)) {
+    for (const match of normalized.matchAll(pattern)) {
       const value = match[1] ?? match[2] ?? match[3];
       const target = remoteTarget(decodeCssEscapes(value));
       if (target) targets.push(target);
     }
   }
+  for (const value of imageSetArguments(normalized)) {
+    const target = remoteTarget(decodeCssEscapes(value));
+    if (target) targets.push(target);
+  }
   return targets;
 }
 

tests/svg-active-content.test.mjs

@@ -1124,3 +1124,156 @@ test('an interior backslash in a local CSS path stays local', () => {
     '</style></svg>';
   assert.deepEqual(findRemoteReferences(svg), []);
 });
+
+test('a bare-string image-set() target is a remote reference', () => {
+  // CSS Images 4: "Each <string> inside image-set() represents a <url>", so
+  // this fetches exactly as url("...") does while carrying no url( token.
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    'rect{cursor:image-set("https://evil.example/set.png" 1x)}' +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), [
+    'references a remote resource in a <style> block: https://evil.example/set.png',
+  ]);
+});
+
+test('the -webkit-image-set() alias is scanned like image-set()', () => {
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    "rect{cursor:-webkit-image-set('https://evil.example/pre.png' 2x)}" +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), [
+    'references a remote resource in a <style> block: https://evil.example/pre.png',
+  ]);
+});
+
+test('a CSS escape inside an image-set() string does not hide the host', () => {
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    'rect{cursor:image-set("\\68 ttps://evil.example/esc-set.png" 1x)}' +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), [
+    'references a remote resource in a <style> block: https://evil.example/esc-set.png',
+  ]);
+});
+
+test('an unclosed image-set() is read to the end rather than skipped', () => {
+  // Fail closed: a target hidden behind a missing parenthesis is still a
+  // target, and the browser's error recovery is not this scanner's to guess.
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    'rect{cursor:image-set("https://evil.example/open.png" 1x' +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), [
+    'references a remote resource in a <style> block: https://evil.example/open.png',
+  ]);
+});
+
+test('a parenthesis inside an image-set() string does not close the list', () => {
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    'rect{cursor:image-set("https://evil.example/a).png" 1x,' +
+    '"https://evil.example/b.png" 2x)}' +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), [
+    'references a remote resource in a <style> block: https://evil.example/a).png',
+    'references a remote resource in a <style> block: https://evil.example/b.png',
+  ]);
+});
+
+test('an escaped parenthesis inside image-set() does not close the list', () => {
+  // A backslash escape is how an unquoted url token carries a parenthesis.
+  // Counting that ')' as a closer would end the argument list early and hide
+  // every later option, so the walk steps over the escaped character.
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    'rect{cursor:image-set(url(https://evil.example/a\\).png) 1x,' +
+    '"https://evil.example/b.png" 2x)}' +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), [
+    'references a remote resource in a <style> block: https://evil.example/a).png',
+    'references a remote resource in a <style> block: https://evil.example/b.png',
+  ]);
+});
+
+test('an image-set() type() media type is not mistaken for a target', () => {
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    'rect{cursor:image-set(url("./local.png") 1x type("image/png"))}' +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), []);
+});
+
+test('an image-set() target reached through both spellings is reported once', () => {
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    'rect{cursor:image-set(url("https://evil.example/dup.png") 1x)}' +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), [
+    'references a remote resource in a <style> block: https://evil.example/dup.png',
+  ]);
+});
+
+test('an image-set() in a presentation attribute is scanned', () => {
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg">' +
+    '<rect style=\'cursor:image-set("https://evil.example/attr-set.png" 1x)\'/>' +
+    '</svg>';
+  assert.deepEqual(findRemoteReferences(svg), [
+    'references a remote resource in a style attribute: https://evil.example/attr-set.png',
+  ]);
+});
+
+test('a CSS comment between url( and its argument does not hide the host', () => {
+  // A browser's tokenizer discards comments before the value is read.
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    'rect{background:url( /* c */ "https://evil.example/cmt.png")}' +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), [
+    'references a remote resource in a <style> block: https://evil.example/cmt.png',
+  ]);
+});
+
+test('a CSS comment after @import does not hide the host', () => {
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    '@import /* c */ "https://evil.example/cmt.css";' +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), [
+    'references a remote resource in a <style> block: https://evil.example/cmt.css',
+  ]);
+});
+
+test('a comment opener inside a CSS string does not blind the rest of the scan', () => {
+  // `content: "/*"` is two literal characters. Treating it as a comment would
+  // swallow every declaration after it, hiding the live fetch below.
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    'rect::before{content:"/*"}' +
+    'rect{background:url("https://evil.example/after.png")}' +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), [
+    'references a remote resource in a <style> block: https://evil.example/after.png',
+  ]);
+});
+
+test('an unterminated CSS comment runs to the end of the block', () => {
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    'rect{background:url("https://evil.example/before.png")}/* trailing' +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), [
+    'references a remote resource in a <style> block: https://evil.example/before.png',
+  ]);
+});
+
+test('a comment is replaced by a separator, not deleted', () => {
+  // Deleting it would fuse the text either side into a `url(` token the
+  // browser never sees, inventing a finding out of inert text.
+  const svg =
+    '<svg xmlns="http://www.w3.org/2000/svg"><style>' +
+    'rect{background:ur/* x */l("https://evil.example/fused.png")}' +
+    '</style></svg>';
+  assert.deepEqual(findRemoteReferences(svg), []);
+});