Commit 4ce9841f authored by Miguel Rincon's avatar Miguel Rincon

Merge branch 'djadmin-dompurify-data-attrs' into 'master'

Disallow specific data attributes from DOMPurify's default config [RUN AS-IF-FOSS] [RUN ALL RSPEC]

See merge request gitlab-org/gitlab!65301
parents 028f5170 d9604942
...@@ -7,6 +7,8 @@ const defaultConfig = { ...@@ -7,6 +7,8 @@ const defaultConfig = {
ADD_TAGS: ['use'], ADD_TAGS: ['use'],
}; };
const forbiddenDataAttrs = ['data-remote', 'data-url', 'data-type', 'data-method'];
// Only icons urls from `gon` are allowed // Only icons urls from `gon` are allowed
const getAllowedIconUrls = (gon = window.gon) => const getAllowedIconUrls = (gon = window.gon) =>
[gon.sprite_file_icons, gon.sprite_icons].filter(Boolean); [gon.sprite_file_icons, gon.sprite_icons].filter(Boolean);
...@@ -44,10 +46,19 @@ const sanitizeSvgIcon = (node) => { ...@@ -44,10 +46,19 @@ const sanitizeSvgIcon = (node) => {
removeUnsafeHref(node, 'xlink:href'); removeUnsafeHref(node, 'xlink:href');
}; };
const sanitizeHTMLAttributes = (node) => {
forbiddenDataAttrs.forEach((attr) => {
if (node.hasAttribute(attr)) {
node.removeAttribute(attr);
}
});
};
addHook('afterSanitizeAttributes', (node) => { addHook('afterSanitizeAttributes', (node) => {
if (node.tagName.toLowerCase() === 'use') { if (node.tagName.toLowerCase() === 'use') {
sanitizeSvgIcon(node); sanitizeSvgIcon(node);
} }
sanitizeHTMLAttributes(node);
}); });
export const sanitize = (val, config = defaultConfig) => dompurifySanitize(val, config); export const sanitize = (val, config = defaultConfig) => dompurifySanitize(val, config);
...@@ -30,6 +30,9 @@ const unsafeUrls = [ ...@@ -30,6 +30,9 @@ const unsafeUrls = [
`https://evil.url/${absoluteGon.sprite_file_icons}`, `https://evil.url/${absoluteGon.sprite_file_icons}`,
]; ];
const forbiddenDataAttrs = ['data-remote', 'data-url', 'data-type', 'data-method'];
const acceptedDataAttrs = ['data-random', 'data-custom'];
describe('~/lib/dompurify', () => { describe('~/lib/dompurify', () => {
let originalGon; let originalGon;
...@@ -95,4 +98,17 @@ describe('~/lib/dompurify', () => { ...@@ -95,4 +98,17 @@ describe('~/lib/dompurify', () => {
expect(sanitize(htmlXlink)).toBe(expectedSanitized); expect(sanitize(htmlXlink)).toBe(expectedSanitized);
}); });
}); });
describe('handles data attributes correctly', () => {
it.each(forbiddenDataAttrs)('removes %s attributes', (attr) => {
const htmlHref = `<a ${attr}="true">hello</a>`;
expect(sanitize(htmlHref)).toBe('<a>hello</a>');
});
it.each(acceptedDataAttrs)('does not remove %s attributes', (attr) => {
const attrWithValue = `${attr}="true"`;
const htmlHref = `<a ${attrWithValue}>hello</a>`;
expect(sanitize(htmlHref)).toBe(`<a ${attrWithValue}>hello</a>`);
});
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment