From d52dff79d4a292109070a20c9855bcfb5c2b279f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Tue, 1 Sep 2026 12:37:04 +0200 Subject: [PATCH 1/3] fix --- app/email_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/email_utils.py b/app/email_utils.py index 221000d57..6fb1e4c7f 100644 --- a/app/email_utils.py +++ b/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 From da6b57f73d7e96a6173ebfe440aba943a78843ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Tue, 1 Sep 2026 12:48:12 +0200 Subject: [PATCH 2/3] Escape data before injecting it --- app/handler/dmarc.py | 6 ++++-- email_handler.py | 4 ++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/handler/dmarc.py b/app/handler/dmarc.py index 8d139b20d..99c29a13a 100644 --- a/app/handler/dmarc.py +++ b/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( diff --git a/email_handler.py b/email_handler.py index d899c6f13..b29a0c4e3 100644 --- a/email_handler.py +++ b/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""", From d2c52681ac5f0758cb41df804198749276eaf8bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Tue, 1 Sep 2026 15:38:05 +0200 Subject: [PATCH 3/3] Escape message emails in js context --- static/js/index.js | 18 ++++-- templates/base.html | 7 ++- templates/dashboard/index.html | 9 ++- templates/dashboard/notifications.html | 2 +- templates/header.html | 11 +++- templates/partials/toggle_contact.html | 2 +- tests/dashboard/test_index.py | 36 ++++++++++- tests/dashboard/test_notification.py | 84 ++++++++++++++++++++++++++ 8 files changed, 154 insertions(+), 15 deletions(-) create mode 100644 tests/dashboard/test_notification.py diff --git a/static/js/index.js b/static/js/index.js index e434e0669..2ec545264 100644 --- a/static/js/index.js +++ b/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}`, { diff --git a/templates/base.html b/templates/base.html index fcc715a5d..a33f19a67 100644 --- a/templates/base.html +++ b/templates/base.html @@ -80,7 +80,10 @@ type="text/css" href="/static/style.css?v={{ VERSION }}" /> - + {% block head %}{% endblock %} @@ -99,7 +102,7 @@ {% if messages %} - {% for category, message in messages %}{% endfor %} + {% for category, message in messages %}{% endfor %} {% endif %} {% endwith %} diff --git a/templates/dashboard/index.html b/templates/dashboard/index.html index b5232e89a..2c76dbe08 100644 --- a/templates/dashboard/index.html +++ b/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 "" }} @@ -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 %} @@ -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 }})"> diff --git a/templates/dashboard/notifications.html b/templates/dashboard/notifications.html index 66b19c2f2..e3a921888 100644 --- a/templates/dashboard/notifications.html +++ b/templates/dashboard/notifications.html @@ -11,7 +11,7 @@

Notifications

-
{{ notification.title | safe or "" }}
+
{{ notification.title or "" }}
See all notifications ➡
-
+
+
{% endif %} +{% if toast_msg %}{% endif %} diff --git a/tests/dashboard/test_index.py b/tests/dashboard/test_index.py index 770e13b3f..3d969b955 100644 --- a/tests/dashboard/test_index.py +++ b/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) diff --git a/tests/dashboard/test_notification.py b/tests/dashboard/test_notification.py new file mode 100644 index 000000000..6a2c0476a --- /dev/null +++ b/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"" + 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 "<img" in body + + +def test_single_notification_page_escapes_title(flask_client): + user = login(flask_client) + + marker = random_token() + payload = f"" + 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 "<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"" + 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