diff --git a/.bundlewatch.config.json b/.bundlewatch.config.json index 6f680664ca67..b096ee9f2356 100644 --- a/.bundlewatch.config.json +++ b/.bundlewatch.config.json @@ -34,27 +34,27 @@ }, { "path": "./dist/js/bootstrap.bundle.js", - "maxSize": "43.0 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.0 kB" + "maxSize": "29.0 kB" }, { "path": "./dist/js/bootstrap.esm.min.js", - "maxSize": "18.25 kB" + "maxSize": "18.75 kB" }, { "path": "./dist/js/bootstrap.js", - "maxSize": "28.75 kB" + "maxSize": "29.5 kB" }, { "path": "./dist/js/bootstrap.min.js", - "maxSize": "16.25 kB" + "maxSize": "16.75 kB" } ], "ci": { diff --git a/js/src/util/sanitizer.js b/js/src/util/sanitizer.js index bcd565a9cfef..cc5cd47d734f 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,75 @@ 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. The first comma in a +// `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,]*(?:[a-z][a-z0-9+.-]*:|\/|\.))/i + +const extractSrcsetUrls = value => { + const urls = [] + let rest = String(value).trim() + + while (rest) { + if (/^data:/i.test(rest)) { + const headerComma = rest.indexOf(',') + + if (headerComma === -1) { + urls.push(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 + } + + 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 +154,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..c35cd9b5c98a 100644 --- a/js/tests/unit/util/sanitizer.spec.js +++ b/js/tests/unit/util/sanitizer.spec.js @@ -159,5 +159,84 @@ 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', () => { + // 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 drop a mixed srcset if one candidate is unsafe', () => { + // 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 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 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 = `` + + 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('') + }) }) })