From 6666a273a4a12b1b10efcb80e42504cdef50c0a5 Mon Sep 17 00:00:00 2001
From: ilja <git@ilja.space>
Date: Sun, 11 Aug 2024 17:48:36 +0200
Subject: [PATCH] MFM only use sanitised data-* attribute values

We take the value from a data-* attribute and then add this to the style attribute.
This will probably be OK in most cases, but just to be sure, we check for "weird" characters first.
For now we only allow letters, numbers, dot, hash, and plus and minus sign, because those are the ones I currently know of who are used in MFM.
The data-* attribute remains because it was already considered proper HTML as-is.
---
 src/components/rich_content/rich_content.jsx  | 10 ++++---
 .../specs/components/rich_content.spec.js     | 29 +++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)

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 = '<span class="mfm-spin" data-mfm-speed="-0.2s" data-mfm-color="#000" data-mfm-deg="+30">brrr</span>'
+    const expected_ok = '<span class="mfm-spin" data-mfm-speed="-0.2s" data-mfm-color="#000" data-mfm-deg="+30" style="--mfm-speed: -0.2s; --mfm-color: #000; --mfm-deg: +30;">brrr</span>'
+    const wrapper_ok = shallowMount(RichContent, {
+      global,
+      props: {
+        attentions,
+        handleLinks: true,
+        greentext: true,
+        emoji: [],
+        html: html_ok
+      }
+    })
+    const html_nok = '<span class="mfm-spin" data-mfm-speed="<" data-mfm-color="\\">brrr</span>'
+    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 &#39;em all'),