diff --git a/.bundlewatch.config.json b/.bundlewatch.config.json index 82ebf2677d03..4befec87e020 100644 --- a/.bundlewatch.config.json +++ b/.bundlewatch.config.json @@ -38,7 +38,7 @@ }, { "path": "./dist/js/bootstrap.bundle.min.js", - "maxSize": "55.0 kB" + "maxSize": "55.25 kB" }, { "path": "./dist/js/bootstrap.js", @@ -46,7 +46,7 @@ }, { "path": "./dist/js/bootstrap.min.js", - "maxSize": "33.0 kB" + "maxSize": "33.25 kB" } ], "ci": { diff --git a/.cspell.json b/.cspell.json index 19b89343cc8f..5fc52b2be0e7 100644 --- a/.cspell.json +++ b/.cspell.json @@ -194,6 +194,7 @@ "urlize", "urlquery", "vbtn", + "viewbox", "viewports", "Vite", "vstack", diff --git a/js/src/chips.ts b/js/src/chips.ts index 2f2e5f768944..28708c43e657 100644 --- a/js/src/chips.ts +++ b/js/src/chips.ts @@ -8,6 +8,7 @@ import BaseComponent from './base-component.js' import EventHandler, { type BootstrapEvent } from './dom/event-handler.js' import SelectorEngine from './dom/selector-engine.js' +import { DefaultIconAllowlist, sanitizeHtml } from './util/sanitizer.js' /** * Constants @@ -344,7 +345,9 @@ class Chips extends BaseComponent { button.className = CLASS_NAME_CHIP_DISMISS button.setAttribute('aria-label', 'Remove') button.setAttribute('tabindex', '-1') // Not in tab order, chips handle keyboard - button.innerHTML = this._config.dismissIcon + // dismissIcon accepts HTML (SVGs, icon fonts) and is also settable via the + // data API, so run it through the icon allowlist before insertion. + button.innerHTML = sanitizeHtml(this._config.dismissIcon, DefaultIconAllowlist) return button } diff --git a/js/src/nav-overflow.ts b/js/src/nav-overflow.ts index da19dae9a835..c643b82e77a3 100644 --- a/js/src/nav-overflow.ts +++ b/js/src/nav-overflow.ts @@ -8,6 +8,7 @@ import BaseComponent from './base-component.js' import EventHandler from './dom/event-handler.js' import SelectorEngine from './dom/selector-engine.js' +import { DefaultIconAllowlist, sanitizeHtml } from './util/sanitizer.js' /** * Constants @@ -162,25 +163,40 @@ class NavOverflow extends BaseComponent { return } - const iconHtml = this._resolveIcon() - const iconSpan = `${iconHtml}` - const textSpan = `${this._config.moreText}` - const toggleContent = this._config.iconPlacement === 'end' ? - `${textSpan}${iconSpan}` : - `${iconSpan}${textSpan}` - + // Build with DOM APIs instead of string templates so user-supplied + // moreText / menuPlacement / moreIcon cannot break out of their slots. const overflowItem = document.createElement('li') overflowItem.className = 'nav-item nav-overflow-item' - overflowItem.innerHTML = ` - -
- ` + const button = document.createElement('button') + button.type = 'button' + button.className = 'nav-link nav-overflow-toggle' + button.setAttribute('data-bs-toggle', 'menu') + button.setAttribute('data-bs-placement', this._config.menuPlacement) + button.setAttribute('aria-expanded', 'false') + + const iconSpan = document.createElement('span') + iconSpan.className = 'nav-overflow-icon' + iconSpan.innerHTML = sanitizeHtml(this._resolveIcon(), DefaultIconAllowlist) + + const textSpan = document.createElement('span') + textSpan.className = 'nav-overflow-text' + textSpan.textContent = this._config.moreText + + if (this._config.iconPlacement === 'end') { + button.append(textSpan, iconSpan) + } else { + button.append(iconSpan, textSpan) + } + + const menu = document.createElement('div') + menu.className = `${CLASS_NAME_OVERFLOW_MENU} menu` + + overflowItem.append(button, menu) this._element.append(overflowItem) - this._overflowToggle = overflowItem.querySelector(SELECTOR_OVERFLOW_TOGGLE) - this._overflowMenu = overflowItem.querySelector(SELECTOR_OVERFLOW_MENU) + + this._overflowToggle = button + this._overflowMenu = menu } protected _resolveIcon(): string { @@ -196,6 +212,7 @@ class NavOverflow extends BaseComponent { customIconElement.remove() + // Returned HTML is sanitized in `_createOverflowMenu` before insertion. return iconHtml } diff --git a/js/src/util/sanitizer.ts b/js/src/util/sanitizer.ts index 3f777f4e76ec..08f30ba10786 100644 --- a/js/src/util/sanitizer.ts +++ b/js/src/util/sanitizer.ts @@ -48,6 +48,32 @@ export const DefaultAllowlist: SanitizerAllowList = { } // js-docs-end allow-list +// js-docs-start icon-allow-list +/** + * Allowlist for icon HTML options (Chips `dismissIcon`, NavOverflow `moreIcon`, + * and markup supplied via `[data-bs-overflow-icon]`). Covers the default SVG + * icons plus common inline-icon markup. Event-handler attributes and tags not + * listed here are stripped by `sanitizeHtml`. + */ +export const DefaultIconAllowlist: SanitizerAllowList = { + '*': ['class', 'role', ARIA_ATTRIBUTE_PATTERN], + // Attribute names are matched lowercased (see allowedAttribute). `viewBox` is + // listed as `viewbox` so the default SVG icons keep their coordinate system. + svg: ['xmlns', 'width', 'height', 'viewbox', 'fill', 'stroke', 'stroke-width', 'stroke-linecap', 'stroke-linejoin', 'focusable'], + path: ['d', 'fill', 'stroke', 'stroke-width', 'fill-rule', 'clip-rule'], + line: ['x1', 'y1', 'x2', 'y2', 'stroke', 'stroke-width', 'stroke-linecap'], + circle: ['cx', 'cy', 'r', 'fill', 'stroke', 'stroke-width'], + rect: ['x', 'y', 'width', 'height', 'rx', 'ry', 'fill', 'stroke', 'stroke-width'], + polyline: ['points', 'fill', 'stroke', 'stroke-width'], + polygon: ['points', 'fill', 'stroke', 'stroke-width'], + g: ['fill', 'stroke', 'stroke-width', 'transform'], + // No `use` here: `href` / `xlink:href` on can load external SVG + // fragments. Apps that need sprites can extend this allowlist deliberately. + span: [], + i: [] +} +// js-docs-end icon-allow-list + const uriAttributes = new Set([ 'background', 'cite', diff --git a/js/tests/unit/chips.spec.js b/js/tests/unit/chips.spec.js index e3a28726c148..5859aee78e70 100644 --- a/js/tests/unit/chips.spec.js +++ b/js/tests/unit/chips.spec.js @@ -157,6 +157,42 @@ describe('Chips', () => { expect(chipsEl.querySelector('.chip-dismiss')).toBeNull() }) + + it('should keep the default dismiss SVG after sanitization', () => { + const { chips, chipsEl } = makeChips() + chips.add('alpha') + + const dismiss = chipsEl.querySelector('.chip-dismiss') + expect(dismiss.querySelector('svg')).not.toBeNull() + expect(dismiss.querySelector('line')).not.toBeNull() + }) + + it('should sanitize dismissIcon HTML before inserting it', () => { + const { chips, chipsEl } = makeChips({ + dismissIcon: '' + }) + chips.add('alpha') + + const dismiss = chipsEl.querySelector('.chip-dismiss') + expect(dismiss.querySelector('img')).toBeNull() + expect(dismiss.innerHTML).not.toMatch(/onerror/i) + expect(dismiss.querySelector('svg.safe-icon')).not.toBeNull() + expect(window.__chipsXss).toBeUndefined() + }) + + it('should sanitize dismissIcon supplied via data attributes', () => { + const { chips, chipsEl } = makeChips( + null, + '
x\'>
' + ) + chips.add('alpha') + + const dismiss = chipsEl.querySelector('.chip-dismiss') + expect(dismiss.querySelector('img')).toBeNull() + expect(dismiss.innerHTML).not.toMatch(/onerror/i) + expect(dismiss.querySelector('.icon-ok')).not.toBeNull() + expect(window.__chipsDataXss).toBeUndefined() + }) }) describe('remove', () => { diff --git a/js/tests/unit/nav-overflow.spec.js b/js/tests/unit/nav-overflow.spec.js index 6cdd798e8ac3..cf187b325528 100644 --- a/js/tests/unit/nav-overflow.spec.js +++ b/js/tests/unit/nav-overflow.spec.js @@ -625,6 +625,88 @@ describe('NavOverflow', () => { navOverflow.dispose() }) + + it('should treat moreText as plain text, not HTML', () => { + fixtureEl.innerHTML = [ + '' + ].join('') + + const navEl = fixtureEl.querySelector('[data-bs-toggle="nav-overflow"]') + const navOverflow = new NavOverflow(navEl, { + moreText: 'More' + }) + + const toggleText = navEl.querySelector('.nav-overflow-text') + expect(toggleText.querySelector('img')).toBeNull() + expect(toggleText.textContent).toEqual('More') + expect(window.__navTextXss).toBeUndefined() + + navOverflow.dispose() + }) + + it('should sanitize moreIcon HTML before inserting it', () => { + fixtureEl.innerHTML = [ + '' + ].join('') + + const navEl = fixtureEl.querySelector('[data-bs-toggle="nav-overflow"]') + const navOverflow = new NavOverflow(navEl, { + moreIcon: '…' + }) + + const iconContainer = navEl.querySelector('.nav-overflow-icon') + expect(iconContainer.querySelector('img')).toBeNull() + expect(iconContainer.innerHTML).not.toMatch(/onerror/i) + expect(iconContainer.querySelector('.safe-icon')).not.toBeNull() + expect(window.__navIconXss).toBeUndefined() + + navOverflow.dispose() + }) + + it('should not let menuPlacement break out of its attribute', () => { + fixtureEl.innerHTML = [ + '' + ].join('') + + const navEl = fixtureEl.querySelector('[data-bs-toggle="nav-overflow"]') + const maliciousPlacement = 'bottom-end">' + const navOverflow = new NavOverflow(navEl, { + menuPlacement: maliciousPlacement + }) + + const toggle = navEl.querySelector('.nav-overflow-toggle') + expect(toggle.getAttribute('data-bs-placement')).toEqual(maliciousPlacement) + expect(navEl.querySelector('img.broken-out')).toBeNull() + expect(window.__navPlacementXss).toBeUndefined() + + navOverflow.dispose() + }) + + it('should sanitize markup from [data-bs-overflow-icon]', () => { + fixtureEl.innerHTML = [ + '' + ].join('') + + const navEl = fixtureEl.querySelector('[data-bs-toggle="nav-overflow"]') + const navOverflow = new NavOverflow(navEl) + + const iconContainer = navEl.querySelector('.nav-overflow-icon') + expect(iconContainer.querySelector('img')).toBeNull() + expect(iconContainer.innerHTML).not.toMatch(/onerror/i) + expect(iconContainer.querySelector('i.bi-ok')).not.toBeNull() + expect(window.__navCustomIconXss).toBeUndefined() + + navOverflow.dispose() + }) }) describe('collapseBelow', () => { diff --git a/js/tests/unit/util/sanitizer.spec.js b/js/tests/unit/util/sanitizer.spec.js index ccf5c5cf6dd4..f4ac5c85f797 100644 --- a/js/tests/unit/util/sanitizer.spec.js +++ b/js/tests/unit/util/sanitizer.spec.js @@ -1,4 +1,4 @@ -import { DefaultAllowlist, sanitizeHtml } from '../../../src/util/sanitizer.js' +import { DefaultAllowlist, DefaultIconAllowlist, sanitizeHtml } from '../../../src/util/sanitizer.js' describe('Sanitizer', () => { describe('sanitizeHtml', () => { @@ -165,5 +165,40 @@ describe('Sanitizer', () => { expect(firstResult).toContain('src') expect(secondResult).toContain('src') }) + + it('should keep safe SVG icon markup with DefaultIconAllowlist', () => { + const template = '' + + const result = sanitizeHtml(template, DefaultIconAllowlist, null) + + expect(result).toContain(' { + const template = '' + + const result = sanitizeHtml(template, DefaultIconAllowlist, null) + + expect(result).toContain('class="ok"') + expect(result).toContain(' from icon HTML to avoid external SVG loads', () => { + const template = '' + + const result = sanitizeHtml(template, DefaultIconAllowlist, null) + + expect(result).not.toContain('...'` | SVG or HTML icon for the overflow toggle button. Overridden by a child element with `data-bs-overflow-icon` if present. | +| `moreText` | string | `'More'` | Text label for the overflow toggle button. Inserted as plain text. | +| `moreIcon` | string | `'...'` | SVG or HTML icon for the overflow toggle button. Passed through the [icon content sanitizer]([[docsref:/getting-started/javascript#sanitizer]]) before insertion. Overridden by a child element with `data-bs-overflow-icon` if present (also sanitized). | | `threshold` | number | `0` | Minimum number of items to keep visible before showing overflow. | diff --git a/site/src/content/docs/forms/chips.mdx b/site/src/content/docs/forms/chips.mdx index e9648d93d55b..e6e56972985f 100644 --- a/site/src/content/docs/forms/chips.mdx +++ b/site/src/content/docs/forms/chips.mdx @@ -198,7 +198,7 @@ Options can be passed via data attributes or JavaScript: | `maxChips` | number \| null | `null` | Maximum number of chips allowed. `null` for unlimited. | | `placeholder` | string | `''` | Placeholder text for dynamically created inputs. | | `dismissible` | boolean | `true` | Add dismiss buttons to created chips. | -| `dismissIcon` | string | `'...'` | HTML string for the dismiss button icon. | +| `dismissIcon` | string | `'...'` | HTML string for the dismiss button icon. Passed through the [icon content sanitizer]([[docsref:/getting-started/javascript#sanitizer]]) before insertion. | | `createOnBlur` | boolean | `true` | Create chip from input value when the input loses focus. | diff --git a/site/src/content/docs/getting-started/javascript.mdx b/site/src/content/docs/getting-started/javascript.mdx index d39b92f7011f..ed1bbdc7bc4a 100644 --- a/site/src/content/docs/getting-started/javascript.mdx +++ b/site/src/content/docs/getting-started/javascript.mdx @@ -306,13 +306,19 @@ Every Bootstrap plugin exposes the following methods and static properties. ## Sanitizer -Our tooltip and popover components are able to render arbitrary HTML to the page if configured to do so. -To prevent cross-site scripting (XSS) attacks, these components use our built-in content sanitizer to sanitize any options which accept HTML before they are rendered to the page. Content sanitization is enabled by default. +Some JavaScript components can render or accept arbitrary HTML in their configuration. To prevent cross-site scripting (XSS) attacks, these components use our built-in content sanitizer to sanitize any options which accept HTML before they are rendered to the page. Content sanitization is enabled by default. In particular: -The tags and attributes allowed by default are as follows. Any tags or attributes not explicitly allowed will be removed during sanitization: +- Popovers and Tooltips can render arbitrary HTML to the page if configured to do so. +- Chips and Nav overflow can accept HTML for their icon options (`dismissIcon`, `moreIcon`, and `[data-bs-overflow-icon]`). + +The tags and attributes allowed by default for tooltip and popover content are as follows. Any tags or attributes not explicitly allowed will be removed during sanitization: +Icon options use a separate, tighter allowlist that covers the default SVG icons and common inline-icon markup: + + + **Exercise caution when using these advanced options.** Refer to [OWASP’s Cross Site Scripting Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html) for more information. Vulnerabilities caused solely by disabling or modifying content sanitization are not considered within scope for Bootstrap’s security model.