Parse data: srcset candidates without swallowing later URLs

A descriptor on a later candidate must not extend a data: URL that has
no descriptor of its own. Split on a comma-plus-space after the data
payload so javascript: (and other) candidates are checked separately.

Author: aljojoby9

js/src/util/sanitizer.js

@@ -68,26 +68,45 @@ const SAFE_URL_PATTERN = /^(?!javascript:)(?:[a-z0-9+.-]+:|[^&:/?#]*(?:[/?#]|$))
 
 const isSafeUrl = url => Boolean(SAFE_URL_PATTERN.test(url))
 
-// `srcset` is a comma-separated list of candidates. Commas also appear inside
-// `data:` URLs, so the whole attribute string cannot be checked as one URI.
+// `srcset` is a comma-separated list of candidates. The first comma in a
+// `data:` URL starts the payload; later commas (with surrounding space) start
+// the next candidate. Do not search the whole remainder for `1x`/`2x`/`w`.
 const SRCSET_DESCRIPTOR = /\s+[\d.]+[wx]\s*(?:,|$)/i
+const SRCSET_NEXT_CANDIDATE = /\s*,\s+(?=\S)/
 
 const extractSrcsetUrls = value => {
   const urls = []
   let rest = String(value).trim()
 
   while (rest) {
     if (/^data:/i.test(rest)) {
-      const descriptor = rest.match(SRCSET_DESCRIPTOR)
+      const headerComma = rest.indexOf(',')
 
-      if (descriptor) {
-        urls.push(rest.slice(0, descriptor.index).trim())
-        rest = rest.slice(descriptor.index + descriptor[0].length).replace(/^,/, '').trim()
-      } else {
+      if (headerComma === -1) {
         urls.push(rest)
-        rest = ''
+        break
+      }
+
+      const payload = rest.slice(headerComma + 1)
+      const descriptor = payload.match(SRCSET_DESCRIPTOR)
+      const nextCandidate = payload.match(SRCSET_NEXT_CANDIDATE)
+      const descriptorAt = descriptor ? descriptor.index : Number.POSITIVE_INFINITY
+      const nextAt = nextCandidate ? nextCandidate.index : Number.POSITIVE_INFINITY
+
+      if (nextAt !== Number.POSITIVE_INFINITY && nextAt <= descriptorAt) {
+        urls.push(rest.slice(0, headerComma + 1 + nextAt).trim())
+        rest = payload.slice(nextAt).replace(/^,/, '').trim()
+        continue
+      }
+
+      if (descriptor) {
+        urls.push(rest.slice(0, headerComma + 1 + descriptor.index).trim())
+        rest = payload.slice(descriptor.index + descriptor[0].length).replace(/^,/, '').trim()
+        continue
       }
 
+      urls.push(rest)
+      rest = ''
       continue
     }
 

js/tests/unit/util/sanitizer.spec.js

@@ -190,6 +190,16 @@ describe('Sanitizer', () => {
       expect(result).not.toContain('srcset')
     })
 
+    it('should drop srcset when a data: candidate is followed by an unsafe candidate', () => {
+      // eslint-disable-next-line no-script-url
+      const unsafeSrcset = 'javascript:alert(1)'
+      const template = `<img src="safe.jpg" srcset="data:image/png;base64,AAAA , ${unsafeSrcset} 2x">`
+
+      const result = sanitizeHtml(template, DefaultAllowlist, null)
+
+      expect(result).not.toContain('srcset')
+    })
+
     it('should keep a data-URI srcset whose commas belong to the payload', () => {
       const dataUrl = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/'
       const template = `<img src="safe.jpg" srcset="${dataUrl} 1x">`