From 7c457466b76d7a4abeca521043de69d3c1f4eb11 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sun, 11 Jan 2026 11:55:52 -0800 Subject: [PATCH] Security: prevent path traversal in storage paths --- src/documents/signals/handlers.py | 24 +++++++++++++++++++++--- src/documents/templating/filepath.py | 17 +++++++++++++++++ src/documents/tests/test_api_objects.py | 24 ++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index 5f2c8b4b2..4ec00258a 100644 --- a/src/documents/signals/handlers.py +++ b/src/documents/signals/handlers.py @@ -418,7 +418,15 @@ def update_filename_and_move_files( return instance = instance.document - def validate_move(instance, old_path: Path, new_path: Path): + def validate_move(instance, old_path: Path, new_path: Path, root: Path): + if not new_path.is_relative_to(root): + msg = ( + f"Document {instance!s}: Refusing to move file outside root {root}: " + f"{new_path}." + ) + logger.warning(msg) + raise CannotMoveFilesException(msg) + if not old_path.is_file(): # Can't do anything if the old file does not exist anymore. msg = f"Document {instance!s}: File {old_path} doesn't exist." @@ -507,12 +515,22 @@ def update_filename_and_move_files( return if move_original: - validate_move(instance, old_source_path, instance.source_path) + validate_move( + instance, + old_source_path, + instance.source_path, + settings.ORIGINALS_DIR, + ) create_source_path_directory(instance.source_path) shutil.move(old_source_path, instance.source_path) if move_archive: - validate_move(instance, old_archive_path, instance.archive_path) + validate_move( + instance, + old_archive_path, + instance.archive_path, + settings.ARCHIVE_DIR, + ) create_source_path_directory(instance.archive_path) shutil.move(old_archive_path, instance.archive_path) diff --git a/src/documents/templating/filepath.py b/src/documents/templating/filepath.py index 7d76e7f31..805cefbdb 100644 --- a/src/documents/templating/filepath.py +++ b/src/documents/templating/filepath.py @@ -262,6 +262,17 @@ def get_custom_fields_context( return field_data +def _is_safe_relative_path(value: str) -> bool: + if value == "": + return True + + path = PurePath(value) + if path.is_absolute() or path.drive: + return False + + return ".." not in path.parts + + def validate_filepath_template_and_render( template_string: str, document: Document | None = None, @@ -309,6 +320,12 @@ def validate_filepath_template_and_render( ) rendered_template = template.render(context) + if not _is_safe_relative_path(rendered_template): + logger.warning( + "Template rendered an unsafe path (absolute or containing traversal).", + ) + return None + # We're good! return rendered_template except UndefinedError: diff --git a/src/documents/tests/test_api_objects.py b/src/documents/tests/test_api_objects.py index 014dd3c2a..0eb99f023 100644 --- a/src/documents/tests/test_api_objects.py +++ b/src/documents/tests/test_api_objects.py @@ -219,6 +219,30 @@ class TestApiStoragePaths(DirectoriesMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(StoragePath.objects.count(), 1) + def test_api_create_storage_path_rejects_traversal(self): + """ + GIVEN: + - API request to create a storage paths + - Storage path attempts directory traversal + WHEN: + - API is called + THEN: + - Correct HTTP 400 response + - No storage path is created + """ + response = self.client.post( + self.ENDPOINT, + json.dumps( + { + "name": "Traversal path", + "path": "../../../../../tmp/proof", + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(StoragePath.objects.count(), 1) + def test_api_storage_path_placeholders(self): """ GIVEN: