escaped HTML entities like > were unescaped in the final mjml ou…

…tput
FelixSchwarz · Feb 21, 2024 · 74bd9ab · 74bd9ab
1 parent 136697f
commit 74bd9ab
Show file tree

Hide file tree

Showing 6 changed files with 147 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,10 @@
 
+0.11.0 (2024-02-xx)
+-------------------
+
+- security fix: escaped HTML entities like `&gt;` were unescaped in the final mjml output, leading to potential injection of untrusted user data (reported by @sh-at-cs)
+
+
 0.10.0 (2023-11-17)
 ------------------
 

diff --git a/mjml/elements/head/mj_style.py b/mjml/elements/head/mj_style.py
@@ -10,10 +10,15 @@ class MjStyle(HeadComponent):
     @classmethod
     def default_attrs(cls):
         return {
-            'inline'            : '',
+            'inline': '',
         }
 
     def handler(self):
         add = self.context['add']
         inline_attr = 'inlineStyle' if (self.get_attr('inline') == 'inline') else 'style'
-        add(inline_attr, self.getContent())
+        html_str = self.getContent()
+        # CSS can contain child selectors (e.g. "h1 > p") beautifulsoup only
+        # returns escaped entities. To make these selectors work, we need to
+        # unescape these.
+        css_str = html_str.replace('&gt;', '>')
+        add(inline_attr, css_str)
diff --git a/mjml/mjml2html.py b/mjml/mjml2html.py
@@ -116,7 +116,7 @@ def parse(_mjml, parentMjClass='', *, template_dir):
             classes = ignore_empty(attributes.get('mj-class', '').split(' '))
 
             # upstream parses text contents (+ comments) in mjml-parser-xml/index.js
-            content = _mjml.decode_contents(formatter=None)
+            content = _mjml.decode_contents()
 
             attributesClasses = {}
             for css_class in classes:
@@ -213,7 +213,7 @@ def _head_data_add(attr, *params):
                 for element in contentSoup.select(selector):
                     element[attrName] = value or ''
 
-        content = contentSoup.decode_contents(formatter=None)
+        content = contentSoup.decode_contents()
 
     content = skeleton(
         content=content,

diff --git a/tests/testdata/mj-text-escaped-html-expected.html b/tests/testdata/mj-text-escaped-html-expected.html
@@ -0,0 +1,120 @@
+<!doctype html>
+<html xmlns="http://www.w3.org/1999/xhtml" xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office">
+
+<head>
+  <title>
+  </title>
+  <!--[if !mso]><!-->
+  <meta http-equiv="X-UA-Compatible" content="IE=edge">
+  <!--<![endif]-->
+  <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
+  <meta name="viewport" content="width=device-width, initial-scale=1">
+  <style type="text/css">
+    #outlook a {
+      padding: 0;
+    }
+
+    body {
+      margin: 0;
+      padding: 0;
+      -webkit-text-size-adjust: 100%;
+      -ms-text-size-adjust: 100%;
+    }
+
+    table,
+    td {
+      border-collapse: collapse;
+      mso-table-lspace: 0pt;
+      mso-table-rspace: 0pt;
+    }
+
+    img {
+      border: 0;
+      height: auto;
+      line-height: 100%;
+      outline: none;
+      text-decoration: none;
+      -ms-interpolation-mode: bicubic;
+    }
+
+    p {
+      display: block;
+      margin: 13px 0;
+    }
+
+  </style>
+  <!--[if mso]>
+    <noscript>
+    <xml>
+    <o:OfficeDocumentSettings>
+      <o:AllowPNG/>
+      <o:PixelsPerInch>96</o:PixelsPerInch>
+    </o:OfficeDocumentSettings>
+    </xml>
+    </noscript>
+    <![endif]-->
+  <!--[if lte mso 11]>
+    <style type="text/css">
+      .mj-outlook-group-fix { width:100% !important; }
+    </style>
+    <![endif]-->
+  <!--[if !mso]><!-->
+  <link href="https://fonts.googleapis.com/css?family=Ubuntu:300,400,500,700" rel="stylesheet" type="text/css">
+  <style type="text/css">
+    @import url(https://fonts.googleapis.com/css?family=Ubuntu:300,400,500,700);
+
+  </style>
+  <!--<![endif]-->
+  <style type="text/css">
+    @media only screen and (min-width:480px) {
+      .mj-column-per-100 {
+        width: 100% !important;
+        max-width: 100%;
+      }
+    }
+
+  </style>
+  <style media="screen and (min-width:480px)">
+    .moz-text-html .mj-column-per-100 {
+      width: 100% !important;
+      max-width: 100%;
+    }
+
+  </style>
+  <style type="text/css">
+  </style>
+  <style type="text/css">
+  </style>
+</head>
+
+<body style="word-spacing:normal;">
+  <div style="">
+    <!--[if mso | IE]><table align="center" border="0" cellpadding="0" cellspacing="0" class="" role="presentation" style="width:600px;" width="600" ><tr><td style="line-height:0px;font-size:0px;mso-line-height-rule:exactly;"><![endif]-->
+    <div style="margin:0px auto;max-width:600px;">
+      <table align="center" border="0" cellpadding="0" cellspacing="0" role="presentation" style="width:100%;">
+        <tbody>
+          <tr>
+            <td style="direction:ltr;font-size:0px;padding:20px 0;text-align:center;">
+              <!--[if mso | IE]><table role="presentation" border="0" cellpadding="0" cellspacing="0"><tr><td class="" style="vertical-align:top;width:600px;" ><![endif]-->
+              <div class="mj-column-per-100 mj-outlook-group-fix" style="font-size:0px;text-align:left;direction:ltr;display:inline-block;vertical-align:top;width:100%;">
+                <table border="0" cellpadding="0" cellspacing="0" role="presentation" style="vertical-align:top;" width="100%">
+                  <tbody>
+                    <tr>
+                      <td align="left" style="font-size:0px;padding:10px 25px;word-break:break-word;">
+                        <div style="font-family:Ubuntu, Helvetica, Arial, sans-serif;font-size:13px;line-height:1;text-align:left;color:#000000;">Pretty unsafe: &lt;script&gt;</div>
+                      </td>
+                    </tr>
+                  </tbody>
+                </table>
+              </div>
+              <!--[if mso | IE]></td></tr></table><![endif]-->
+            </td>
+          </tr>
+        </tbody>
+      </table>
+    </div>
+    <!--[if mso | IE]></td></tr></table><![endif]-->
+  </div>
+</body>
+
+</html>
diff --git a/tests/testdata/mj-text-escaped-html.mjml b/tests/testdata/mj-text-escaped-html.mjml
@@ -0,0 +1,11 @@
+<mjml>
+  <mj-body>
+  <mj-section>
+    <mj-column>
+    <mj-text>
+      Pretty unsafe: &lt;script&gt;
+    </mj-text>
+    </mj-column>
+    </mj-section>
+  </mj-body>
+</mjml>
diff --git a/tests/upstream_alignment_test.py b/tests/upstream_alignment_test.py
@@ -48,6 +48,7 @@
     'mj-raw-head-with-tags',
     'mj-social',
     'mj-spacer',
+    'mj-text-escaped-html', # this test is security-critical
     'mj-wrapper',
 )