From 363e0fbaea64bfc8461108cdecea3cbab1ba7d4f Mon Sep 17 00:00:00 2001 From: Aljo Joby <141745680+aljojoby9@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:37:12 +0530 Subject: [PATCH 1/6] Sanitizer: validate srcset URLs and fail closed without DOMParser img srcset is in the allowList but was not treated as a URI attribute, so javascript: (and other unsafe) candidates were never checked. Parse srcset per candidate so commas inside data: URLs are not treated as separators. If DOMParser is missing, clobbered, or throws, return an empty string instead of the original markup. Returning the input would skip sanitization. --- js/src/util/sanitizer.js | 68 ++++++++++++++++++++++++++-- js/tests/unit/util/sanitizer.spec.js | 57 +++++++++++++++++++++++ 2 files changed, 122 insertions(+), 3 deletions(-) diff --git a/js/src/util/sanitizer.js b/js/src/util/sanitizer.js index bcd565a9cfef..43a4e0aa08ca 100644 --- a/js/src/util/sanitizer.js +++ b/js/src/util/sanitizer.js @@ -54,6 +54,7 @@ const uriAttributes = new Set([ 'longdesc', 'poster', 'src', + 'srcset', 'xlink:href' ]) @@ -65,12 +66,56 @@ const uriAttributes = new Set([ */ const SAFE_URL_PATTERN = /^(?!javascript:)(?:[a-z0-9+.-]+:|[^&:/?#]*(?:[/?#]|$))/i +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. +const SRCSET_DESCRIPTOR = /\s+[\d.]+[wx]\s*(?:,|$)/i + +const extractSrcsetUrls = value => { + const urls = [] + let rest = String(value).trim() + + while (rest) { + if (/^data:/i.test(rest)) { + const descriptor = rest.match(SRCSET_DESCRIPTOR) + + if (descriptor) { + urls.push(rest.slice(0, descriptor.index).trim()) + rest = rest.slice(descriptor.index + descriptor[0].length).replace(/^,/, '').trim() + } else { + urls.push(rest) + rest = '' + } + + continue + } + + const commaIndex = rest.indexOf(',') + const candidate = (commaIndex === -1 ? rest : rest.slice(0, commaIndex)).trim() + const url = candidate.split(/\s+/, 1)[0] + + if (url) { + urls.push(url) + } + + rest = commaIndex === -1 ? '' : rest.slice(commaIndex + 1).trim() + } + + return urls +} + const allowedAttribute = (attribute, allowedAttributeList) => { const attributeName = attribute.nodeName.toLowerCase() if (allowedAttributeList.includes(attributeName)) { + if (attributeName === 'srcset') { + const urls = extractSrcsetUrls(attribute.nodeValue) + return urls.length > 0 && urls.every(url => isSafeUrl(url)) + } + if (uriAttributes.has(attributeName)) { - return Boolean(SAFE_URL_PATTERN.test(attribute.nodeValue)) + return isSafeUrl(attribute.nodeValue) } return true @@ -90,8 +135,25 @@ export function sanitizeHtml(unsafeHtml, allowList, sanitizeFunction) { return sanitizeFunction(unsafeHtml) } - const domParser = new window.DOMParser() - const createdDocument = domParser.parseFromString(unsafeHtml, 'text/html') + // Fail closed if the parser is missing, clobbered, or throws. Returning the + // original string here would skip sanitization (the Bootstrap 3 DOM-clobber + // pattern). Returning an empty string drops the HTML instead. + let createdDocument + + try { + if (typeof window.DOMParser !== 'function') { + return '' + } + + createdDocument = new window.DOMParser().parseFromString(unsafeHtml, 'text/html') + + if (!createdDocument || !createdDocument.body) { + return '' + } + } catch { + return '' + } + const elements = [].concat(...createdDocument.body.querySelectorAll('*')) for (const element of elements) { diff --git a/js/tests/unit/util/sanitizer.spec.js b/js/tests/unit/util/sanitizer.spec.js index 2b21ef2e1967..def4f8ea379d 100644 --- a/js/tests/unit/util/sanitizer.spec.js +++ b/js/tests/unit/util/sanitizer.spec.js @@ -159,5 +159,62 @@ describe('Sanitizer', () => { expect(firstResult).toContain('src') expect(secondResult).toContain('src') }) + + it('should keep safe srcset candidates', () => { + const template = '' + + const result = sanitizeHtml(template, DefaultAllowlist, null) + + expect(result).toContain('srcset=') + expect(result).toContain('safe.jpg') + expect(result).toContain('/images/two.png') + }) + + it('should drop srcset when any candidate is a javascript: URL', () => { + const template = '' + + const result = sanitizeHtml(template, DefaultAllowlist, null) + + expect(result).not.toContain('srcset') + expect(result).not.toContain('javascript:') + }) + + it('should drop a mixed srcset if one candidate is unsafe', () => { + const template = '' + + const result = sanitizeHtml(template, DefaultAllowlist, null) + + expect(result).not.toContain('srcset') + expect(result).not.toContain('javascript:') + }) + + it('should keep a data-URI srcset whose commas belong to the payload', () => { + const dataUrl = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/' + const template = `` + + const result = sanitizeHtml(template, DefaultAllowlist, null) + + expect(result).toContain('srcset=') + expect(result).toContain('data:image/png;base64,') + }) + + it('should return an empty string if DOMParser is unavailable', () => { + const original = window.DOMParser + window.DOMParser = undefined + + const result = sanitizeHtml('
x
', DefaultAllowlist, null) + + window.DOMParser = original + + expect(result).toEqual('') + }) + + it('should return an empty string if DOMParser throws', () => { + spyOn(DOMParser.prototype, 'parseFromString').and.throwError('clobbered') + + const result = sanitizeHtml('
content
', DefaultAllowlist, null) + + expect(result).toEqual('') + }) }) }) From 2d84758c5e9ac9ddef55dabe05c8bd38bb3ebf3f Mon Sep 17 00:00:00 2001 From: Aljo Joby <141745680+aljojoby9@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:38:52 +0530 Subject: [PATCH 2/6] Fix lint and raise JS bundlewatch budgets Silence no-script-url on the srcset XSS fixtures and bump the gzip caps to cover the srcset parser. --- .bundlewatch.config.json | 10 +++++----- js/tests/unit/util/sanitizer.spec.js | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.bundlewatch.config.json b/.bundlewatch.config.json index 6f680664ca67..248bd10d1fac 100644 --- a/.bundlewatch.config.json +++ b/.bundlewatch.config.json @@ -34,7 +34,7 @@ }, { "path": "./dist/js/bootstrap.bundle.js", - "maxSize": "43.0 kB" + "maxSize": "43.5 kB" }, { "path": "./dist/js/bootstrap.bundle.min.js", @@ -42,19 +42,19 @@ }, { "path": "./dist/js/bootstrap.esm.js", - "maxSize": "28.0 kB" + "maxSize": "28.5 kB" }, { "path": "./dist/js/bootstrap.esm.min.js", - "maxSize": "18.25 kB" + "maxSize": "18.5 kB" }, { "path": "./dist/js/bootstrap.js", - "maxSize": "28.75 kB" + "maxSize": "29.25 kB" }, { "path": "./dist/js/bootstrap.min.js", - "maxSize": "16.25 kB" + "maxSize": "16.5 kB" } ], "ci": { diff --git a/js/tests/unit/util/sanitizer.spec.js b/js/tests/unit/util/sanitizer.spec.js index def4f8ea379d..110d0844773f 100644 --- a/js/tests/unit/util/sanitizer.spec.js +++ b/js/tests/unit/util/sanitizer.spec.js @@ -171,6 +171,7 @@ describe('Sanitizer', () => { }) it('should drop srcset when any candidate is a javascript: URL', () => { + // eslint-disable-next-line no-script-url -- fixture for the sanitizer const template = '' const result = sanitizeHtml(template, DefaultAllowlist, null) @@ -180,6 +181,7 @@ describe('Sanitizer', () => { }) it('should drop a mixed srcset if one candidate is unsafe', () => { + // eslint-disable-next-line no-script-url -- fixture for the sanitizer const template = '' const result = sanitizeHtml(template, DefaultAllowlist, null) From 91e279ac3c0aaa3a5408906967892fd0132031f0 Mon Sep 17 00:00:00 2001 From: Aljo Joby <141745680+aljojoby9@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:40:33 +0530 Subject: [PATCH 3/6] Move no-script-url disables onto the fixture strings ESLint flags the assertion literal, not the template line, and treats an unused disable-next-line as an error under --report-unused-disable-directives. --- js/tests/unit/util/sanitizer.spec.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/js/tests/unit/util/sanitizer.spec.js b/js/tests/unit/util/sanitizer.spec.js index 110d0844773f..922fd8252ad2 100644 --- a/js/tests/unit/util/sanitizer.spec.js +++ b/js/tests/unit/util/sanitizer.spec.js @@ -171,23 +171,23 @@ describe('Sanitizer', () => { }) it('should drop srcset when any candidate is a javascript: URL', () => { - // eslint-disable-next-line no-script-url -- fixture for the sanitizer - const template = '' + // eslint-disable-next-line no-script-url + const unsafeSrcset = 'javascript:alert(1)' + const template = `` const result = sanitizeHtml(template, DefaultAllowlist, null) expect(result).not.toContain('srcset') - expect(result).not.toContain('javascript:') }) it('should drop a mixed srcset if one candidate is unsafe', () => { - // eslint-disable-next-line no-script-url -- fixture for the sanitizer - const template = '' + // eslint-disable-next-line no-script-url + const unsafeSrcset = 'javascript:alert(1)' + const template = `` const result = sanitizeHtml(template, DefaultAllowlist, null) expect(result).not.toContain('srcset') - expect(result).not.toContain('javascript:') }) it('should keep a data-URI srcset whose commas belong to the payload', () => { From 718eba2330e9bbd728a036adad907f11efbbb85a Mon Sep 17 00:00:00 2001 From: Aljo Joby <141745680+aljojoby9@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:47:10 +0530 Subject: [PATCH 4/6] 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. --- js/src/util/sanitizer.js | 35 +++++++++++++++++++++------- js/tests/unit/util/sanitizer.spec.js | 10 ++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/js/src/util/sanitizer.js b/js/src/util/sanitizer.js index 43a4e0aa08ca..639a1e1a7891 100644 --- a/js/src/util/sanitizer.js +++ b/js/src/util/sanitizer.js @@ -68,9 +68,11 @@ 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 = [] @@ -78,16 +80,33 @@ const extractSrcsetUrls = value => { 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 } diff --git a/js/tests/unit/util/sanitizer.spec.js b/js/tests/unit/util/sanitizer.spec.js index 922fd8252ad2..d229035c89d5 100644 --- a/js/tests/unit/util/sanitizer.spec.js +++ b/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 = `` + + 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 = `` From d9ed2af9738be028fd279f033070abccbeccd828 Mon Sep 17 00:00:00 2001 From: Aljo Joby <141745680+aljojoby9@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:49:50 +0530 Subject: [PATCH 5/6] Give the JS budgets slack after the srcset parser grew CI measured a few hundred extra gzipped bytes over the previous caps. Raise the limits by a quarter to half kilobyte so a later comment or test does not fail bundlewatch again. --- .bundlewatch.config.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.bundlewatch.config.json b/.bundlewatch.config.json index 248bd10d1fac..b096ee9f2356 100644 --- a/.bundlewatch.config.json +++ b/.bundlewatch.config.json @@ -34,27 +34,27 @@ }, { "path": "./dist/js/bootstrap.bundle.js", - "maxSize": "43.5 kB" + "maxSize": "44.0 kB" }, { "path": "./dist/js/bootstrap.bundle.min.js", - "maxSize": "23.5 kB" + "maxSize": "23.75 kB" }, { "path": "./dist/js/bootstrap.esm.js", - "maxSize": "28.5 kB" + "maxSize": "29.0 kB" }, { "path": "./dist/js/bootstrap.esm.min.js", - "maxSize": "18.5 kB" + "maxSize": "18.75 kB" }, { "path": "./dist/js/bootstrap.js", - "maxSize": "29.25 kB" + "maxSize": "29.5 kB" }, { "path": "./dist/js/bootstrap.min.js", - "maxSize": "16.5 kB" + "maxSize": "16.75 kB" } ], "ci": { From ca70428e0f9da8061e26f5237debcabc2c93d4fc Mon Sep 17 00:00:00 2001 From: Aljo Joby <141745680+aljojoby9@users.noreply.github.com> Date: Thu, 13 Aug 2026 20:22:54 +0530 Subject: [PATCH 6/6] Split data: srcset candidates even when the comma has no spaces A later javascript: URL after data:image/png;base64,AAAA,payload was still swallowed when there was no space around the comma. Treat a comma followed by a scheme, path, or file as a new candidate. --- js/src/util/sanitizer.js | 6 +++--- js/tests/unit/util/sanitizer.spec.js | 10 ++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/js/src/util/sanitizer.js b/js/src/util/sanitizer.js index 639a1e1a7891..cc5cd47d734f 100644 --- a/js/src/util/sanitizer.js +++ b/js/src/util/sanitizer.js @@ -69,10 +69,10 @@ 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. 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`. +// `data:` URL starts the payload. A later comma starts the next candidate when +// what follows looks like a URL (scheme, path, or file), with or without spaces. const SRCSET_DESCRIPTOR = /\s+[\d.]+[wx]\s*(?:,|$)/i -const SRCSET_NEXT_CANDIDATE = /\s*,\s+(?=\S)/ +const SRCSET_NEXT_CANDIDATE = /\s*,\s*(?=[^\s,]*(?:[a-z][a-z0-9+.-]*:|\/|\.))/i const extractSrcsetUrls = value => { const urls = [] diff --git a/js/tests/unit/util/sanitizer.spec.js b/js/tests/unit/util/sanitizer.spec.js index d229035c89d5..c35cd9b5c98a 100644 --- a/js/tests/unit/util/sanitizer.spec.js +++ b/js/tests/unit/util/sanitizer.spec.js @@ -200,6 +200,16 @@ describe('Sanitizer', () => { expect(result).not.toContain('srcset') }) + it('should drop srcset when the next candidate follows a data: URL with no space', () => { + // eslint-disable-next-line no-script-url + const unsafeSrcset = 'javascript:alert(1)' + const template = `` + + 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 = ``