mirror of
https://github.com/pretix/pretix.git
synced 2026-05-17 17:14:04 +00:00
SECURITY: Prevent placeholder injcetion in plaintext emails
This commit is contained in:
committed by
Raphael Michel
parent
5d87f9a26f
commit
ff351f2856
@@ -222,8 +222,8 @@ def mail(email: Union[str, Sequence[str]], subject: str, template: Union[str, La
|
|||||||
'invoice_company': ''
|
'invoice_company': ''
|
||||||
})
|
})
|
||||||
renderer = ClassicMailRenderer(None, organizer)
|
renderer = ClassicMailRenderer(None, organizer)
|
||||||
body_plain = render_mail(template, context, placeholder_mode=SafeFormatter.MODE_RICH_TO_PLAIN)
|
content_plain = render_mail(template, context, placeholder_mode=None)
|
||||||
subject = str(subject).format_map(TolerantDict(context))
|
subject = format_map(subject, context)
|
||||||
sender = (
|
sender = (
|
||||||
sender or
|
sender or
|
||||||
(event.settings.get('mail_from') if event else None) or
|
(event.settings.get('mail_from') if event else None) or
|
||||||
@@ -255,6 +255,7 @@ def mail(email: Union[str, Sequence[str]], subject: str, template: Union[str, La
|
|||||||
else:
|
else:
|
||||||
timezone = ZoneInfo(settings.TIME_ZONE)
|
timezone = ZoneInfo(settings.TIME_ZONE)
|
||||||
|
|
||||||
|
body_plain = render_mail(template, context, placeholder_mode=SafeFormatter.MODE_RICH_TO_PLAIN)
|
||||||
if settings_holder:
|
if settings_holder:
|
||||||
if settings_holder.settings.mail_bcc:
|
if settings_holder.settings.mail_bcc:
|
||||||
for bcc_mail in settings_holder.settings.mail_bcc.split(','):
|
for bcc_mail in settings_holder.settings.mail_bcc.split(','):
|
||||||
@@ -270,7 +271,7 @@ def mail(email: Union[str, Sequence[str]], subject: str, template: Union[str, La
|
|||||||
|
|
||||||
signature = str(settings_holder.settings.get('mail_text_signature'))
|
signature = str(settings_holder.settings.get('mail_text_signature'))
|
||||||
if signature:
|
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 += signature
|
||||||
body_plain += "\r\n\r\n-- \r\n"
|
body_plain += "\r\n\r\n-- \r\n"
|
||||||
|
|
||||||
@@ -288,7 +289,7 @@ def mail(email: Union[str, Sequence[str]], subject: str, template: Union[str, La
|
|||||||
body_plain += _(
|
body_plain += _(
|
||||||
"You can view your order details at the following URL:\n{orderurl}."
|
"You can view your order details at the following URL:\n{orderurl}."
|
||||||
).replace("\n", "\r\n").format(
|
).replace("\n", "\r\n").format(
|
||||||
event=event.name, orderurl=build_absolute_uri(
|
orderurl=build_absolute_uri(
|
||||||
order.event, 'presale:event.order.position', kwargs={
|
order.event, 'presale:event.order.position', kwargs={
|
||||||
'order': order.code,
|
'order': order.code,
|
||||||
'secret': position.web_secret,
|
'secret': position.web_secret,
|
||||||
@@ -304,7 +305,7 @@ def mail(email: Union[str, Sequence[str]], subject: str, template: Union[str, La
|
|||||||
body_plain += _(
|
body_plain += _(
|
||||||
"You can view your order details at the following URL:\n{orderurl}."
|
"You can view your order details at the following URL:\n{orderurl}."
|
||||||
).replace("\n", "\r\n").format(
|
).replace("\n", "\r\n").format(
|
||||||
event=event.name, orderurl=build_absolute_uri(
|
orderurl=build_absolute_uri(
|
||||||
order.event, 'presale:event.order.open', kwargs={
|
order.event, 'presale:event.order.open', kwargs={
|
||||||
'order': order.code,
|
'order': order.code,
|
||||||
'secret': order.secret,
|
'secret': order.secret,
|
||||||
@@ -316,7 +317,6 @@ def mail(email: Union[str, Sequence[str]], subject: str, template: Union[str, La
|
|||||||
|
|
||||||
with override(timezone):
|
with override(timezone):
|
||||||
try:
|
try:
|
||||||
content_plain = render_mail(template, context, placeholder_mode=None)
|
|
||||||
if plain_text_only:
|
if plain_text_only:
|
||||||
body_html = None
|
body_html = None
|
||||||
elif 'context' in inspect.signature(renderer.render).parameters:
|
elif 'context' in inspect.signature(renderer.render).parameters:
|
||||||
@@ -337,8 +337,6 @@ def mail(email: Union[str, Sequence[str]], subject: str, template: Union[str, La
|
|||||||
logger.exception('Could not render HTML body')
|
logger.exception('Could not render HTML body')
|
||||||
body_html = None
|
body_html = None
|
||||||
|
|
||||||
body_plain = format_map(body_plain, context, mode=SafeFormatter.MODE_RICH_TO_PLAIN)
|
|
||||||
|
|
||||||
send_task = mail_send_task.si(
|
send_task = mail_send_task.si(
|
||||||
to=[email] if isinstance(email, str) else list(email),
|
to=[email] if isinstance(email, str) else list(email),
|
||||||
cc=cc,
|
cc=cc,
|
||||||
|
|||||||
@@ -32,8 +32,10 @@
|
|||||||
# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
# 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.
|
# License for the specific language governing permissions and limitations under the License.
|
||||||
|
|
||||||
|
import datetime
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
|
from decimal import Decimal
|
||||||
from email.mime.text import MIMEText
|
from email.mime.text import MIMEText
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
@@ -41,11 +43,11 @@ from django.conf import settings
|
|||||||
from django.core import mail as djmail
|
from django.core import mail as djmail
|
||||||
from django.utils.timezone import now
|
from django.utils.timezone import now
|
||||||
from django.utils.translation import gettext_lazy as _
|
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 i18nfield.strings import LazyI18nString
|
||||||
|
|
||||||
from pretix.base.email import get_email_context
|
from pretix.base.email import get_email_context
|
||||||
from pretix.base.models import Event, Organizer, User
|
from pretix.base.models import Event, InvoiceAddress, Order, Organizer, User
|
||||||
from pretix.base.services.mail import mail
|
from pretix.base.services.mail import mail
|
||||||
|
|
||||||
|
|
||||||
@@ -67,6 +69,45 @@ def env():
|
|||||||
yield event, user, o
|
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
|
@pytest.mark.django_db
|
||||||
def test_send_mail_with_prefix(env):
|
def test_send_mail_with_prefix(env):
|
||||||
djmail.outbox = []
|
djmail.outbox = []
|
||||||
@@ -272,3 +313,141 @@ def test_placeholder_html_rendering_from_string(env):
|
|||||||
r'URL with text: <a href="https://google.com" rel="noopener" style="[^"]+" target="_blank">Test</a>',
|
r'URL with text: <a href="https://google.com" rel="noopener" style="[^"]+" target="_blank">Test</a>',
|
||||||
html
|
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 & {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
|
||||||
|
|||||||
Reference in New Issue
Block a user