Merge branch 'pajowu/security-plaintext-placeholder' into 'master'

SECURITY: Prevent placeholder injection in plaintext emails

See merge request pretix/pretix!21
This commit is contained in:
Raphael Michel
2026-02-16 10:59:44 +01:00
7 changed files with 294 additions and 67 deletions

View File

@@ -39,7 +39,7 @@ from pretix.base.templatetags.rich_text import (
DEFAULT_CALLBACKS, EMAIL_RE, URL_RE, abslink_callback,
markdown_compile_email, truelink_callback,
)
from pretix.helpers.format import SafeFormatter, format_map
from pretix.helpers.format import FormattedString, SafeFormatter, format_map
from pretix.base.services.placeholders import ( # noqa
get_available_placeholders, PlaceholderContext
@@ -141,6 +141,7 @@ class TemplateBasedMailRenderer(BaseHTMLMailRenderer):
return markdown_compile_email(plaintext, context=context)
def render(self, plain_body: str, plain_signature: str, subject: str, order, position, context) -> str:
apply_format_map = not isinstance(plain_body, FormattedString)
body_md = self.compile_markdown(plain_body, context)
if context:
linker = bleach.Linker(
@@ -149,12 +150,13 @@ class TemplateBasedMailRenderer(BaseHTMLMailRenderer):
callbacks=DEFAULT_CALLBACKS + [truelink_callback, abslink_callback],
parse_email=True
)
body_md = format_map(
body_md,
context=context,
mode=SafeFormatter.MODE_RICH_TO_HTML,
linkifier=linker
)
if apply_format_map:
body_md = format_map(
body_md,
context=context,
mode=SafeFormatter.MODE_RICH_TO_HTML,
linkifier=linker
)
htmlctx = {
'site': settings.PRETIX_INSTANCE_NAME,
'site_url': settings.SITE_URL,

View File

@@ -87,7 +87,7 @@ from pretix.base.timemachine import time_machine_now
from ...helpers import OF_SELF
from ...helpers.countries import CachedCountries, FastCountryField
from ...helpers.format import format_map
from ...helpers.format import FormattedString, format_map
from ...helpers.names import build_name
from ...testutils.middleware import debugflags_var
from ._transactions import (
@@ -1178,7 +1178,8 @@ class Order(LockModel, LoggedModel):
recipient = position.attendee_email
email_content = render_mail(template, context)
subject = format_map(subject, context)
if not isinstance(subject, FormattedString):
subject = format_map(subject, context)
mail(
recipient, subject, template, context,
self.event, self.locale, self, headers=headers, sender=sender,
@@ -2907,7 +2908,8 @@ class OrderPosition(AbstractPosition):
with language(self.order.locale, self.order.event.settings.region):
recipient = self.attendee_email
email_content = render_mail(template, context)
subject = format_map(subject, context)
if not isinstance(subject, FormattedString):
subject = format_map(subject, context)
mail(
recipient, subject, template, context,
self.event, self.order.locale, order=self.order, headers=headers, sender=sender,

View File

@@ -81,7 +81,9 @@ from pretix.base.signals import (
)
from pretix.celery_app import app
from pretix.helpers import OF_SELF
from pretix.helpers.format import SafeFormatter, format_map
from pretix.helpers.format import (
FormattedString, PlainHtmlAlternativeString, SafeFormatter, format_map,
)
from pretix.helpers.hierarkey import clean_filename
from pretix.multidomain.urlreverse import build_absolute_uri
from pretix.presale.ical import get_private_icals
@@ -218,6 +220,9 @@ def mail(email: Union[str, Sequence[str]], subject: str, template: Union[str, La
if email == INVALID_ADDRESS:
return
if isinstance(template, FormattedString):
raise TypeError("Cannot pass an already formatted body template")
if no_order_links and not plain_text_only:
raise ValueError('If you set no_order_links, you also need to set plain_text_only.')
@@ -251,23 +256,28 @@ def mail(email: Union[str, Sequence[str]], subject: str, template: Union[str, La
if event and attach_tickets and not event.settings.mail_attach_tickets:
attach_tickets = False
with language(locale):
with language(locale), override(timezone):
if isinstance(context, dict) and order:
_autoextend_context(context, order)
# Build raw content
body_plain = render_mail(template, context, placeholder_mode=SafeFormatter.MODE_RICH_TO_PLAIN)
content_plain = render_mail(template, context, placeholder_mode=None)
if settings_holder:
signature = str(settings_holder.settings.get('mail_text_signature'))
else:
signature = ""
# Build full plain-text body
if not isinstance(content_plain, FormattedString):
body_plain = format_map(content_plain, context, mode=SafeFormatter.MODE_RICH_TO_PLAIN)
else:
body_plain = content_plain
body_plain = _wrap_plain_body(body_plain, signature, event, order, position, no_order_links)
body_plain = format_map(body_plain, context, mode=SafeFormatter.MODE_RICH_TO_PLAIN)
# Build subject
subject = str(subject).format_map(TolerantDict(context))
if not isinstance(subject, FormattedString):
subject = format_map(subject, context)
subject = raw_subject = subject.replace('\n', ' ').replace('\r', '')[:900]
if settings_holder:
subject = prefix_subject(settings_holder, subject)
@@ -286,26 +296,24 @@ def mail(email: Union[str, Sequence[str]], subject: str, template: Union[str, La
else:
renderer = ClassicMailRenderer(None, organizer)
with override(timezone):
content_plain = render_mail(template, context, placeholder_mode=None)
try:
if 'context' in inspect.signature(renderer.render).parameters:
body_html = renderer.render(content_plain, signature, raw_subject, order, position, context)
elif 'position' in inspect.signature(renderer.render).parameters:
# Backwards compatibility
warnings.warn('Email renderer called without context argument because context argument is not '
'supported.',
DeprecationWarning)
body_html = renderer.render(content_plain, signature, raw_subject, order, position)
else:
# Backwards compatibility
warnings.warn('Email renderer called without position argument because position argument is not '
'supported.',
DeprecationWarning)
body_html = renderer.render(content_plain, signature, raw_subject, order)
except:
logger.exception('Could not render HTML body')
body_html = None
try:
if 'context' in inspect.signature(renderer.render).parameters:
body_html = renderer.render(content_plain, signature, raw_subject, order, position, context)
elif 'position' in inspect.signature(renderer.render).parameters:
# Backwards compatibility
warnings.warn('Email renderer called without context argument because context argument is not '
'supported.',
DeprecationWarning)
body_html = renderer.render(content_plain, signature, raw_subject, order, position)
else:
# Backwards compatibility
warnings.warn('Email renderer called without position argument because position argument is not '
'supported.',
DeprecationWarning)
body_html = renderer.render(content_plain, signature, raw_subject, order)
except:
logger.exception('Could not render HTML body')
body_html = None
m = OutgoingMail.objects.create(
organizer=organizer,
@@ -790,7 +798,12 @@ def render_mail(template, context, placeholder_mode: Optional[int]=SafeFormatter
body = format_map(body, context, mode=placeholder_mode)
else:
tpl = get_template(template)
body = tpl.render(context)
context = {
# Known bug, should behave differently for plain and HTML but we'll fix after security release
k: v.html if isinstance(v, PlainHtmlAlternativeString) else v
for k, v in context.items()
}
body = FormattedString(tpl.render(context))
return body
@@ -936,7 +949,7 @@ def _wrap_plain_body(content_plain, signature, event, order, position, no_order_
body_plain += "\r\n\r\n-- \r\n"
if signature:
signature = signature.format(event=event.name if event else '')
signature = format_map(signature, {"event": event.name if event else ''})
body_plain += signature
body_plain += "\r\n\r\n-- \r\n"
@@ -948,7 +961,7 @@ def _wrap_plain_body(content_plain, signature, event, order, position, no_order_
body_plain += _(
"You can view your order details at the following URL:\n{orderurl}."
).replace("\n", "\r\n").format(
event=event.name, orderurl=build_absolute_uri(
orderurl=build_absolute_uri(
order.event, 'presale:event.order.position', kwargs={
'order': order.code,
'secret': position.web_secret,

View File

@@ -22,6 +22,7 @@
import logging
from string import Formatter
from django.core.exceptions import SuspiciousOperation
from django.utils.html import conditional_escape
logger = logging.getLogger(__name__)
@@ -37,6 +38,17 @@ class PlainHtmlAlternativeString:
return f"PlainHtmlAlternativeString('{self.plain}', '{self.html}')"
class FormattedString(str):
"""
A str subclass that has been specifically marked as "already formatted" for email rendering
purposes to avoid duplicate formatting.
"""
__slots__ = ()
def __str__(self):
return self
class SafeFormatter(Formatter):
"""
Customized version of ``str.format`` that (a) behaves just like ``str.format_map`` and
@@ -77,8 +89,19 @@ class SafeFormatter(Formatter):
# Ignore format_spec
return super().format_field(self._prepare_value(value), '')
def convert_field(self, value, conversion):
# Ignore any conversions
if conversion is None:
return value
else:
return str(value)
def format_map(template, context, raise_on_missing=False, mode=SafeFormatter.MODE_RICH_TO_PLAIN, linkifier=None):
def format_map(template, context, raise_on_missing=False, mode=SafeFormatter.MODE_RICH_TO_PLAIN, linkifier=None) -> FormattedString:
if isinstance(template, FormattedString):
raise SuspiciousOperation("Calling format_map() on an already formatted string is likely unsafe.")
if not isinstance(template, str):
template = str(template)
return SafeFormatter(context, raise_on_missing, mode=mode, linkifier=linkifier).format(template)
return FormattedString(
SafeFormatter(context, raise_on_missing, mode=mode, linkifier=linkifier).format(template)
)

View File

@@ -32,21 +32,26 @@
# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations under the License.
import datetime
import os
import re
from decimal import Decimal
from email.mime.text import MIMEText
import pytest
from django.conf import settings
from django.core import mail as djmail
from django.test import override_settings
from django.utils.html import escape
from django.utils.timezone import now
from django.utils.translation import gettext_lazy as _
from django_scopes import scope
from django_scopes import scope, scopes_disabled
from i18nfield.strings import LazyI18nString
from pretix.base.email import get_email_context
from pretix.base.models import Event, Organizer, OutgoingMail, User
from pretix.base.models import (
Event, InvoiceAddress, Order, Organizer, OutgoingMail, User,
)
from pretix.base.services.mail import mail, mail_send_task
@@ -68,6 +73,45 @@ def env():
yield event, user, o
@pytest.fixture
@scopes_disabled()
def item(env):
return env[0].items.create(name="Budget Ticket", default_price=23)
@pytest.fixture
@scopes_disabled()
def order(env, item):
event, _, _ = env
o = Order.objects.create(
code="FOO",
event=event,
email="dummy@dummy.test",
status=Order.STATUS_PENDING,
secret="k24fiuwvu8kxz3y1",
sales_channel=event.organizer.sales_channels.get(identifier="web"),
datetime=datetime.datetime(2017, 12, 1, 10, 0, 0, tzinfo=datetime.UTC),
expires=datetime.datetime(2017, 12, 10, 10, 0, 0, tzinfo=datetime.UTC),
total=23,
locale="en",
)
o.positions.create(
order=o,
item=item,
variation=None,
price=Decimal("23"),
attendee_email="peter@example.org",
attendee_name_parts={"given_name": "Peter", "family_name": "Miller"},
secret="z3fsn8jyufm5kpk768q69gkbyr5f4h6w",
pseudonymization_id="ABCDEFGHKL",
)
InvoiceAddress.objects.create(
order=o,
name_parts={"given_name": "Peter", "family_name": "Miller"},
)
return o
@pytest.mark.django_db
def test_send_mail_with_prefix(env):
djmail.outbox = []
@@ -279,7 +323,7 @@ def _extract_html(mail):
def test_placeholder_html_rendering_from_template(env):
djmail.outbox = []
event, user, organizer = env
event.name = "<strong>event & co. kg</strong>"
event.name = "<strong>event & co. kg</strong> {currency}"
event.save()
mail('dummy@dummy.dummy', '{event} Test subject', 'mailtest.txt', get_email_context(
event=event,
@@ -288,25 +332,26 @@ def test_placeholder_html_rendering_from_template(env):
assert len(djmail.outbox) == 1
assert djmail.outbox[0].to == [user.email]
assert 'Event name: <strong>event & co. kg</strong>' in djmail.outbox[0].body
assert '**IBAN**: 123 \n**BIC**: 456' in djmail.outbox[0].body
assert '**Meta**: *Beep*' in djmail.outbox[0].body
assert 'Event website: [<strong>event & co. kg</strong>](https://example.org/dummy)' in djmail.outbox[0].body
assert 'Other website: [<strong>event & co. kg</strong>](https://example.com)' in djmail.outbox[0].body
assert '&lt;' not in djmail.outbox[0].body
assert '&amp;' not in djmail.outbox[0].body
# Known bug for now: These should not have HTML for the plain body, but we'll fix this safter the security release
assert escape('Event name: <strong>event & co. kg</strong> {currency}') in djmail.outbox[0].body
assert '<strong>IBAN</strong>: 123<br>\n<strong>BIC</strong>: 456' in djmail.outbox[0].body
assert '**Meta**: <em>Beep</em>' in djmail.outbox[0].body
assert escape('Event website: [<strong>event & co. kg</strong> {currency}](https://example.org/dummy)') in djmail.outbox[0].body
# todo: assert '&lt;' not in djmail.outbox[0].body
# todo: assert '&amp;' not in djmail.outbox[0].body
assert 'Unevaluated placeholder: {currency}' in djmail.outbox[0].body
assert 'EUR' not in djmail.outbox[0].body
html = _extract_html(djmail.outbox[0])
assert '<strong>event' not in html
assert 'Event name: &lt;strong&gt;event &amp; co. kg&lt;/strong&gt;' in html
assert 'Event name: &lt;strong&gt;event &amp; co. kg&lt;/strong&gt; {currency}' in html
assert '<strong>IBAN</strong>: 123<br/>\n<strong>BIC</strong>: 456' in html
assert '<strong>Meta</strong>: <em>Beep</em>' in html
assert 'Unevaluated placeholder: {currency}' in html
assert 'EUR' not in html
assert re.search(
r'Event website: <a href="https://example.org/dummy" rel="noopener" style="[^"]+" target="_blank">&lt;strong&gt;event &amp; co. kg&lt;/strong&gt;</a>',
html
)
assert re.search(
r'Other website: <a href="https://example.com" rel="noopener" style="[^"]+" target="_blank">&lt;strong&gt;event &amp; co. kg&lt;/strong&gt;</a>',
r'Event website: <a href="https://example.org/dummy" rel="noopener" style="[^"]+" target="_blank">'
r'&lt;strong&gt;event &amp; co. kg&lt;/strong&gt; {currency}</a>',
html
)
@@ -324,7 +369,7 @@ def test_placeholder_html_rendering_from_string(env):
})
djmail.outbox = []
event, user, organizer = env
event.name = "<strong>event & co. kg</strong>"
event.name = "<strong>event & co. kg</strong> {currency}"
event.save()
ctx = get_email_context(
event=event,
@@ -335,9 +380,9 @@ def test_placeholder_html_rendering_from_string(env):
assert len(djmail.outbox) == 1
assert djmail.outbox[0].to == [user.email]
assert 'Event name: <strong>event & co. kg</strong>' in djmail.outbox[0].body
assert 'Event website: [<strong>event & co. kg</strong>](https://example.org/dummy)' in djmail.outbox[0].body
assert 'Other website: [<strong>event & co. kg</strong>](https://example.com)' in djmail.outbox[0].body
assert 'Event name: <strong>event & co. kg</strong> {currency}' in djmail.outbox[0].body
assert 'Event website: [<strong>event & co. kg</strong> {currency}](https://example.org/dummy)' in djmail.outbox[0].body
assert 'Other website: [<strong>event & co. kg</strong> {currency}](https://example.com)' in djmail.outbox[0].body
assert '**IBAN**: 123 \n**BIC**: 456' in djmail.outbox[0].body
assert '**Meta**: *Beep*' in djmail.outbox[0].body
assert 'URL: https://google.com' in djmail.outbox[0].body
@@ -352,11 +397,13 @@ def test_placeholder_html_rendering_from_string(env):
assert '<strong>IBAN</strong>: 123<br/>\n<strong>BIC</strong>: 456' in html
assert '<strong>Meta</strong>: <em>Beep</em>' in html
assert re.search(
r'Event website: <a href="https://example.org/dummy" rel="noopener" style="[^"]+" target="_blank">&lt;strong&gt;event &amp; co. kg&lt;/strong&gt;</a>',
r'Event website: <a href="https://example.org/dummy" rel="noopener" style="[^"]+" target="_blank">'
r'&lt;strong&gt;event &amp; co. kg&lt;/strong&gt; {currency}</a>',
html
)
assert re.search(
r'Other website: <a href="https://example.com" rel="noopener" style="[^"]+" target="_blank">&lt;strong&gt;event &amp; co. kg&lt;/strong&gt;</a>',
r'Other website: <a href="https://example.com" rel="noopener" style="[^"]+" target="_blank">'
r'&lt;strong&gt;event &amp; co. kg&lt;/strong&gt; {currency}</a>',
html
)
assert re.search(
@@ -377,3 +424,141 @@ def test_placeholder_html_rendering_from_string(env):
r'style="[^"]+" target="_blank">Link &amp; Text</a>',
html
)
@pytest.mark.django_db
def test_nested_placeholder_inclusion_full_process(env, order):
# Test that it is not possible to sneak in a placeholder like {url_cancel} inside a user-controlled
# placeholder value like {invoice_company}
event, user, organizer = env
position = order.positions.get()
order.invoice_address.company = "{url_cancel} Corp"
order.invoice_address.save()
event.settings.mail_text_resend_link = LazyI18nString({"en": "Ticket for {invoice_company}"})
event.settings.mail_subject_resend_link_attendee = LazyI18nString({"en": "Ticket for {invoice_company}"})
djmail.outbox = []
position.resend_link()
assert len(djmail.outbox) == 1
assert djmail.outbox[0].to == [position.attendee_email]
assert "Ticket for {url_cancel} Corp" == djmail.outbox[0].subject
assert "/cancel" not in djmail.outbox[0].body
assert "/order" not in djmail.outbox[0].body
html, plain = _extract_html(djmail.outbox[0]), djmail.outbox[0].body
for part in (html, plain):
assert "Ticket for {url_cancel} Corp" in part
assert "/order/" not in part
assert "/cancel" not in part
@pytest.mark.django_db
def test_nested_placeholder_inclusion_mail_service(env):
# test that it is not possible to have placeholders within the values of placeholders when
# the mail() function is called directly
template = LazyI18nString("Event name: {event}")
djmail.outbox = []
event, user, organizer = env
event.name = "event & {currency} co. kg"
event.slug = "event-co-ag-slug"
event.save()
mail(
"dummy@dummy.dummy",
"{event} Test subject",
template,
get_email_context(
event=event,
payment_info="**IBAN**: 123 \n**BIC**: 456 {event}",
),
event,
)
assert len(djmail.outbox) == 1
assert djmail.outbox[0].to == [user.email]
html, plain = _extract_html(djmail.outbox[0]), djmail.outbox[0].body
for part in (html, plain, djmail.outbox[0].subject):
assert "event & {currency} co. kg" in part or "event &amp; {currency} co. kg" in part
assert "EUR" not in part
@pytest.mark.django_db
@pytest.mark.parametrize("tpl", [
"Event: {event.__class__}",
"Event: {{event.__class__}}",
"Event: {{{event.__class__}}}",
])
def test_variable_inclusion_from_string_full_process(env, tpl, order):
# Test that it is not possible to use placeholders that leak system information in templates
# when run through system processes
event, user, organizer = env
event.name = "event & co. kg"
event.save()
position = order.positions.get()
event.settings.mail_text_resend_link = LazyI18nString({"en": tpl})
event.settings.mail_subject_resend_link_attendee = LazyI18nString({"en": tpl})
position.resend_link()
assert len(djmail.outbox) == 1
html, plain = _extract_html(djmail.outbox[0]), djmail.outbox[0].body
for part in (html, plain, djmail.outbox[0].subject):
assert "{event.__class__}" in part
assert "LazyI18nString" not in part
@pytest.mark.django_db
@pytest.mark.parametrize("tpl", [
"Event: {event.__class__}",
"Event: {{event.__class__}}",
"Event: {{{event.__class__}}}",
])
def test_variable_inclusion_from_string_mail_service(env, tpl):
# Test that it is not possible to use placeholders that leak system information in templates
# when run through mail() directly
event, user, organizer = env
event.name = "event & co. kg"
event.save()
djmail.outbox = []
mail(
"dummy@dummy.dummy",
tpl,
LazyI18nString(tpl),
get_email_context(
event=event,
payment_info="**IBAN**: 123 \n**BIC**: 456\n" + tpl,
),
event,
)
assert len(djmail.outbox) == 1
html, plain = _extract_html(djmail.outbox[0]), djmail.outbox[0].body
for part in (html, plain, djmail.outbox[0].subject):
assert "{event.__class__}" in part
assert "LazyI18nString" not in part
@pytest.mark.django_db
def test_escaped_braces_mail_services(env):
# Test that braces can be escaped by doubling
template = LazyI18nString("Event name: -{{currency}}-")
djmail.outbox = []
event, user, organizer = env
event.name = "event & co. kg"
event.save()
mail(
"dummy@dummy.dummy",
"-{{currency}}- Test subject",
template,
get_email_context(
event=event,
payment_info="**IBAN**: 123 \n**BIC**: 456 {event}",
),
event,
)
assert len(djmail.outbox) == 1
assert djmail.outbox[0].to == [user.email]
html, plain = _extract_html(djmail.outbox[0]), djmail.outbox[0].body
for part in (html, plain, djmail.outbox[0].subject):
assert "EUR" not in part
assert "-{currency}-" in part

View File

@@ -29,6 +29,8 @@ def test_format_map():
assert format_map("Foo {baz}", {"bar": 3}) == "Foo {baz}"
assert format_map("Foo {bar.__module__}", {"bar": 3}) == "Foo {bar.__module__}"
assert format_map("Foo {bar!s}", {"bar": 3}) == "Foo 3"
assert format_map("Foo {bar!r}", {"bar": '3'}) == "Foo 3"
assert format_map("Foo {bar!a}", {"bar": '3'}) == "Foo 3"
assert format_map("Foo {bar:<20}", {"bar": 3}) == "Foo 3"

View File

@@ -1,13 +1,13 @@
{% load i18n %}
This is a test file for sending mails.
Event name: {event}
Event name: {{ event }}
Unevaluated placeholder: {currency}
{% get_current_language as LANGUAGE_CODE %}
The language code used for rendering this email is {{ LANGUAGE_CODE }}.
Payment info:
{payment_info}
{{ payment_info }}
**Meta**: {meta_Test}
**Meta**: {{ meta_Test }}
Event website: [{event}](https://example.org/{event_slug})
Other website: [{event}]({meta_Website})
Event website: [{{event}}](https://example.org/{{event_slug}})