From 673fa14525c32ce577cdbdf3d55d0de8eaf4751c Mon Sep 17 00:00:00 2001 From: David Brownman Date: Fri, 28 Aug 2026 18:07:52 -0700 Subject: [PATCH 1/3] swap to a secure multipart boundary --- stripe/_multipart_data_generator.py | 30 +++++++++++---- tests/api_resources/test_file.py | 4 +- tests/api_resources/test_file_upload.py | 4 +- tests/services/test_file_upload.py | 4 +- tests/test_multipart_data_generator.py | 49 +++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 13 deletions(-) diff --git a/stripe/_multipart_data_generator.py b/stripe/_multipart_data_generator.py index 3151df83e..ec91d22b4 100644 --- a/stripe/_multipart_data_generator.py +++ b/stripe/_multipart_data_generator.py @@ -1,13 +1,29 @@ -import random import io +import secrets from stripe._encode import _api_encode +# Number of random bytes used to build the boundary, matching stripe-ruby's +# `SecureRandom.hex(30)` and Go's mime/multipart. This must come from a CSPRNG: +# multipart/form-data is only safe if the delimiter cannot be guessed by anyone +# able to influence the content, since a value containing the delimiter gets +# parsed as additional parts. +BOUNDARY_BYTES = 30 + + +def _escape_header_value(value: str) -> str: + """Make a value safe to interpolate into a part header. + + An unescaped quote would end the quoted-string early, and CR/LF would + introduce additional header lines or parts. + """ + return value.replace('"', "%22").replace("\r", " ").replace("\n", " ") + class MultipartDataGenerator(object): data: io.BytesIO line_break: str - boundary: int + boundary: str chunk_size: int def __init__(self, chunk_size: int = 1028): @@ -36,9 +52,9 @@ def add_params(self, params): filename = str(value.name) self._write('Content-Disposition: form-data; name="') - self._write(key) + self._write(_escape_header_value(key)) self._write('"; filename="') - self._write(filename) + self._write(_escape_header_value(filename)) self._write('"') self._write(self.line_break) self._write("Content-Type: application/octet-stream") @@ -48,7 +64,7 @@ def add_params(self, params): self._write_file(value) else: self._write('Content-Disposition: form-data; name="') - self._write(key) + self._write(_escape_header_value(key)) self._write('"') self._write(self.line_break) self._write(self.line_break) @@ -83,5 +99,5 @@ def _write_file(self, f): break self._write(file_contents) - def _initialize_boundary(self): - return random.randint(0, 2**63) + def _initialize_boundary(self) -> str: + return secrets.token_hex(BOUNDARY_BYTES) diff --git a/tests/api_resources/test_file.py b/tests/api_resources/test_file.py index 4494f543e..8ce21c45d 100644 --- a/tests/api_resources/test_file.py +++ b/tests/api_resources/test_file.py @@ -33,7 +33,7 @@ def test_is_retrievable(self, http_client_mock): assert isinstance(resource, stripe.File) def test_is_creatable(self, setup_upload_api_base, http_client_mock): - MultipartDataGenerator._initialize_boundary = lambda self: 1234567890 + MultipartDataGenerator._initialize_boundary = lambda self: "abc123" test_file = tempfile.TemporaryFile() resource = stripe.File.create( purpose="dispute_evidence", @@ -44,7 +44,7 @@ def test_is_creatable(self, setup_upload_api_base, http_client_mock): "post", path="/v1/files", api_base=stripe.upload_api_base, - content_type="multipart/form-data; boundary=1234567890", + content_type="multipart/form-data; boundary=abc123", ) assert isinstance(resource, stripe.File) diff --git a/tests/api_resources/test_file_upload.py b/tests/api_resources/test_file_upload.py index 8e52cf103..f6f63d7bf 100644 --- a/tests/api_resources/test_file_upload.py +++ b/tests/api_resources/test_file_upload.py @@ -34,7 +34,7 @@ def test_is_retrievable(self, http_client_mock): assert isinstance(resource, File) def test_is_creatable(self, setup_upload_api_base, http_client_mock): - MultipartDataGenerator._initialize_boundary = lambda self: 1234567890 + MultipartDataGenerator._initialize_boundary = lambda self: "abc123" test_file = tempfile.TemporaryFile() resource = File.create( purpose="dispute_evidence", @@ -45,7 +45,7 @@ def test_is_creatable(self, setup_upload_api_base, http_client_mock): "post", api_base=stripe.upload_api_base, path="/v1/files", - content_type="multipart/form-data; boundary=1234567890", + content_type="multipart/form-data; boundary=abc123", ) assert isinstance(resource, File) diff --git a/tests/services/test_file_upload.py b/tests/services/test_file_upload.py index cae1c79b7..12b30a5b5 100644 --- a/tests/services/test_file_upload.py +++ b/tests/services/test_file_upload.py @@ -29,7 +29,7 @@ def test_is_creatable( file_stripe_mock_stripe_client, http_client_mock, ): - MultipartDataGenerator._initialize_boundary = lambda self: 1234567890 + MultipartDataGenerator._initialize_boundary = lambda self: "abc123" test_file = tempfile.TemporaryFile() # We create a new client here instead of re-using the stripe_mock_stripe_client fixture @@ -46,6 +46,6 @@ def test_is_creatable( "post", api_base=stripe.upload_api_base, path="/v1/files", - content_type="multipart/form-data; boundary=1234567890", + content_type="multipart/form-data; boundary=abc123", ) assert isinstance(resource, File) diff --git a/tests/test_multipart_data_generator.py b/tests/test_multipart_data_generator.py index b8f4fa738..0b9d5f93e 100644 --- a/tests/test_multipart_data_generator.py +++ b/tests/test_multipart_data_generator.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- +import random import re import io @@ -87,3 +88,51 @@ def test_multipart_data_unicode_file_name(self): string = io.StringIO("foo") string.name = "паспорт.png" self.run_test_multipart_data_with_file(string) + + def test_boundary_is_not_derived_from_the_random_module(self): + # Seeding the `random` module must not determine the boundary. A + # boundary an attacker can predict lets a caller-influenced value + # (including file bytes) inject additional parts. + random.seed(0) + first = MultipartDataGenerator().boundary + random.seed(0) + second = MultipartDataGenerator().boundary + + assert first != second + assert re.fullmatch(r"[0-9a-f]{60}", first) + assert re.fullmatch(r"[0-9a-f]{60}", second) + + @staticmethod + def lines_starting_with(http_body, prefix): + return [ + line for line in http_body.split("\r\n") if line.startswith(prefix) + ] + + def test_escapes_quotes_and_crlf_in_param_names(self): + injected = 'a\r\nContent-Disposition: form-data; name="purpose' + generator = MultipartDataGenerator() + generator.add_params({injected: "value"}) + http_body = generator.get_post_data().decode("utf-8") + + # The injected CRLF must not begin a second header line, and the + # injected quote must not end the quoted-string early. + assert self.lines_starting_with(http_body, "Content-Disposition:") == [ + 'Content-Disposition: form-data; name="a Content-Disposition: ' + 'form-data; name=%22purpose"' + ] + # One opening delimiter and one closing delimiter: a single part. + assert http_body.count("--%s" % generator.boundary) == 2 + + def test_escapes_quotes_and_crlf_in_file_names(self): + test_file = io.StringIO("foo") + test_file.name = 'a\r\nX-Injected: yes"b.png' + generator = MultipartDataGenerator() + generator.add_params({"file": test_file}) + http_body = generator.get_post_data().decode("utf-8") + + assert self.lines_starting_with(http_body, "Content-Disposition:") == [ + 'Content-Disposition: form-data; name="file"; ' + 'filename="a X-Injected: yes%22b.png"' + ] + assert self.lines_starting_with(http_body, "X-Injected:") == [] + assert http_body.count("--%s" % generator.boundary) == 2 From 3b72e99928b59a95e2f11a376b3fc9ea99e3898b Mon Sep 17 00:00:00 2001 From: David Brownman Date: Mon, 31 Aug 2026 10:55:18 -0700 Subject: [PATCH 2/3] use monkeypatch insteado of bare assignment --- tests/api_resources/test_file.py | 11 +++++++++-- tests/api_resources/test_file_upload.py | 11 +++++++++-- tests/services/test_file_upload.py | 8 +++++++- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/tests/api_resources/test_file.py b/tests/api_resources/test_file.py index 8ce21c45d..59680db4e 100644 --- a/tests/api_resources/test_file.py +++ b/tests/api_resources/test_file.py @@ -32,8 +32,15 @@ def test_is_retrievable(self, http_client_mock): ) assert isinstance(resource, stripe.File) - def test_is_creatable(self, setup_upload_api_base, http_client_mock): - MultipartDataGenerator._initialize_boundary = lambda self: "abc123" + def test_is_creatable( + self, setup_upload_api_base, http_client_mock, monkeypatch + ): + # Pin the boundary so the Content-Type assertion below is stable. + monkeypatch.setattr( + MultipartDataGenerator, + "_initialize_boundary", + lambda self: "abc123", + ) test_file = tempfile.TemporaryFile() resource = stripe.File.create( purpose="dispute_evidence", diff --git a/tests/api_resources/test_file_upload.py b/tests/api_resources/test_file_upload.py index f6f63d7bf..f14749a7d 100644 --- a/tests/api_resources/test_file_upload.py +++ b/tests/api_resources/test_file_upload.py @@ -33,8 +33,15 @@ def test_is_retrievable(self, http_client_mock): ) assert isinstance(resource, File) - def test_is_creatable(self, setup_upload_api_base, http_client_mock): - MultipartDataGenerator._initialize_boundary = lambda self: "abc123" + def test_is_creatable( + self, setup_upload_api_base, http_client_mock, monkeypatch + ): + # Pin the boundary so the Content-Type assertion below is stable. + monkeypatch.setattr( + MultipartDataGenerator, + "_initialize_boundary", + lambda self: "abc123", + ) test_file = tempfile.TemporaryFile() resource = File.create( purpose="dispute_evidence", diff --git a/tests/services/test_file_upload.py b/tests/services/test_file_upload.py index 12b30a5b5..96e6b6836 100644 --- a/tests/services/test_file_upload.py +++ b/tests/services/test_file_upload.py @@ -28,8 +28,14 @@ def test_is_creatable( self, file_stripe_mock_stripe_client, http_client_mock, + monkeypatch, ): - MultipartDataGenerator._initialize_boundary = lambda self: "abc123" + # Pin the boundary so the Content-Type assertion below is stable. + monkeypatch.setattr( + MultipartDataGenerator, + "_initialize_boundary", + lambda self: "abc123", + ) test_file = tempfile.TemporaryFile() # We create a new client here instead of re-using the stripe_mock_stripe_client fixture From 4e43e4e67d2603ee18c392d0da868e84f3269f28 Mon Sep 17 00:00:00 2001 From: David Brownman Date: Mon, 31 Aug 2026 11:00:26 -0700 Subject: [PATCH 3/3] remove unneded comments --- tests/api_resources/test_file.py | 1 - tests/api_resources/test_file_upload.py | 1 - tests/services/test_file_upload.py | 1 - 3 files changed, 3 deletions(-) diff --git a/tests/api_resources/test_file.py b/tests/api_resources/test_file.py index 59680db4e..fe7037961 100644 --- a/tests/api_resources/test_file.py +++ b/tests/api_resources/test_file.py @@ -35,7 +35,6 @@ def test_is_retrievable(self, http_client_mock): def test_is_creatable( self, setup_upload_api_base, http_client_mock, monkeypatch ): - # Pin the boundary so the Content-Type assertion below is stable. monkeypatch.setattr( MultipartDataGenerator, "_initialize_boundary", diff --git a/tests/api_resources/test_file_upload.py b/tests/api_resources/test_file_upload.py index f14749a7d..a2896a378 100644 --- a/tests/api_resources/test_file_upload.py +++ b/tests/api_resources/test_file_upload.py @@ -36,7 +36,6 @@ def test_is_retrievable(self, http_client_mock): def test_is_creatable( self, setup_upload_api_base, http_client_mock, monkeypatch ): - # Pin the boundary so the Content-Type assertion below is stable. monkeypatch.setattr( MultipartDataGenerator, "_initialize_boundary", diff --git a/tests/services/test_file_upload.py b/tests/services/test_file_upload.py index 96e6b6836..fa671c1ff 100644 --- a/tests/services/test_file_upload.py +++ b/tests/services/test_file_upload.py @@ -30,7 +30,6 @@ def test_is_creatable( http_client_mock, monkeypatch, ): - # Pin the boundary so the Content-Type assertion below is stable. monkeypatch.setattr( MultipartDataGenerator, "_initialize_boundary",