Add further js escape for data coming from emails

app/email_utils.py

@@ -1030,7 +1030,7 @@ def should_add_dkim_signature(domain: str) -> bool:
         return True
 
     custom_domain: CustomDomain = CustomDomain.get_by(domain=domain)
-    if custom_domain.dkim_verified:
+    if custom_domain and custom_domain.dkim_verified:
         return True
 
     return False

app/handler/dmarc.py

@@ -1,3 +1,4 @@
+import html
 import uuid
 from io import BytesIO
 from typing import Optional, Tuple
@@ -35,6 +36,7 @@ def apply_dmarc_policy_for_forward_phase(
     LOG.i(f"Spam check result in {spam_result}")
 
     from_header = get_header_unicode(msg[headers.FROM])
+    from_header = html.escape(from_header)
 
     warning_plain_text = """This email failed anti-phishing checks when it was received by SimpleLogin, be careful with its content.
 More info on https://simplelogin.io/docs/getting-started/anti-phishing/
@@ -86,8 +88,8 @@ def apply_dmarc_policy_for_forward_phase(
         DmarcCheckResult.reject,
     ):
         LOG.w(
-            f"dmarc forward: put email from {contact} to {alias} to quarantine. {spam_result.event_data()}, "
-            f"mail_from:{envelope.mail_from}, from_header: {msg[headers.FROM]}"
+            f"dmarc forward: put email from {contact.email} to {alias.email} to quarantine. {spam_result.event_data()}, "
+            f"mail_from:{envelope.mail_from}, from_header: {from_header}"
         )
         email_log = quarantine_dmarc_failed_forward_email(alias, contact, envelope, msg)
         Notification.create(

email_handler.py

@@ -33,6 +33,7 @@
 
 import argparse
 import email
+import html
 import time
 import uuid
 from email import encoders
@@ -872,9 +873,12 @@ def forward_email_to_mailbox(
         LOG.d("Use a generic subject for %s", mailbox)
         orig_subject = msg[headers.SUBJECT]
         orig_subject = get_header_unicode(orig_subject)
+        orig_subject = html.escape(orig_subject)
         add_or_replace_header(msg, "Subject", mailbox.generic_subject)
         sender = msg[headers.FROM]
         sender = get_header_unicode(sender)
+        sender = html.escape(sender)
+
         msg = add_header(
             msg,
             f"""Forwarded by SimpleLogin to {alias.email} from "{sender}" with "{orig_subject}" as subject""",

static/js/index.js

@@ -112,8 +112,10 @@ $(".pin-alias").change(async function () {
   }
 });
 
-async function handleNoteChange(aliasId, aliasEmail) {
-  const note = document.getElementById(`note-${aliasId}`).value;
+async function handleNoteChange(aliasId) {
+  const noteInput = document.getElementById(`note-${aliasId}`);
+  const note = noteInput.value;
+  const aliasEmail = noteInput.dataset.aliasEmail;
 
   try {
     let res = await fetch(`/api/aliases/${aliasId}`, {
@@ -143,8 +145,10 @@ function handleNoteBlur(aliasId) {
   document.getElementById(`note-focus-message-${aliasId}`).classList.add('d-none');
 }
 
-async function handleMailboxChange(aliasId, aliasEmail) {
-  const selectedOptions = document.getElementById(`mailbox-${aliasId}`).selectedOptions;
+async function handleMailboxChange(aliasId) {
+  const mailboxSelect = document.getElementById(`mailbox-${aliasId}`);
+  const aliasEmail = mailboxSelect.dataset.aliasEmail;
+  const selectedOptions = mailboxSelect.selectedOptions;
   const mailbox_ids = Array.from(selectedOptions).map((selectedOption) => selectedOption.value);
 
   if (mailbox_ids.length === 0) {
@@ -172,8 +176,10 @@ async function handleMailboxChange(aliasId, aliasEmail) {
 
 }
 
-async function handleDisplayNameChange(aliasId, aliasEmail) {
-  const name = document.getElementById(`alias-name-${aliasId}`).value;
+async function handleDisplayNameChange(aliasId) {
+  const nameInput = document.getElementById(`alias-name-${aliasId}`);
+  const name = nameInput.value;
+  const aliasEmail = nameInput.dataset.aliasEmail;
 
   try {
     let res = await fetch(`/api/aliases/${aliasId}`, {

templates/base.html

@@ -80,7 +80,10 @@
           type="text/css"
           href="/static/style.css?v={{ VERSION }}" />
     <script src="{{ url_for('static', filename='js/theme.js') }}"></script>
-    <script>toastr.options.closeButton = true;</script>
+    <script>
+      toastr.options.closeButton = true;
+      toastr.options.escapeHtml = true;
+    </script>
     <!-- For additional head -->
     {% block head %}{% endblock %}
   </head>
@@ -99,7 +102,7 @@
           <!-- Categories: success (green), info (blue), warning (yellow), danger (red) -->
           {% if messages %}
 
-            {% for category, message in messages %}<script>toastr.{{category }}("{{ message }}");</script>{% endfor %}
+            {% for category, message in messages %}<script>toastr.{{category }}({{ message | tojson }});</script>{% endfor %}
           {% endif %}
         {% endwith %}
       </div>

templates/dashboard/index.html

@@ -348,7 +348,8 @@
                     style="font-size: 12px"
                     rows="2"
                     placeholder="e.g. where the alias is used or why is it created"
-                    onchange="handleNoteChange({{ alias.id }}, '{{ alias.email }}')"
+                    data-alias-email="{{ alias.email }}"
+                    onchange="handleNoteChange({{ alias.id }})"
                     onfocus="handleNoteFocus({{ alias.id }})"
                     onblur="handleNoteBlur({{ alias.id }})">{{ alias.note or "" }}</textarea>
         </div>
@@ -426,7 +427,8 @@
                     class="mailbox-select"
                     multiple
                     name="mailbox"
-                    onchange="handleMailboxChange({{ alias.id }}, '{{ alias.email }}')">
+                    data-alias-email="{{ alias.email }}"
+                    onchange="handleMailboxChange({{ alias.id }})">
               {% for mailbox in mailboxes %}
 
                 <option value="{{ mailbox.id }}" {% if alias_info.contain_mailbox(mailbox.id) %}selected{% endif %}>{{ mailbox.email }}</option>
@@ -453,7 +455,8 @@
                  value="{{ alias.name or '' }}"
                  class="form-control"
                  placeholder="{{ alias.custom_domain.name or "Alias name" }}"
-                 onchange="handleDisplayNameChange({{ alias.id }}, '{{ alias.email }}')"
+                 data-alias-email="{{ alias.email }}"
+                 onchange="handleDisplayNameChange({{ alias.id }})"
                  onfocus="handleDisplayNameFocus({{ alias.id }})"
                  onblur="handleDisplayNameBlur({{ alias.id }})">
         </div>

templates/dashboard/notifications.html

@@ -11,7 +11,7 @@ <h1 class="h3">Notifications</h1>
 
         <div class="card">
           <div class="card-body">
-            <div class="h4">{{ notification.title | safe or "" }}</div>
+            <div class="h4">{{ notification.title or "" }}</div>
             <div style="width: 40em;
                         word-wrap:break-word;
                         white-space: normal;

templates/header.html

@@ -45,7 +45,16 @@
                class="mr-5 mb-2 float-right">See all notifications ➡</a>
             <div class="dropdown-item d-flex" v-for="notification in notifications">
               <div class="flex-grow-1">
-                <div v-html="notification.title || notification.message"
+                <!-- title is plain text; only message is server-rendered HTML -->
+                <div v-if="notification.title"
+                     v-text="notification.title"
+                     :class="!notification.read && 'font-weight-bold'"
+                     style="width: 40em;
+                            word-wrap:break-word;
+                            white-space: normal;
+                            overflow: hidden"></div>
+                <div v-else
+                     v-html="notification.message"
                      :class="!notification.read && 'font-weight-bold'"
                      style="width: 40em;
                             word-wrap:break-word;

templates/partials/toggle_contact.html

@@ -11,4 +11,4 @@
   {% endif %}
   </button>
 </form>
-{% if toast_msg %}<script>toastr.success("{{ toast_msg }}");</script>{% endif %}
+{% if toast_msg %}<script>toastr.success({{ toast_msg | tojson }});</script>{% endif %}

tests/dashboard/test_index.py

@@ -1,3 +1,8 @@
+import html
+import re
+
+from markupsafe import escape
+
 from flask import url_for
 
 from app import config
@@ -6,7 +11,7 @@
     Alias,
     Mailbox,
 )
-from tests.utils import fix_rate_limit_after_request, login
+from tests.utils import fix_rate_limit_after_request, login, random_token
 
 
 def test_create_random_alias_success(flask_client):
@@ -22,6 +27,35 @@ def test_create_random_alias_success(flask_client):
     assert Alias.filter(Alias.user_id == user.id).count() == 2
 
 
+def test_alias_email_is_never_injected_into_an_event_handler(flask_client):
+    """
+    An alias local part can be chosen by an outsider (directory / catch-all
+    auto-creation) and RFC 5322 atext allows ' and `. Interpolating alias.email
+    into an inline on*= handler is exploitable even with Jinja autoescape,
+    because the HTML parser entity-decodes attribute values *before* the
+    handler body is parsed as JS. Assert the email never reaches a JS context.
+    """
+    user = login(flask_client)
+
+    marker = f"alert`{random_token()}`"
+    alias = Alias.create_new_random(user)
+    # a valid address as far as email-validator is concerned
+    alias.email = f"poc+x'-{marker}-'@{config.EMAIL_DOMAIN}"
+    Session.commit()
+
+    r = flask_client.get(url_for("dashboard.index"))
+    assert r.status_code == 200
+    body = r.data.decode()
+
+    # the alias is rendered on the page, as escaped data
+    assert f'data-alias-email="{escape(alias.email)}"' in body
+
+    # ...and its local part appears in no inline event handler, even after the
+    # entity decoding the browser applies to attribute values
+    for handler_body in re.findall(r'\son\w+="([^"]*)"', body):
+        assert marker not in html.unescape(handler_body)
+
+
 def test_too_many_requests(flask_client):
     config.DISABLE_RATE_LIMIT = False
     login(flask_client)

tests/dashboard/test_notification.py

@@ -0,0 +1,84 @@
+from flask import url_for
+
+from app.db import Session
+from app.models import Notification
+from tests.api.utils import get_new_user_and_api_key
+from tests.utils import login, random_token
+
+# A notification title can contain data lifted verbatim from an inbound email's
+# From header (e.g. contact address in email_handler.py bounce notifications),
+# which is NOT constrained by email-validator the way an alias local part is.
+# It must never reach an HTML/JS sink unescaped, in any of the three places a
+# title is rendered.
+
+
+def test_notifications_list_page_escapes_title(flask_client):
+    user = login(flask_client)
+
+    marker = random_token()
+    payload = f"<img src=x onerror=alert`{marker}`>"
+    Notification.create(user_id=user.id, title=payload, message="body", commit=True)
+
+    r = flask_client.get(url_for("dashboard.notifications_route"))
+    assert r.status_code == 200
+    body = r.data.decode()
+
+    # the raw tag must not appear; only its escaped form may
+    assert payload not in body
+    assert "&lt;img" in body
+
+
+def test_single_notification_page_escapes_title(flask_client):
+    user = login(flask_client)
+
+    marker = random_token()
+    payload = f"<img src=x onerror=alert`{marker}`>"
+    notification_id = Notification.create(
+        user_id=user.id, title=payload, message="body", commit=True
+    ).id
+
+    r = flask_client.get(
+        url_for("dashboard.notification_route", notification_id=notification_id)
+    )
+    assert r.status_code == 200
+    body = r.data.decode()
+
+    assert payload not in body
+    assert "&lt;img" in body
+
+
+def test_notification_bell_renders_title_as_text_not_html(flask_client):
+    """
+    The bell dropdown (header.html, present on every dashboard page) is a Vue
+    component that fetches titles as JSON and binds them. Binding the title with
+    v-html would execute markup client-side; assert it uses v-text instead.
+    """
+    login(flask_client)
+
+    r = flask_client.get(url_for("dashboard.index"))
+    assert r.status_code == 200
+    body = r.data.decode()
+
+    assert 'v-text="notification.title"' in body
+    assert 'v-html="notification.title' not in body
+
+
+def test_notification_api_returns_title_as_json_value(flask_client):
+    """
+    The API is the data source for the bell. It must hand back the title as a
+    JSON string value (data), never as pre-rendered HTML.
+    """
+    user, api_key = get_new_user_and_api_key()
+    marker = random_token()
+    payload = f"<img src=x onerror=alert`{marker}`>"
+    Notification.create(user_id=user.id, title=payload, message="body", commit=True)
+    Session.commit()
+
+    r = flask_client.get(
+        url_for("api.get_notifications", page=0),
+        headers={"Authentication": api_key.code},
+    )
+    assert r.status_code == 200
+    # JSON transport carries the title verbatim as a value; the escaping
+    # boundary is the client-side v-text binding, verified above.
+    assert r.json["notifications"][0]["title"] == payload