diff --git a/apps/sponsors/contracts.py b/apps/sponsors/contracts.py index 2b5d586e1..9274bc045 100644 --- a/apps/sponsors/contracts.py +++ b/apps/sponsors/contracts.py @@ -13,6 +13,10 @@ DOCXPAGEBREAK_FILTER = str(_dirname / "pandoc_filters" / "pagebreak.py") REFERENCE_DOCX = str(_dirname / "reference.docx") +# Disable raw TeX and TeX math so user input can't reach the LaTeX engine as commands +# or math. The template's own raw TeX uses raw_attribute (`...`{=latex}), which stays on. +CONTRACT_MARKDOWN_FORMAT = "markdown-raw_tex-tex_math_dollars-tex_math_single_backslash" + def _clean_split(text, separator="\n"): """Split text by newlines and strip dashes and whitespace from each part.""" @@ -58,7 +62,7 @@ def render_contract_to_pdf_file(contract, **context): """Convert the contract markdown to a PDF file and return its bytes.""" with tempfile.NamedTemporaryFile(), tempfile.NamedTemporaryFile(suffix=".pdf") as pdf_file: markdown = render_markdown_from_template(contract, **context) - pypandoc.convert_text(markdown, "pdf", outputfile=pdf_file.name, format="md") + pypandoc.convert_text(markdown, "pdf", outputfile=pdf_file.name, format=CONTRACT_MARKDOWN_FORMAT) return pdf_file.read() @@ -82,7 +86,7 @@ def render_contract_to_docx_file(contract, **context): markdown, "docx", outputfile=docx_file.name, - format="md", + format=CONTRACT_MARKDOWN_FORMAT, filters=[DOCXPAGEBREAK_FILTER], extra_args=["--reference-doc", REFERENCE_DOCX], ) diff --git a/apps/sponsors/models/contract.py b/apps/sponsors/models/contract.py index bc7ccae8f..6eda163f4 100644 --- a/apps/sponsors/models/contract.py +++ b/apps/sponsors/models/contract.py @@ -1,5 +1,7 @@ """Contract generation models for the sponsors app.""" +import posixpath +import secrets import uuid from itertools import chain from pathlib import Path @@ -217,9 +219,13 @@ def set_final_version(self, pdf_file, docx_file=None): sponsor = self.sponsorship.sponsor.name.upper() + # A random token makes the stored path unguessable. Without it the path is + # derived solely from the sponsor name (e.g. "SoW: .pdf"), so anyone + # who knows the sponsor could fetch the contract directly from media storage. + token = secrets.token_hex(16) + # save contract as PDF file - path = f"{self.FINAL_VERSION_PDF_DIR}" - pdf_filename = f"{path}SoW: {sponsor}.pdf" + pdf_filename = posixpath.join(self.FINAL_VERSION_PDF_DIR, f"SoW: {sponsor}-{token}.pdf") file = file_from_storage(pdf_filename, mode="wb") file.write(pdf_file) file.close() @@ -227,8 +233,7 @@ def set_final_version(self, pdf_file, docx_file=None): # save contract as docx file if docx_file: - path = f"{self.FINAL_VERSION_DOCX_DIR}" - docx_filename = f"{path}SoW: {sponsor}.docx" + docx_filename = posixpath.join(self.FINAL_VERSION_DOCX_DIR, f"SoW: {sponsor}-{token}.docx") file = file_from_storage(docx_filename, mode="wb") file.write(docx_file) file.close() diff --git a/apps/sponsors/templates/sponsors/admin/contracts/sponsorship-agreement.md b/apps/sponsors/templates/sponsors/admin/contracts/sponsorship-agreement.md index 21a85b706..01cc84e21 100644 --- a/apps/sponsors/templates/sponsors/admin/contracts/sponsorship-agreement.md +++ b/apps/sponsors/templates/sponsors/admin/contracts/sponsorship-agreement.md @@ -1,4 +1,4 @@ -{% load humanize %} +{% load humanize sponsors %} --- title: SPONSORSHIP AGREEMENT{% if renewal %} RENEWAL{% endif %} geometry: @@ -6,7 +6,7 @@ geometry: font-size: 12pt pagestyle: empty header-includes: -- \pagenumbering{gobble} +- '`\pagenumbering{gobble}`{=latex}' --- **THIS SPONSORSHIP AGREEMENT{% if renewal %} RENEWAL{% endif %}** (the **"Agreement"**) @@ -15,8 +15,8 @@ is entered into and made effective as of the (the **"Effective Date"**), by and between the **Python Software Foundation** (the **"PSF"**), a Delaware nonprofit corporation, -and **{{sponsor.name|upper}}** (**"Sponsor"**), -a {{sponsor.incorporation_location}} corporation. +and **{{sponsor.name|upper|escape_markdown}}** (**"Sponsor"**), +a {{sponsor.incorporation_location|escape_markdown}} corporation. Each of the PSF and Sponsor are hereinafter sometimes individually referred to as a **"Party"** and collectively as the **"Parties"**. @@ -27,7 +27,7 @@ whose mission is to promote, protect, and advance the Python programming languag and to support and facilitate the growth of a diverse and international community of Python programmers (the **"Programs"**); -**WHEREAS**, Sponsor is {{contract.sponsor_info}}; and +**WHEREAS**, Sponsor is {{contract.sponsor_info|escape_markdown}}; and **WHEREAS**, Sponsor {% if renewal %} and the PSF previously entered into a Sponsorship Agreement @@ -94,13 +94,13 @@ wishes to support the Programs by making a contribution to the PSF. If to Sponsor: - >          {{sponsor.primary_contact.name}} - >          {{sponsor.name}} - >          {{sponsor.mailing_address_line_1}}{%if sponsor.mailing_address_line_2%} - >          {{sponsor.mailing_address_line_2 }}{% endif %} - >          {{sponsor.city}}, {{sponsor.state}} {{sponsor.postal_code}} {{sponsor.country}} - >          Facsimile: {{sponsor.primary_contact.phone}} - >          Email: {{sponsor.primary_contact.email}} + >          {{sponsor.primary_contact.name|escape_markdown}} + >          {{sponsor.name|escape_markdown}} + >          {{sponsor.mailing_address_line_1|escape_markdown}}{%if sponsor.mailing_address_line_2%} + >          {{sponsor.mailing_address_line_2|escape_markdown}}{% endif %} + >          {{sponsor.city|escape_markdown}}, {{sponsor.state|escape_markdown}} {{sponsor.postal_code|escape_markdown}} {{sponsor.country|escape_markdown}} + >          Facsimile: {{sponsor.primary_contact.phone|escape_markdown}} + >          Email: {{sponsor.primary_contact.email|escape_markdown}}   @@ -155,7 +155,9 @@ wishes to support the Programs by making a contribution to the PSF. ### \[Signature Page Follows\] ::: {.page-break} +```{=latex} \newpage +``` ::: ## SPONSORSHIP AGREEMENT{% if renewal %} RENEWAL{% endif %} @@ -179,8 +181,8 @@ wishes to support the Programs by making a contribution to the PSF.   > **SPONSOR**: -> **{{sponsor.name|upper}}**, -> a {{sponsor.incorporation_location}} entity +> **{{sponsor.name|upper|escape_markdown}}**, +> a {{sponsor.incorporation_location|escape_markdown}} entity   @@ -189,7 +191,9 @@ wishes to support the Programs by making a contribution to the PSF. > Title:     ________________________________ ::: {.page-break} +```{=latex} \newpage +``` ::: ## SPONSORSHIP AGREEMENT{% if renewal %} RENEWAL{% endif %} diff --git a/apps/sponsors/templatetags/sponsors.py b/apps/sponsors/templatetags/sponsors.py index 4f4289632..57c4aded4 100644 --- a/apps/sponsors/templatetags/sponsors.py +++ b/apps/sponsors/templatetags/sponsors.py @@ -4,6 +4,7 @@ from collections import OrderedDict from django import template +from django.utils.safestring import mark_safe from apps.sponsors.models import Sponsorship, SponsorshipPackage, TieredBenefitConfiguration from apps.sponsors.models.enums import LogoPlacementChoices, PublisherChoices @@ -11,6 +12,19 @@ register = template.Library() +# Backslash-escape every ASCII punctuation char (Pandoc renders each as a literal) so +# user input can't form a Markdown or raw-TeX construct in any context. +MARKDOWN_SPECIAL_CHARS = frozenset("!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") + + +@register.filter +def escape_markdown(value): + """Escape a sponsor-supplied value so Pandoc renders it as literal text.""" + # mark_safe: the output goes to Pandoc (PDF/DOCX), not HTML, so it is not an XSS + # sink; it only stops Django autoescaping from interfering with the escaping here. + return mark_safe("".join("\\" + ch if ch in MARKDOWN_SPECIAL_CHARS else ch for ch in str(value))) # noqa: S308 + + @register.inclusion_tag("sponsors/partials/full_sponsorship.txt") def full_sponsorship(sponsorship, display_fee=False): """Render a full sponsorship detail block with benefits and fee information.""" diff --git a/apps/sponsors/tests/test_contracts.py b/apps/sponsors/tests/test_contracts.py index be2dd58f4..fbf1a32f4 100644 --- a/apps/sponsors/tests/test_contracts.py +++ b/apps/sponsors/tests/test_contracts.py @@ -1,11 +1,18 @@ -from unittest.mock import Mock +from unittest.mock import Mock, patch +import pypandoc from django.http import HttpRequest from django.test import TestCase from django.utils import timezone from model_bakery import baker -from apps.sponsors.contracts import render_contract_to_docx_response +from apps.sponsors.contracts import ( + CONTRACT_MARKDOWN_FORMAT, + render_contract_to_docx_file, + render_contract_to_docx_response, + render_contract_to_pdf_file, + render_markdown_from_template, +) class TestRenderContract(TestCase): @@ -35,3 +42,83 @@ def test_render_renewal_response_with_docx_attachment(self): self.assertEqual( response.get("Content-Type"), "application/vnd.openxmlformats-officedocument.wordprocessingml.document" ) + + +class ContractTemplateEscapingTests(TestCase): + """Regression tests for the unescaped sponsor-input contract injection chain. + + Sponsor fields flow into the Pandoc Markdown template and are escaped so attacker + input can be interpreted as neither a LaTeX command (arbitrary file read) nor a + Markdown image (SSRF). This is defense in depth on top of the hardened reader + format (see ContractPandocHardeningTests). + """ + + def _render_with_sponsor_info(self, sponsor_info): + contract = baker.make_recipe( + "apps.sponsors.tests.empty_contract", sponsorship__start_date=timezone.now().date() + ) + contract.sponsor_info = sponsor_info + return render_markdown_from_template(contract) + + def test_latex_file_read_payload_is_neutralized(self): + rendered = self._render_with_sponsor_info(r"ACME Corp, see \input{/proc/self/environ}") + + # The escaped (backslash-doubled) form is what reaches Pandoc... + self.assertIn(r"\\input", rendered) + # ...and no un-doubled \input survives to be executed as a LaTeX command. + self.assertNotIn(r"\input", rendered.replace(r"\\input", "")) + + def test_markdown_image_ssrf_payload_is_neutralized(self): + rendered = self._render_with_sponsor_info("contact ![](http://169.254.169.254/latest/meta-data/)") + + # Neither the image opener nor a link/target association can form. + self.assertNotIn("![](", rendered) + self.assertNotIn("](http", rendered) + # The `[` was escaped to a literal, and the address survives only as text. + self.assertIn(r"\[", rendered) + self.assertIn(r"169\.254\.169\.254", rendered) + + +class ContractPandocHardeningTests(TestCase): + """The Pandoc reader is configured to disable raw TeX and TeX math. + + This closes the injection sink for every interpolated field at once (defense in + depth alongside escape_pandoc_markdown), while the template's own legitimate raw + TeX is preserved via raw_attribute blocks. + """ + + def setUp(self): + self.contract = baker.make_recipe( + "apps.sponsors.tests.empty_contract", sponsorship__start_date=timezone.now().date() + ) + + def test_pdf_render_uses_hardened_reader_format(self): + with patch("apps.sponsors.contracts.pypandoc.convert_text") as convert: + render_contract_to_pdf_file(self.contract) + self.assertEqual(convert.call_args.kwargs["format"], CONTRACT_MARKDOWN_FORMAT) + + def test_docx_render_uses_hardened_reader_format(self): + with patch("apps.sponsors.contracts.pypandoc.convert_text") as convert: + render_contract_to_docx_file(self.contract) + self.assertEqual(convert.call_args.kwargs["format"], CONTRACT_MARKDOWN_FORMAT) + + def test_hardened_format_parses_tex_and_math_as_literal_text(self): + # A backslash command or $math$ in the input becomes a literal Str, never a + # RawInline/RawBlock (raw TeX) or Math node, so it can't reach the LaTeX engine. + native = pypandoc.convert_text(r"\input{/etc/passwd} and $x$", "native", format=CONTRACT_MARKDOWN_FORMAT) + self.assertNotIn("RawInline", native) + self.assertNotIn("RawBlock", native) + self.assertNotIn("Math", native) + + def test_template_page_breaks_survive_as_raw_latex(self): + # The template's own \newpage / \pagenumbering must still reach LaTeX via + # raw_attribute even though raw_tex is disabled for the reader. + markdown = render_markdown_from_template(self.contract) + latex = pypandoc.convert_text(markdown, "latex", format=CONTRACT_MARKDOWN_FORMAT, extra_args=["--standalone"]) + self.assertIn(r"\newpage", latex) + self.assertIn(r"\pagenumbering{gobble}", latex) + + def test_contract_still_renders_to_pdf(self): + # End-to-end: the raw_attribute rewrites must produce valid LaTeX that compiles. + pdf = render_contract_to_pdf_file(self.contract) + self.assertTrue(pdf.startswith(b"%PDF")) diff --git a/apps/sponsors/tests/test_models.py b/apps/sponsors/tests/test_models.py index aec1f5456..0d5a3618d 100644 --- a/apps/sponsors/tests/test_models.py +++ b/apps/sponsors/tests/test_models.py @@ -619,6 +619,26 @@ def test_set_final_document_version_saves_docx_document_too(self): self.assertTrue(contract.document_docx.name) self.assertEqual(contract.status, Contract.AWAITING_SIGNATURE) + def test_final_document_path_includes_unguessable_token(self): + contract = baker.make_recipe("apps.sponsors.tests.empty_contract", sponsorship__sponsor__name="foo") + + contract.set_final_version(b"pdf binary content", b"docx binary content") + contract.refresh_from_db() + + # A random 32-char hex token is appended before the extension, so the path + # can no longer be guessed from the (uppercased) sponsor name. + self.assertRegex(contract.document.name, r"sponsors/contracts/SoW: FOO-[0-9a-f]{32}\.pdf$") + self.assertRegex(contract.document_docx.name, r"sponsors/contracts/docx/SoW: FOO-[0-9a-f]{32}\.docx$") + + def test_each_send_uses_a_distinct_token(self): + first = baker.make_recipe("apps.sponsors.tests.empty_contract", sponsorship__sponsor__name="foo") + second = baker.make_recipe("apps.sponsors.tests.empty_contract", sponsorship__sponsor__name="foo") + + first.set_final_version(b"pdf binary content") + second.set_final_version(b"pdf binary content") + + self.assertNotEqual(first.document.name, second.document.name) + def test_raise_invalid_status_exception_if_not_draft(self): contract = baker.make_recipe("apps.sponsors.tests.empty_contract", status=Contract.AWAITING_SIGNATURE) diff --git a/apps/sponsors/tests/test_templatetags.py b/apps/sponsors/tests/test_templatetags.py index 37513cfaf..5c13d092c 100644 --- a/apps/sponsors/tests/test_templatetags.py +++ b/apps/sponsors/tests/test_templatetags.py @@ -1,3 +1,4 @@ +import string from unittest.mock import patch from django.test import TestCase @@ -7,6 +8,7 @@ from apps.sponsors.templatetags.sponsors import ( benefit_name_for_display, benefit_quantity_for_package, + escape_markdown, full_sponsorship, list_sponsors, ) @@ -88,3 +90,29 @@ def test_display_name_for_display_from_benefit(self, mocked_name_for_display): self.assertEqual(name, "Modified name") mocked_name_for_display.assert_called_once_with(package=package) + + +class EscapePandocMarkdownTests(TestCase): + """Unit tests for the boundary escaper that neutralizes sponsor input.""" + + def test_backslash_is_doubled(self): + # No LaTeX command (\input, \write, ...) can form once every backslash is + # itself escaped. + self.assertEqual(escape_markdown("\\"), "\\\\") + + def test_every_ascii_punctuation_char_is_escaped(self): + for ch in string.punctuation: + self.assertEqual(escape_markdown(ch), "\\" + ch) + + def test_latex_command_cannot_form(self): + self.assertEqual(escape_markdown(r"\input{x}"), r"\\input\{x\}") + + def test_markdown_image_is_neutralized(self): + # ![](url) is what makes Pandoc fetch a URL server-side (SSRF). + self.assertEqual(escape_markdown("![](x)"), r"\!\[\]\(x\)") + + def test_letters_digits_and_spaces_are_untouched(self): + self.assertEqual(escape_markdown("Acme Corp 123"), "Acme Corp 123") + + def test_non_string_input_is_coerced(self): + self.assertEqual(escape_markdown(42), "42")