diff --git a/app/assets/javascripts/lib/dompurify.js b/app/assets/javascripts/lib/dompurify.js index 3861007d64640a3a72faed0ab0a8214fd234c0e8..e072f06429e864e9018eac7fa487bee4abb2cffa 100644 --- a/app/assets/javascripts/lib/dompurify.js +++ b/app/assets/javascripts/lib/dompurify.js @@ -13,6 +13,7 @@ const isValidCssColor = (color) => { export const defaultConfig = { // Safely allow SVG tags ADD_TAGS: ['use', 'gl-emoji', 'copy-code'], + ADD_ATTR: ['temp-href-target', 'temp-bg'], // Prevent possible XSS attacks with data-* attributes used by @rails/ujs // See https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1421 [CLOSED] FORBID_ATTR: [ @@ -25,6 +26,8 @@ export const defaultConfig = { 'data-disable', 'data-turbo', ], + // FORBID_ATTR can be removed after this change is shippable + ALLOW_DATA_ATTR: false, FORBID_TAGS: ['style', 'mstyle', 'form'], ALLOW_UNKNOWN_PROTOCOLS: true, }; @@ -102,7 +105,7 @@ addHook('afterSanitizeAttributes', (node) => { } }); -const TEMPORARY_ATTRIBUTE = 'data-temp-href-target'; +const TEMPORARY_ATTRIBUTE = 'temp-href-target'; addHook('beforeSanitizeAttributes', (node, _, config) => { if (node.tagName === 'A' && node.hasAttribute('target')) { @@ -119,7 +122,7 @@ addHook('beforeSanitizeAttributes', (node, _, config) => { // Only preserve the background color if it's valid. if (isValidCssColor(bgColor)) { // eslint-disable-next-line no-param-reassign - node.dataset.tempBg = bgColor; + node.setAttribute('temp-bg', bgColor); } node.removeAttribute('style'); } @@ -136,16 +139,17 @@ addHook('afterSanitizeAttributes', (node, _, config) => { } // Restore background-color on GlLabel when style tags are forbidden. + const tempBg = node.getAttribute('temp-bg'); if ( config.FORBID_TAGS.includes('style') && node.classList?.contains('gl-label-text') && - node.dataset.tempBg && - isValidCssColor(node.dataset.tempBg) + tempBg && + isValidCssColor(tempBg) ) { // eslint-disable-next-line no-param-reassign - node.style.backgroundColor = node.dataset.tempBg; + node.style.backgroundColor = tempBg; // eslint-disable-next-line no-param-reassign - delete node.dataset.tempBg; + node.removeAttribute('temp-bg'); } }); diff --git a/spec/frontend/lib/dompurify_spec.js b/spec/frontend/lib/dompurify_spec.js index 9178aa65689d3ce6cbff0b740fedad30ae139854..55a1e123d03b36e86b91f1def603fd94bd627de5 100644 --- a/spec/frontend/lib/dompurify_spec.js +++ b/spec/frontend/lib/dompurify_spec.js @@ -167,10 +167,10 @@ describe('~/lib/dompurify', () => { expect(sanitize(htmlHref)).toBe('hello'); }); - it.each(acceptedDataAttrs)('does not remove %s attributes', (attr) => { + it.each(acceptedDataAttrs)('removes %s attributes', (attr) => { const attrWithValue = `${attr}="true"`; const htmlHref = `hello`; - expect(sanitize(htmlHref)).toBe(`hello`); + expect(sanitize(htmlHref)).toBe(`hello`); }); });