From 4364d6069996191de7dfc71d6cb1ef6548596fd4 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 21 Jan 2021 13:41:27 -0500
Subject: [PATCH 1/7] Enable autoescape for email templates.

---
 synapse/config/_base.py       |  5 ++++-
 synapse/config/emailconfig.py |  9 +++++++--
 synapse/push/mailer.py        | 18 ++++++++++++++++--
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/synapse/config/_base.py b/synapse/config/_base.py
index 94144efc87b3..1263bbbf7683 100644
--- a/synapse/config/_base.py
+++ b/synapse/config/_base.py
@@ -249,7 +249,10 @@ def read_templates(
             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() if autoescape else False,
+        )
 
         # Update the environment with our custom filters
         env.filters.update(
diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py
index 6a487afd3495..e98e9331879c 100644
--- a/synapse/config/emailconfig.py
+++ b/synapse/config/emailconfig.py
@@ -246,6 +246,7 @@ def read_config(self, config, **kwargs):
                     add_threepid_template_success_html,
                 ],
                 template_dir,
+                autoescape=True,
             )
 
             # Render templates that do not contain any placeholders
@@ -281,7 +282,9 @@ def read_config(self, config, **kwargs):
                 self.email_notif_template_html,
                 self.email_notif_template_text,
             ) = self.read_templates(
-                [notif_template_html, notif_template_text], template_dir,
+                [notif_template_html, notif_template_text],
+                template_dir,
+                autoescape=True,
             )
 
             self.email_notif_for_new_users = email_config.get(
@@ -303,7 +306,9 @@ def read_config(self, config, **kwargs):
                 self.account_validity_template_html,
                 self.account_validity_template_text,
             ) = self.read_templates(
-                [expiry_template_html, expiry_template_text], template_dir,
+                [expiry_template_html, expiry_template_text],
+                template_dir,
+                autoescape=True,
             )
 
         subjects_config = email_config.get("subjects", {})
diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py
index 4d875dcb91ff..745b1dde9459 100644
--- a/synapse/push/mailer.py
+++ b/synapse/push/mailer.py
@@ -668,6 +668,15 @@ def make_unsubscribe_link(
 
 
 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(
@@ -684,8 +693,13 @@ 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))

From ca6e3b1d051e1c0078a58cd71c81c0512a980912 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 21 Jan 2021 13:50:25 -0500
Subject: [PATCH 2/7] Enable autoescape for SSO templates.

---
 synapse/config/sso.py                           |  1 +
 synapse/res/templates/sso_auth_bad_user.html    |  2 +-
 synapse/res/templates/sso_auth_confirm.html     |  4 ++--
 synapse/res/templates/sso_error.html            |  2 +-
 synapse/res/templates/sso_login_idp_picker.html | 12 ++++++------
 synapse/res/templates/sso_redirect_confirm.html |  6 +++---
 6 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/synapse/config/sso.py b/synapse/config/sso.py
index 59be825532f5..77bae29abc1c 100644
--- a/synapse/config/sso.py
+++ b/synapse/config/sso.py
@@ -49,6 +49,7 @@ def read_config(self, config, **kwargs):
                 "sso_auth_bad_user.html",
             ],
             template_dir,
+            autoescape=True,
         )
 
         # These templates have no placeholders, so render them here
diff --git a/synapse/res/templates/sso_auth_bad_user.html b/synapse/res/templates/sso_auth_bad_user.html
index 3611191bf99d..f7099098c7f3 100644
--- a/synapse/res/templates/sso_auth_bad_user.html
+++ b/synapse/res/templates/sso_auth_bad_user.html
@@ -5,7 +5,7 @@
     <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>
diff --git a/synapse/res/templates/sso_auth_confirm.html b/synapse/res/templates/sso_auth_confirm.html
index 0d9de9d46528..4e7ca3a2eda5 100644
--- a/synapse/res/templates/sso_auth_confirm.html
+++ b/synapse/res/templates/sso_auth_confirm.html
@@ -5,8 +5,8 @@
     <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>
diff --git a/synapse/res/templates/sso_error.html b/synapse/res/templates/sso_error.html
index 944bc9c9cab2..af8459719ae4 100644
--- a/synapse/res/templates/sso_error.html
+++ b/synapse/res/templates/sso_error.html
@@ -12,7 +12,7 @@
     <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
diff --git a/synapse/res/templates/sso_login_idp_picker.html b/synapse/res/templates/sso_login_idp_picker.html
index 5b384810123f..62a640dad25d 100644
--- a/synapse/res/templates/sso_login_idp_picker.html
+++ b/synapse/res/templates/sso_login_idp_picker.html
@@ -3,22 +3,22 @@
     <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 %}
diff --git a/synapse/res/templates/sso_redirect_confirm.html b/synapse/res/templates/sso_redirect_confirm.html
index 20a15e1e74ab..a45a50916c04 100644
--- a/synapse/res/templates/sso_redirect_confirm.html
+++ b/synapse/res/templates/sso_redirect_confirm.html
@@ -5,10 +5,10 @@
     <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>

From 6899434df00a3813a5820ba8c0c4fc90dfad76e4 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 21 Jan 2021 13:53:07 -0500
Subject: [PATCH 3/7] Remove unused parameter.

---
 synapse/config/_base.py          | 16 ++++------------
 synapse/config/captcha.py        |  4 +---
 synapse/config/consent_config.py |  2 +-
 synapse/config/emailconfig.py    |  9 ++-------
 synapse/config/registration.py   |  4 +---
 synapse/config/sso.py            |  1 -
 6 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/synapse/config/_base.py b/synapse/config/_base.py
index 1263bbbf7683..e1d3ff534c83 100644
--- a/synapse/config/_base.py
+++ b/synapse/config/_base.py
@@ -204,10 +204,7 @@ def read_file(cls, file_path, config_name):
             return file_stream.read()
 
     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.
 
@@ -215,7 +212,8 @@ def read_templates(
         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. These templates are not rendered yet
+        and have autoescape enabled.
 
         Args:
             filenames: A list of template filenames to read.
@@ -223,9 +221,6 @@ def read_templates(
             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.
 
@@ -249,10 +244,7 @@ def read_templates(
             search_directories.insert(0, custom_template_directory)
 
         loader = jinja2.FileSystemLoader(search_directories)
-        env = jinja2.Environment(
-            loader=loader,
-            autoescape=jinja2.select_autoescape() if autoescape else False,
-        )
+        env = jinja2.Environment(loader=loader, autoescape=jinja2.select_autoescape(),)
 
         # Update the environment with our custom filters
         env.filters.update(
diff --git a/synapse/config/captcha.py b/synapse/config/captcha.py
index cb009581651b..bc5eef49cb8d 100644
--- a/synapse/config/captcha.py
+++ b/synapse/config/captcha.py
@@ -28,9 +28,7 @@ def read_config(self, config, **kwargs):
             "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_templates(["recaptcha.html"])[0]
 
     def generate_config_section(self, **kwargs):
         return """\
diff --git a/synapse/config/consent_config.py b/synapse/config/consent_config.py
index 6efa59b110b0..551b889010d2 100644
--- a/synapse/config/consent_config.py
+++ b/synapse/config/consent_config.py
@@ -89,7 +89,7 @@ def __init__(self, *args):
 
     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_templates(["terms.html"])[0]
 
         if consent_config is None:
             return
diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py
index e98e9331879c..6a487afd3495 100644
--- a/synapse/config/emailconfig.py
+++ b/synapse/config/emailconfig.py
@@ -246,7 +246,6 @@ def read_config(self, config, **kwargs):
                     add_threepid_template_success_html,
                 ],
                 template_dir,
-                autoescape=True,
             )
 
             # Render templates that do not contain any placeholders
@@ -282,9 +281,7 @@ def read_config(self, config, **kwargs):
                 self.email_notif_template_html,
                 self.email_notif_template_text,
             ) = self.read_templates(
-                [notif_template_html, notif_template_text],
-                template_dir,
-                autoescape=True,
+                [notif_template_html, notif_template_text], template_dir,
             )
 
             self.email_notif_for_new_users = email_config.get(
@@ -306,9 +303,7 @@ def read_config(self, config, **kwargs):
                 self.account_validity_template_html,
                 self.account_validity_template_text,
             ) = self.read_templates(
-                [expiry_template_html, expiry_template_text],
-                template_dir,
-                autoescape=True,
+                [expiry_template_html, expiry_template_text], template_dir,
             )
 
         subjects_config = email_config.get("subjects", {})
diff --git a/synapse/config/registration.py b/synapse/config/registration.py
index 4bfc69cb7ad9..0c4c75c90e37 100644
--- a/synapse/config/registration.py
+++ b/synapse/config/registration.py
@@ -176,9 +176,7 @@ def read_config(self, config, **kwargs):
         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_templates(["auth_success.html"])[0]
 
     def generate_config_section(self, generate_secrets=False, **kwargs):
         if generate_secrets:
diff --git a/synapse/config/sso.py b/synapse/config/sso.py
index 77bae29abc1c..59be825532f5 100644
--- a/synapse/config/sso.py
+++ b/synapse/config/sso.py
@@ -49,7 +49,6 @@ def read_config(self, config, **kwargs):
                 "sso_auth_bad_user.html",
             ],
             template_dir,
-            autoescape=True,
         )
 
         # These templates have no placeholders, so render them here

From 31620d2bddba55ce6e4cb2b11ffe5809d09571a6 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 21 Jan 2021 13:55:20 -0500
Subject: [PATCH 4/7] Support loading a single template with a simpler method
 signature.

---
 synapse/config/_base.py          | 22 +++++++++++++++++++++-
 synapse/config/captcha.py        |  2 +-
 synapse/config/consent_config.py |  2 +-
 synapse/config/registration.py   |  2 +-
 4 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/synapse/config/_base.py b/synapse/config/_base.py
index e1d3ff534c83..8c672ceec7f3 100644
--- a/synapse/config/_base.py
+++ b/synapse/config/_base.py
@@ -203,6 +203,26 @@ def read_file(cls, 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,
     ) -> List[jinja2.Template]:
@@ -212,7 +232,7 @@ def read_templates(
         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:
diff --git a/synapse/config/captcha.py b/synapse/config/captcha.py
index bc5eef49cb8d..9e48f865cc23 100644
--- a/synapse/config/captcha.py
+++ b/synapse/config/captcha.py
@@ -28,7 +28,7 @@ def read_config(self, config, **kwargs):
             "recaptcha_siteverify_api",
             "https://www.recaptcha.net/recaptcha/api/siteverify",
         )
-        self.recaptcha_template = self.read_templates(["recaptcha.html"])[0]
+        self.recaptcha_template = self.read_template("recaptcha.html")
 
     def generate_config_section(self, **kwargs):
         return """\
diff --git a/synapse/config/consent_config.py b/synapse/config/consent_config.py
index 551b889010d2..c47f364b146d 100644
--- a/synapse/config/consent_config.py
+++ b/synapse/config/consent_config.py
@@ -89,7 +89,7 @@ def __init__(self, *args):
 
     def read_config(self, config, **kwargs):
         consent_config = config.get("user_consent")
-        self.terms_template = self.read_templates(["terms.html"])[0]
+        self.terms_template = self.read_template("terms.html")
 
         if consent_config is None:
             return
diff --git a/synapse/config/registration.py b/synapse/config/registration.py
index 0c4c75c90e37..ac48913a0bd1 100644
--- a/synapse/config/registration.py
+++ b/synapse/config/registration.py
@@ -176,7 +176,7 @@ def read_config(self, config, **kwargs):
         self.session_lifetime = session_lifetime
 
         # The success template used during fallback auth.
-        self.fallback_success_template = self.read_templates(["auth_success.html"])[0]
+        self.fallback_success_template = self.read_template("auth_success.html")
 
     def generate_config_section(self, generate_secrets=False, **kwargs):
         if generate_secrets:

From 86335912b15f3d82c6912a65c889cc1d9415d182 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 21 Jan 2021 14:04:30 -0500
Subject: [PATCH 5/7] Simplify some logic.

---
 synapse/config/_base.py | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/synapse/config/_base.py b/synapse/config/_base.py
index 8c672ceec7f3..6a0768ce000f 100644
--- a/synapse/config/_base.py
+++ b/synapse/config/_base.py
@@ -247,7 +247,6 @@ def read_templates(
         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
@@ -274,12 +273,8 @@ def read_templates(
             }
         )
 
-        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):

From 0508a3aa051407ce425bf44c4b427d661a13255e Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 21 Jan 2021 14:49:42 -0500
Subject: [PATCH 6/7] Newsfragment

---
 changelog.d/9200.misc | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/9200.misc

diff --git a/changelog.d/9200.misc b/changelog.d/9200.misc
new file mode 100644
index 000000000000..5f239ff9da2d
--- /dev/null
+++ b/changelog.d/9200.misc
@@ -0,0 +1 @@
+Clean-up template loading code.

From b4af9578a780f0f2ef85f60b4a7418d6b1f6af36 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 27 Jan 2021 08:26:20 -0500
Subject: [PATCH 7/7] Add upgrade notes.

---
 UPGRADE.rst | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/UPGRADE.rst b/UPGRADE.rst
index d09dbd4e212d..e62e647a1d34 100644
--- a/UPGRADE.rst
+++ b/UPGRADE.rst
@@ -85,6 +85,43 @@ for example:
      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
 ====================