From d48e0145e87c745e4dc9c0313e5d875651186192 Mon Sep 17 00:00:00 2001 From: Harsh Thakare Date: Mon, 13 Jul 2026 12:22:19 +0530 Subject: [PATCH 1/3] Sanitize buffered upload filenames --- .../reflex_components_core/core/_upload.py | 45 ++++++++++---- tests/units/components/core/test_upload.py | 62 ++++++++++++++++--- 2 files changed, 87 insertions(+), 20 deletions(-) diff --git a/packages/reflex-components-core/src/reflex_components_core/core/_upload.py b/packages/reflex-components-core/src/reflex_components_core/core/_upload.py index 423bc83403c..9283634e484 100644 --- a/packages/reflex-components-core/src/reflex_components_core/core/_upload.py +++ b/packages/reflex-components-core/src/reflex_components_core/core/_upload.py @@ -14,7 +14,7 @@ Callable, MutableMapping, ) -from pathlib import Path +from pathlib import Path, PurePosixPath, PureWindowsPath from typing import TYPE_CHECKING, Any, BinaryIO, cast from python_multipart.multipart import MultipartParser, parse_options_header @@ -75,6 +75,35 @@ def name(self) -> str | None: return None +def _sanitize_upload_filename(filename: str) -> str: + """Strip client-supplied path segments from an upload filename. + + Args: + filename: The raw multipart filename. + + Returns: + The basename of the uploaded file. + """ + return PureWindowsPath(PurePosixPath(filename.lstrip("/\\")).name).name + + +def _upload_file_from_starlette(file: StarletteUploadFile) -> UploadFile: + """Create a Reflex upload file from a Starlette upload file. + + Args: + file: The Starlette upload file. + + Returns: + The Reflex upload file. + """ + return UploadFile( + file=file.file, + path=Path(_sanitize_upload_filename(file.filename)) if file.filename else None, + size=file.size, + headers=file.headers, + ) + + @dataclasses.dataclass(frozen=True, kw_only=True, slots=True) class UploadChunk: """A chunk of uploaded file data.""" @@ -371,8 +400,9 @@ def on_headers_finished(self) -> None: # phantom file upload; reject it instead of silently dropping it. msg = "Upload event args must be a text field, not a file." raise MultiPartException(msg) - filename = _user_safe_decode(options[b"filename"], self._charset) - filename = Path(filename.lstrip("/")).name + filename = _sanitize_upload_filename( + _user_safe_decode(options[b"filename"], self._charset) + ) content_type = "" for header_name, header_value in self._current_part.item_headers: @@ -587,14 +617,7 @@ def _create_upload_event() -> Event: raise UploadValueError( "Uploaded file is not an UploadFile." + str(file) ) - file_uploads.append( - UploadFile( - file=file.file, - path=Path(file.filename.lstrip("/")) if file.filename else None, - size=file.size, - headers=file.headers, - ) - ) + file_uploads.append(_upload_file_from_starlette(file)) return Event( name=handler_name, diff --git a/tests/units/components/core/test_upload.py b/tests/units/components/core/test_upload.py index eddef933789..ab64b2de2c7 100644 --- a/tests/units/components/core/test_upload.py +++ b/tests/units/components/core/test_upload.py @@ -12,6 +12,8 @@ UploadChunkIterator, _buffered_upload_args, _decode_event_args, + _sanitize_upload_filename, + _upload_file_from_starlette, _UploadChunkMultipartParser, ) from reflex_components_core.core.upload import ( @@ -333,8 +335,37 @@ def test_buffered_upload_args_file_rejected(): assert exc_info.value.status_code == 400 +@pytest.mark.parametrize( + ("filename", "expected"), + [ + ("plain.txt", "plain.txt"), + ("../secret.txt", "secret.txt"), + ("nested/path/report.csv", "report.csv"), + (r"..\secret.txt", "secret.txt"), + (r"C:\Users\name\report.csv", "report.csv"), + ], +) +def test_upload_filename_sanitization_drops_path_segments(filename: str, expected: str): + """Uploaded filenames are normalized to a basename on every platform.""" + assert _sanitize_upload_filename(filename) == expected + + +def test_buffered_upload_file_path_drops_path_segments(): + """Buffered uploads expose the same sanitized filename as streamed uploads.""" + upload = StarletteUploadFile(file=io.BytesIO(b"data"), filename="../secret.txt") + reflex_upload = _upload_file_from_starlette(upload) + + assert reflex_upload.path is not None + assert str(reflex_upload.path) == "secret.txt" + assert reflex_upload.name == "secret.txt" + + def _multipart_body( - boundary: str, *, args: str | None = None, file_first: bool = False + boundary: str, + *, + args: str | None = None, + file_first: bool = False, + filename: str = "a.txt", ) -> bytes: """Build a multipart upload body, optionally with a bound-args field. @@ -342,6 +373,7 @@ def _multipart_body( boundary: The multipart boundary token. args: The raw bound-args field value, or ``None`` to omit it. file_first: Place the file part before the args field (contract violation). + filename: The raw multipart filename. Returns: The encoded multipart body. @@ -355,7 +387,7 @@ def _multipart_body( ) file_part = ( f"--{boundary}\r\n" - 'Content-Disposition: form-data; name="files"; filename="a.txt"\r\n' + f'Content-Disposition: form-data; name="files"; filename="{filename}"\r\n' "Content-Type: text/plain\r\n\r\n" "hello\r\n" ) @@ -365,7 +397,7 @@ def _multipart_body( async def _run_chunk_parser( body: bytes, boundary: str, *, charset: str | None = None -) -> tuple[str | None, list[bytes]]: +) -> tuple[str | None, list[bytes], list[str]]: """Drive the streaming chunk parser over a multipart body. Args: @@ -374,7 +406,7 @@ async def _run_chunk_parser( charset: Optional charset to advertise in the request Content-Type. Returns: - The raw args field passed to dispatch and the file chunk payloads. + The raw args field passed to dispatch, file chunk payloads, and chunk filenames. """ async def _stream(): @@ -402,13 +434,17 @@ async def _on_args_ready(raw: str | None) -> None: ) await parser.parse() await chunk_iter.finish() - chunks = [chunk.data async for chunk in chunk_iter] - return await args_received.get(), chunks + chunks = [chunk async for chunk in chunk_iter] + return ( + await args_received.get(), + [chunk.data for chunk in chunks], + [chunk.filename for chunk in chunks], + ) async def test_chunk_parser_captures_bound_args_before_file(): """The streaming parser captures the leading args field and emits the file.""" - raw, chunks = await _run_chunk_parser( + raw, chunks, _ = await _run_chunk_parser( _multipart_body("BOUNDARY", args='{"field": "value"}'), "BOUNDARY" ) assert _decode_event_args(raw) == {"field": "value"} @@ -417,14 +453,14 @@ async def test_chunk_parser_captures_bound_args_before_file(): async def test_chunk_parser_without_args_dispatches_empty(): """A streaming upload with no args field still dispatches (with empty args).""" - raw, chunks = await _run_chunk_parser(_multipart_body("BOUNDARY"), "BOUNDARY") + raw, chunks, _ = await _run_chunk_parser(_multipart_body("BOUNDARY"), "BOUNDARY") assert _decode_event_args(raw) == {} assert b"".join(chunks) == b"hello" async def test_chunk_parser_bogus_charset_does_not_crash(): """A bogus request charset must fall back, not raise LookupError -> 500.""" - raw, chunks = await _run_chunk_parser( + raw, chunks, _ = await _run_chunk_parser( _multipart_body("BOUNDARY", args='{"field": "value"}'), "BOUNDARY", charset="bogus-cs", @@ -440,6 +476,14 @@ async def test_chunk_parser_args_after_file_rejected(): await _run_chunk_parser(body, "BOUNDARY") +async def test_chunk_parser_sanitizes_uploaded_filename(): + """Streaming uploads drop client-supplied path segments from filenames.""" + _, _, filenames = await _run_chunk_parser( + _multipart_body("BOUNDARY", filename="../secret.txt"), "BOUNDARY" + ) + assert filenames == ["secret.txt"] + + async def test_chunk_parser_args_as_file_rejected(): """A file under the args field name is rejected, not treated as a phantom file.""" body = ( From 0d530016e98234b8376aeac341120be279befa43 Mon Sep 17 00:00:00 2001 From: Harsh Thakare Date: Mon, 13 Jul 2026 12:51:00 +0530 Subject: [PATCH 2/3] Add upload filename changelog fragment --- packages/reflex-components-core/news/6753.bugfix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 packages/reflex-components-core/news/6753.bugfix.md diff --git a/packages/reflex-components-core/news/6753.bugfix.md b/packages/reflex-components-core/news/6753.bugfix.md new file mode 100644 index 00000000000..d5bae4aaf50 --- /dev/null +++ b/packages/reflex-components-core/news/6753.bugfix.md @@ -0,0 +1 @@ +Sanitize buffered upload filenames the same way streamed upload filenames are sanitized. From a82891058bfecc0f946c7827269352ceb2fb81fb Mon Sep 17 00:00:00 2001 From: Harsh Thakare Date: Tue, 14 Jul 2026 12:28:00 +0530 Subject: [PATCH 3/3] Preserve safe upload directory paths --- .../reflex_components_core/core/_upload.py | 16 +++++++--- tests/units/components/core/test_upload.py | 31 ++++++++++++------- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/packages/reflex-components-core/src/reflex_components_core/core/_upload.py b/packages/reflex-components-core/src/reflex_components_core/core/_upload.py index 9283634e484..2d9d7abff27 100644 --- a/packages/reflex-components-core/src/reflex_components_core/core/_upload.py +++ b/packages/reflex-components-core/src/reflex_components_core/core/_upload.py @@ -14,7 +14,7 @@ Callable, MutableMapping, ) -from pathlib import Path, PurePosixPath, PureWindowsPath +from pathlib import Path, PureWindowsPath from typing import TYPE_CHECKING, Any, BinaryIO, cast from python_multipart.multipart import MultipartParser, parse_options_header @@ -76,15 +76,23 @@ def name(self) -> str | None: def _sanitize_upload_filename(filename: str) -> str: - """Strip client-supplied path segments from an upload filename. + """Normalize a client-supplied upload filename or relative path. Args: filename: The raw multipart filename. Returns: - The basename of the uploaded file. + A safe relative upload path. """ - return PureWindowsPath(PurePosixPath(filename.lstrip("/\\")).name).name + windows_path = PureWindowsPath(filename) + normalized = filename.replace("\\", "/") + if normalized.startswith("/") or windows_path.drive: + return windows_path.name + + safe_parts = [part for part in normalized.split("/") if part not in {"", ".", ".."}] + if safe_parts: + return "/".join(safe_parts) + return windows_path.name def _upload_file_from_starlette(file: StarletteUploadFile) -> UploadFile: diff --git a/tests/units/components/core/test_upload.py b/tests/units/components/core/test_upload.py index ab64b2de2c7..7a498c0575e 100644 --- a/tests/units/components/core/test_upload.py +++ b/tests/units/components/core/test_upload.py @@ -340,24 +340,33 @@ def test_buffered_upload_args_file_rejected(): [ ("plain.txt", "plain.txt"), ("../secret.txt", "secret.txt"), - ("nested/path/report.csv", "report.csv"), + ("nested/path/report.csv", "nested/path/report.csv"), (r"..\secret.txt", "secret.txt"), (r"C:\Users\name\report.csv", "report.csv"), ], ) def test_upload_filename_sanitization_drops_path_segments(filename: str, expected: str): - """Uploaded filenames are normalized to a basename on every platform.""" + """Unsafe path segments are removed from uploaded filenames.""" assert _sanitize_upload_filename(filename) == expected -def test_buffered_upload_file_path_drops_path_segments(): - """Buffered uploads expose the same sanitized filename as streamed uploads.""" - upload = StarletteUploadFile(file=io.BytesIO(b"data"), filename="../secret.txt") +def test_upload_filename_sanitization_preserves_relative_directory(): + """Directory uploads retain safe relative path segments.""" + assert _sanitize_upload_filename("photos/2026/image.png") == ( + "photos/2026/image.png" + ) + + +def test_buffered_upload_file_path_preserves_relative_directory(): + """Buffered uploads expose safe relative directory paths.""" + upload = StarletteUploadFile( + file=io.BytesIO(b"data"), filename="photos/2026/image.png" + ) reflex_upload = _upload_file_from_starlette(upload) assert reflex_upload.path is not None - assert str(reflex_upload.path) == "secret.txt" - assert reflex_upload.name == "secret.txt" + assert str(reflex_upload.path).replace("\\", "/") == "photos/2026/image.png" + assert reflex_upload.name == "image.png" def _multipart_body( @@ -476,12 +485,12 @@ async def test_chunk_parser_args_after_file_rejected(): await _run_chunk_parser(body, "BOUNDARY") -async def test_chunk_parser_sanitizes_uploaded_filename(): - """Streaming uploads drop client-supplied path segments from filenames.""" +async def test_chunk_parser_preserves_relative_directory_filename(): + """Streaming uploads retain safe relative directory paths.""" _, _, filenames = await _run_chunk_parser( - _multipart_body("BOUNDARY", filename="../secret.txt"), "BOUNDARY" + _multipart_body("BOUNDARY", filename="photos/2026/image.png"), "BOUNDARY" ) - assert filenames == ["secret.txt"] + assert filenames == ["photos/2026/image.png"] async def test_chunk_parser_args_as_file_rejected():