CVE-2021-21333: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Synapse is a Matrix reference homeserver written in python (pypi package matrix-synapse). Matrix is an ecosystem for open federated Instant Messaging and VoIP. In Synapse before version 1.27.0, the notification emails sent for notifications for missed messages or for an expiring account are subject to HTML injection. In the case of the notification for missed messages, this could allow an attacker to insert forged content into the email. The account expiry feature is not enabled by default and the HTML injection is not controllable by an attacker. This is fixed in version 1.27.0.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2021-03-26T20:15Z
Repository: https://github.com/matrix-org/synapse
Language: Python
Files changed: UPGRADE.rst, changelog.d/9200.misc, synapse/config/_base.py, synapse/config/captcha.py, synapse/config/consent_config.py, synapse/config/registration.py, synapse/push/mailer.py, synapse/res/templates/sso_auth_bad_user.html, synapse/res/templates/sso_auth_confirm.html, synapse/res/templates/sso_error.html, synapse/res/templates/sso_login_idp_picker.html, synapse/res/templates/sso_redirect_confirm.html
Commit date: 2021-01-27 10:59:50 -0500
Commit message: Clean-up the template loading code. (#9200)

* Enables autoescape by default for HTML files.
* Adds a new read_template method for reading a single template.
* Some logic clean-up.
Diff: diff --git a/UPGRADE.rst b/UPGRADE.rst
index d09dbd4e21..e62e647a1d 100644
--- a/UPGRADE.rst
+++ b/UPGRADE.rst
@@ -83,10 +83,47 @@ for example:
 
      # replace `1.3.0` and `stretch` accordingly:
      wget https://packages.matrix.org/debian/pool/main/m/matrix-synapse-py3/matrix-synapse-py3_1.3.0+stretch1_amd64.deb
      dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb
 
+Upgrading to v1.27.0
+====================
+
+Changes to HTML templates
+-------------------------
+
+The HTML templates for SSO and email notifications now have `Jinja2's autoescape <https://jinja.palletsprojects.com/en/2.11.x/api/#autoescaping>`_
+enabled for files ending in ``.html``, ``.htm``, and ``.xml``. If you hae customised
+these templates and see issues when viewing them you might need to update them.
+It is expected that most configurations will need no changes.
+
+If you have customised the templates *names* for these templates it is recommended
+to verify they end in ``.html`` to ensure autoescape is enabled.
+
+The above applies to the following templates:
+
+* ``add_threepid.html``
+* ``add_threepid_failure.html``
+* ``add_threepid_success.html``
+* ``notice_expiry.html``
+* ``notice_expiry.html``
+* ``notif_mail.html`` (which, by default, includes ``room.html`` and ``notif.html``)
+* ``password_reset.html``
+* ``password_reset_confirmation.html``
+* ``password_reset_failure.html``
+* ``password_reset_success.html``
+* ``registration.html``
+* ``registration_failure.html``
+* ``registration_success.html``
+* ``sso_account_deactivated.html``
+* ``sso_auth_bad_user.html``
+* ``sso_auth_confirm.html``
+* ``sso_auth_success.html``
+* ``sso_error.html``
+* ``sso_login_idp_picker.html``
+* ``sso_redirect_confirm.html``
+
 Upgrading to v1.26.0
 ====================
 
 Rolling back to v1.25.0 after a failed upgrade
 ----------------------------------------------
diff --git a/changelog.d/9200.misc b/changelog.d/9200.misc
new file mode 100644
index 0000000000..5f239ff9da
--- /dev/null
+++ b/changelog.d/9200.misc
@@ -0,0 +1 @@
+Clean-up template loading code.
diff --git a/synapse/config/_base.py b/synapse/config/_base.py
index 94144efc87..6a0768ce00 100644
--- a/synapse/config/_base.py
+++ b/synapse/config/_base.py
@@ -201,40 +201,54 @@ class Config:
     def read_file(cls, file_path, config_name):
         cls.check_file(file_path, config_name)
         with open(file_path) as file_stream:
             return file_stream.read()
 
+    def read_template(self, filename: str) -> jinja2.Template:
+        """Load a template file from disk.
+
+        This function will attempt to load the given template from the default Synapse
+        template directory.
+
+        Files read are treated as Jinja templates. The templates is not rendered yet
+        and has autoescape enabled.
+
+        Args:
+            filename: A template filename to read.
+
+        Raises:
+            ConfigError: if the file's path is incorrect or otherwise cannot be read.
+
+        Returns:
+            A jinja2 template.
+        """
+        return self.read_templates([filename])[0]
+
     def read_templates(
-        self,
-        filenames: List[str],
-        custom_template_directory: Optional[str] = None,
-        autoescape: bool = False,
+        self, filenames: List[str], custom_template_directory: Optional[str] = None,
     ) -> List[jinja2.Template]:
         """Load a list of template files from disk using the given variables.
 
         This function will attempt to load the given templates from the default Synapse
         template directory. If `custom_template_directory` is supplied, that directory
         is tried first.
 
-        Files read are treated as Jinja templates. These templates are not rendered yet.
+        Files read are treated as Jinja templates. The templates are not rendered yet
+        and have autoescape enabled.
 
         Args:
             filenames: A list of template filenames to read.
 
             custom_template_directory: A directory to try to look for the templates
                 before using the default Synapse template directory instead.
 
-            autoescape: Whether to autoescape variables before inserting them into the
-                template.
-
         Raises:
             ConfigError: if the file's path is incorrect or otherwise cannot be read.
 
         Returns:
             A list of jinja2 templates.
         """
-        templates = []
         search_directories = [self.default_template_dir]
 
         # The loader will first look in the custom template directory (if specified) for the
         # given filename. If it doesn't find it, it will use the default template dir instead
         if custom_template_directory:
@@ -247,26 +261,22 @@ class Config:
 
             # Search the custom template directory as well
             search_directories.insert(0, custom_template_directory)
 
         loader = jinja2.FileSystemLoader(search_directories)
-        env = jinja2.Environment(loader=loader, autoescape=autoescape)
+        env = jinja2.Environment(loader=loader, autoescape=jinja2.select_autoescape(),)
 
         # Update the environment with our custom filters
         env.filters.update(
             {
                 "format_ts": _format_ts_filter,
                 "mxc_to_http": _create_mxc_to_http_filter(self.public_baseurl),
             }
         )
 
-        for filename in filenames:
-            # Load the template
-            template = env.get_template(filename)
-            templates.append(template)
-
-        return templates
+        # Load the templates
+        return [env.get_template(filename) for filename in filenames]
 
 
 def _format_ts_filter(value: int, format: str):
     return time.strftime(format, time.localtime(value / 1000))
 
diff --git a/synapse/config/captcha.py b/synapse/config/captcha.py
index cb00958165..9e48f865cc 100644
--- a/synapse/config/captcha.py
+++ b/synapse/config/captcha.py
@@ -26,13 +26,11 @@ class CaptchaConfig(Config):
         )
         self.recaptcha_siteverify_api = config.get(
             "recaptcha_siteverify_api",
             "https://www.recaptcha.net/recaptcha/api/siteverify",
         )
-        self.recaptcha_template = self.read_templates(
-            ["recaptcha.html"], autoescape=True
-        )[0]
+        self.recaptcha_template = self.read_template("recaptcha.html")
 
     def generate_config_section(self, **kwargs):
         return """\
         ## Captcha ##
         # See docs/CAPTCHA_SETUP.md for full details of configuring this.
diff --git a/synapse/config/consent_config.py b/synapse/config/consent_config.py
index 6efa59b110..c47f364b14 100644
--- a/synapse/config/consent_config.py
+++ b/synapse/config/consent_config.py
@@ -87,11 +87,11 @@ class ConsentConfig(Config):
         self.user_consent_at_registration = False
         self.user_consent_policy_name = "Privacy Policy"
 
     def read_config(self, config, **kwargs):
         consent_config = config.get("user_consent")
-        self.terms_template = self.read_templates(["terms.html"], autoescape=True)[0]
+        self.terms_template = self.read_template("terms.html")
 
         if consent_config is None:
             return
         self.user_consent_version = str(consent_config["version"])
         self.user_consent_template_dir = self.abspath(consent_config["template_dir"])
diff --git a/synapse/config/registration.py b/synapse/config/registration.py
index 4bfc69cb7a..ac48913a0b 100644
--- a/synapse/config/registration.py
+++ b/synapse/config/registration.py
@@ -174,13 +174,11 @@ class RegistrationConfig(Config):
         if session_lifetime is not None:
             session_lifetime = self.parse_duration(session_lifetime)
         self.session_lifetime = session_lifetime
 
         # The success template used during fallback auth.
-        self.fallback_success_template = self.read_templates(
-            ["auth_success.html"], autoescape=True
-        )[0]
+        self.fallback_success_template = self.read_template("auth_success.html")
 
     def generate_config_section(self, generate_secrets=False, **kwargs):
         if generate_secrets:
             registration_shared_secret = 'registration_shared_secret: "%s"' % (
                 random_string_with_symbols(50),
diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py
index 4d875dcb91..745b1dde94 100644
--- a/synapse/push/mailer.py
+++ b/synapse/push/mailer.py
@@ -666,10 +666,19 @@ class Mailer:
             urllib.parse.urlencode(params),
         )
 
 
 def safe_markup(raw_html: str) -> jinja2.Markup:
+    """
+    Sanitise a raw HTML string to a set of allowed tags and attributes, and linkify any bare URLs.
+
+    Args
+        raw_html: Unsafe HTML.
+
+    Returns:
+        A Markup object ready to safely use in a Jinja template.
+    """
     return jinja2.Markup(
         bleach.linkify(
             bleach.clean(
                 raw_html,
                 tags=ALLOWED_TAGS,
@@ -682,12 +691,17 @@ def safe_markup(raw_html: str) -> jinja2.Markup:
     )
 
 
 def safe_text(raw_text: str) -> jinja2.Markup:
     """
-    Process text: treat it as HTML but escape any tags (ie. just escape the
-    HTML) then linkify it.
+    Sanitise text (escape any HTML tags), and then linkify any bare URLs.
+
+    Args
+        raw_text: Unsafe text which might include HTML markup.
+
+    Returns:
+        A Markup object ready to safely use in a Jinja template.
     """
     return jinja2.Markup(
         bleach.linkify(bleach.clean(raw_text, tags=[], attributes={}, strip=False))
     )
 
diff --git a/synapse/res/templates/sso_auth_bad_user.html b/synapse/res/templates/sso_auth_bad_user.html
index 3611191bf9..f7099098c7 100644
--- a/synapse/res/templates/sso_auth_bad_user.html
+++ b/synapse/res/templates/sso_auth_bad_user.html
@@ -3,11 +3,11 @@
     <title>Authentication Failed</title>
 </head>
     <body>
         <div>
             <p>
-                We were unable to validate your <tt>{{server_name | e}}</tt> account via
+                We were unable to validate your <tt>{{ server_name }}</tt> account via
                 single-sign-on (SSO), because the SSO Identity Provider returned
                 different details than when you logged in.
             </p>
             <p>
                 Try the operation again, and ensure that you use the same details on
diff --git a/synapse/res/templates/sso_auth_confirm.html b/synapse/res/templates/sso_auth_confirm.html
index 0d9de9d465..4e7ca3a2ed 100644
--- a/synapse/res/templates/sso_auth_confirm.html
+++ b/synapse/res/templates/sso_auth_confirm.html
@@ -3,12 +3,12 @@
     <title>Authentication</title>
 </head>
     <body>
         <div>
             <p>
-                A client is trying to {{ description | e }}. To confirm this action,
-                <a href="{{ redirect_url | e }}">re-authenticate with single sign-on</a>.
+                A client is trying to {{ description }}. To confirm this action,
+                <a href="{{ redirect_url }}">re-authenticate with single sign-on</a>.
                 If you did not expect this, your account may be compromised!
             </p>
         </div>
     </body>
 </html>
diff --git a/synapse/res/templates/sso_error.html b/synapse/res/templates/sso_error.html
index 944bc9c9ca..af8459719a 100644
--- a/synapse/res/templates/sso_error.html
+++ b/synapse/res/templates/sso_error.html
@@ -10,11 +10,11 @@
     <p>You are not allowed to log in here.</p>
 {% else %}
     <p>
         There was an error during authentication:
     </p>
-    <div id="errormsg" style="margin:20px 80px">{{ error_description | e }}</div>
+    <div id="errormsg" style="margin:20px 80px">{{ error_description }}</div>
     <p>
         If you are seeing this page after clicking a link sent to you via email, make
         sure you only click the confirmation link once, and that you open the
         validation link in the same client you're logging in from.
     </p>
diff --git a/synapse/res/templates/sso_login_idp_picker.html b/synapse/res/templates/sso_login_idp_picker.html
index 5b38481012..62a640dad2 100644
--- a/synapse/res/templates/sso_login_idp_picker.html
+++ b/synapse/res/templates/sso_login_idp_picker.html
@@ -1,26 +1,26 @@
 <!DOCTYPE html>
 <html lang="en">
     <head>
         <meta charset="UTF-8">
         <link rel="stylesheet" href="/_matrix/static/client/login/style.css">
-        <title>{{server_name | e}} Login</title>
+        <title>{{ server_name }} Login</title>
     </head>
     <body>
         <div id="container">
-            <h1 id="title">{{server_name | e}} Login</h1>
+            <h1 id="title">{{ server_name }} Login</h1>
             <div class="login_flow">
                 <p>Choose one of the following identity providers:</p>
             <form>
-                <input type="hidden" name="redirectUrl" value="{{redirect_url | e}}">
+                <input type="hidden" name="redirectUrl" value="{{ redirect_url }}">
                 <ul class="radiobuttons">
 {% for p in providers %}
                     <li>
-                        <input type="radio" name="idp" id="prov{{loop.index}}" value="{{p.idp_id}}">
-                        <label for="prov{{loop.index}}">{{p.idp_name | e}}</label>
+                        <input type="radio" name="idp" id="prov{{ loop.index }}" value="{{ p.idp_id }}">
+                        <label for="prov{{ loop.index }}">{{ p.idp_name }}</label>
 {% if p.idp_icon %}
-                        <img src="{{p.idp_icon | mxc_to_http(32, 32)}}"/>
+                        <img src="{{ p.idp_icon | mxc_to_http(32, 32) }}"/>
 {% endif %}
                     </li>
 {% endfor %}
                 </ul>
                 <input type="submit" class="button button--full-width" id="button-submit" value="Submit">
diff --git a/synapse/res/templates/sso_redirect_confirm.html b/synapse/res/templates/sso_redirect_confirm.html
index 20a15e1e74..a45a50916c 100644
--- a/synapse/res/templates/sso_redirect_confirm.html
+++ b/synapse/res/templates/sso_redirect_confirm.html
@@ -3,12 +3,12 @@
 <head>
     <meta charset="UTF-8">
     <title>SSO redirect confirmation</title>
 </head>
     <body>
-        <p>The application at <span style="font-weight:bold">{{ display_url | e }}</span> is requesting full access to your <span style="font-weight:bold">{{ server_name }}</span> Matrix account.</p>
+        <p>The application at <span style="font-weight:bold">{{ display_url }}</span> is requesting full access to your <span style="font-weight:bold">{{ server_name }}</span> Matrix account.</p>
         <p>If you don't recognise this address, you should ignore this and close this tab.</p>
         <p>
-            <a href="{{ redirect_url | e }}">I trust this address</a>
+            <a href="{{ redirect_url }}">I trust this address</a>
         </p>
     </body>
-</html>
\ No newline at end of file
+</html>

Vulnerable code:         self,
        filenames: List[str],
        custom_template_directory: Optional[str] = None,
        autoescape: bool = False,
        Files read are treated as Jinja templates. These templates are not rendered yet.
            autoescape: Whether to autoescape variables before inserting them into the
                template.

        templates = []
        env = jinja2.Environment(loader=loader, autoescape=autoescape)
        for filename in filenames:
            # Load the template
            template = env.get_template(filename)
            templates.append(template)

        return templates
        self.recaptcha_template = self.read_templates(
            ["recaptcha.html"], autoescape=True
        )[0]
        self.terms_template = self.read_templates(["terms.html"], autoescape=True)[0]
        self.fallback_success_template = self.read_templates(
            ["auth_success.html"], autoescape=True
        )[0]
    Process text: treat it as HTML but escape any tags (ie. just escape the
    HTML) then linkify it.
                We were unable to validate your <tt>{{server_name | e}}</tt> account via
                A client is trying to {{ description | e }}. To confirm this action,
                <a href="{{ redirect_url | e }}">re-authenticate with single sign-on</a>.
    <div id="errormsg" style="margin:20px 80px">{{ error_description | e }}</div>
        <title>{{server_name | e}} Login</title>
            <h1 id="title">{{server_name | e}} Login</h1>
                <input type="hidden" name="redirectUrl" value="{{redirect_url | e}}">
                        <input type="radio" name="idp" id="prov{{loop.index}}" value="{{p.idp_id}}">
                        <label for="prov{{loop.index}}">{{p.idp_name | e}}</label>
                        <img src="{{p.idp_icon | mxc_to_http(32, 32)}}"/>
        <p>The application at <span style="font-weight:bold">{{ display_url | e }}</span> is requesting full access to your <span style="font-weight:bold">{{ server_name }}</span> Matrix account.</p>
            <a href="{{ redirect_url | e }}">I trust this address</a>
</html>
Fixed code: Upgrading to v1.27.0
====================

Changes to HTML templates
-------------------------

The HTML templates for SSO and email notifications now have `Jinja2's autoescape <https://jinja.palletsprojects.com/en/2.11.x/api/#autoescaping>`_
enabled for files ending in ``.html``, ``.htm``, and ``.xml``. If you hae customised
these templates and see issues when viewing them you might need to update them.
It is expected that most configurations will need no changes.

If you have customised the templates *names* for these templates it is recommended
to verify they end in ``.html`` to ensure autoescape is enabled.

The above applies to the following templates:

* ``add_threepid.html``
* ``add_threepid_failure.html``
* ``add_threepid_success.html``
* ``notice_expiry.html``
* ``notice_expiry.html``
* ``notif_mail.html`` (which, by default, includes ``room.html`` and ``notif.html``)
* ``password_reset.html``
* ``password_reset_confirmation.html``
* ``password_reset_failure.html``
* ``password_reset_success.html``
* ``registration.html``
* ``registration_failure.html``
* ``registration_success.html``
* ``sso_account_deactivated.html``
* ``sso_auth_bad_user.html``
* ``sso_auth_confirm.html``
* ``sso_auth_success.html``
* ``sso_error.html``
* ``sso_login_idp_picker.html``
* ``sso_redirect_confirm.html``

Clean-up template loading code.
    def read_template(self, filename: str) -> jinja2.Template:
        """Load a template file from disk.

        This function will attempt to load the given template from the default Synapse
        template directory.

        Files read are treated as Jinja templates. The templates is not rendered yet
        and has autoescape enabled.

        Args:
            filename: A template filename to read.

        Raises:
            ConfigError: if the file's path is incorrect or otherwise cannot be read.

        Returns:
            A jinja2 template.
        """
        return self.read_templates([filename])[0]

        self, filenames: List[str], custom_template_directory: Optional[str] = None,
        Files read are treated as Jinja templates. The templates are not rendered yet
        and have autoescape enabled.
        env = jinja2.Environment(loader=loader, autoescape=jinja2.select_autoescape(),)
        # Load the templates
        return [env.get_template(filename) for filename in filenames]
        self.recaptcha_template = self.read_template("recaptcha.html")
        self.terms_template = self.read_template("terms.html")
        self.fallback_success_template = self.read_template("auth_success.html")
    """
    Sanitise a raw HTML string to a set of allowed tags and attributes, and linkify any bare URLs.

    Args
        raw_html: Unsafe HTML.

    Returns:
        A Markup object ready to safely use in a Jinja template.
    """
    Sanitise text (escape any HTML tags), and then linkify any bare URLs.

    Args
        raw_text: Unsafe text which might include HTML markup.

    Returns:
        A Markup object ready to safely use in a Jinja template.
                We were unable to validate your <tt>{{ server_name }}</tt> account via
                A client is trying to {{ description }}. To confirm this action,
                <a href="{{ redirect_url }}">re-authenticate with single sign-on</a>.
    <div id="errormsg" style="margin:20px 80px">{{ error_description }}</div>
        <title>{{ server_name }} Login</title>
            <h1 id="title">{{ server_name }} Login</h1>
                <input type="hidden" name="redirectUrl" value="{{ redirect_url }}">
                        <input type="radio" name="idp" id="prov{{ loop.index }}" value="{{ p.idp_id }}">
                        <label for="prov{{ loop.index }}">{{ p.idp_name }}</label>
                        <img src="{{ p.idp_icon | mxc_to_http(32, 32) }}"/>
        <p>The application at <span style="font-weight:bold">{{ display_url }}</span> is requesting full access to your <span style="font-weight:bold">{{ server_name }}</span> Matrix account.</p>
            <a href="{{ redirect_url }}">I trust this address</a>
</html>