diff --git a/src/components/rich_content/rich_content.jsx b/src/components/rich_content/rich_content.jsx index 0739afb3..73feb294 100644 --- a/src/components/rich_content/rich_content.jsx +++ b/src/components/rich_content/rich_content.jsx @@ -194,14 +194,16 @@ export default { // Turn data-mfm- attributes into a string for the `style` attribute // If they have a value different than `true`, they need to be added to `style` // e.g. `attrs={'data-mfm-some': '1deg', 'data-mfm-thing': '5s'}` => "--mfm-some: 1deg;--mfm-thing: 5s;" + // Note that we only add the value to `style` when they contain only letters, numbers, dot, hash, or plus or minus signs + // At the moment of writing, this should be enough for legitimite purposes and reduces the chance of injection by using special characters let mfm_style = Object.keys(attrs).filter( - (key) => key.startsWith('data-mfm-') && attrs[key] !== true + (key) => key.startsWith('data-mfm-') && attrs[key] !== true && /^[a-zA-Z0-9.\-+#]*$/.test(attrs[key]) ).map( (key) => '--mfm-' + key.substr(9) + ': ' + attrs[key] + ';' - ).reduce((a,v) => a+v, "") - if (mfm_style !== "") { + ).reduce((a,v) => a+v, '') + if (mfm_style !== '') { return [ - opener.slice(0,-1) + " style=\"" + mfm_style + "\">", + opener.slice(0,-1) + ' style="' + mfm_style + '">', children.map(processItem), closer ] diff --git a/test/unit/specs/components/rich_content.spec.js b/test/unit/specs/components/rich_content.spec.js index ea26ee43..4c01c1e1 100644 --- a/test/unit/specs/components/rich_content.spec.js +++ b/test/unit/specs/components/rich_content.spec.js @@ -40,6 +40,35 @@ describe('RichContent', () => { expect(wrapper.html().replace(/\n/g, '')).to.eql(compwrap(html)) }) + it('does not allow injection through MFM data- attributes', () => { + const html_ok = 'brrr' + const expected_ok = 'brrr' + const wrapper_ok = shallowMount(RichContent, { + global, + props: { + attentions, + handleLinks: true, + greentext: true, + emoji: [], + html: html_ok + } + }) + const html_nok = 'brrr' + const wrapper_nok = shallowMount(RichContent, { + global, + props: { + attentions, + handleLinks: true, + greentext: true, + emoji: [], + html: html_nok + } + }) + + expect(wrapper_ok.html()).to.eql(compwrap(expected_ok)) + expect(wrapper_nok.html()).to.eql(compwrap(html_nok)) + }) + it('unescapes everything as needed', () => { const html = [ p('Testing 'em all'),