diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py index a56eb014a..399282dc3 100644 --- a/src/documents/bulk_edit.py +++ b/src/documents/bulk_edit.py @@ -899,17 +899,26 @@ def edit_pdf( pdf_docs: list[pikepdf.Pdf] = [] try: + if not operations: + raise ValueError("Output document index is out of bounds") + + max_idx = max(op.get("doc", 0) for op in operations) + if update_document and max_idx > 0: + logger.error( + "Update requested but multiple output documents specified", + ) + raise ValueError("Multiple output documents specified") + + if any( + op.get("doc", 0) < 0 or op.get("doc", 0) >= len(operations) + for op in operations + ): + raise ValueError("Output document index is out of bounds") + with pikepdf.open(pair.source_doc.source_path) as src: # prepare output documents - max_idx = max(op.get("doc", 0) for op in operations) pdf_docs = [pikepdf.new() for _ in range(max_idx + 1)] - if update_document and len(pdf_docs) > 1: - logger.error( - "Update requested but multiple output documents specified", - ) - raise ValueError("Multiple output documents specified") - for op in operations: dst = pdf_docs[op.get("doc", 0)] page = src.pages[op["page"] - 1] diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 8b817b3f2..8afa2c53d 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -1788,6 +1788,12 @@ class EditPdfDocumentsSerializer(DocumentListSerializer, SourceModeValidationMix "update_document only allowed with a single output document", ) + if any( + op.get("doc", 0) < 0 or op.get("doc", 0) >= len(operations) + for op in operations + ): + raise serializers.ValidationError("doc index is out of bounds") + doc = Document.objects.get(id=documents[0]) if doc.page_count: for op in operations: @@ -2151,6 +2157,12 @@ class BulkEditSerializer( "update_document only allowed with a single output document", ) + if any( + op.get("doc", 0) < 0 or op.get("doc", 0) >= len(parameters["operations"]) + for op in parameters["operations"] + ): + raise serializers.ValidationError("doc index is out of bounds") + doc = Document.objects.get(id=document_id) # doc existence is already validated if doc.page_count: diff --git a/src/documents/tests/test_api_bulk_edit.py b/src/documents/tests/test_api_bulk_edit.py index 714e1bcfd..c4f26f877 100644 --- a/src/documents/tests/test_api_bulk_edit.py +++ b/src/documents/tests/test_api_bulk_edit.py @@ -1649,6 +1649,24 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + def test_legacy_bulk_edit_rejects_out_of_bounds_pdf_doc_index(self) -> None: + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "edit_pdf", + "parameters": { + "operations": [{"page": 1, "doc": 2**32}], + }, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"doc index is out of bounds", response.content) + @mock.patch("documents.views.bulk_edit.edit_pdf") def test_edit_pdf(self, m) -> None: self.setup_mock(m, "edit_pdf") @@ -1751,6 +1769,21 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertIn(b"doc must be an integer", response.content) + for doc_index in (-1, 2**32): + with self.subTest(doc_index=doc_index): + response = self.client.post( + "/api/documents/edit_pdf/", + json.dumps( + { + "documents": [self.doc2.id], + "operations": [{"page": 1, "doc": doc_index}], + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"doc index is out of bounds", response.content) + response = self.client.post( "/api/documents/edit_pdf/", json.dumps( diff --git a/src/documents/tests/test_bulk_edit.py b/src/documents/tests/test_bulk_edit.py index 6df2a9c53..72cbd20e0 100644 --- a/src/documents/tests/test_bulk_edit.py +++ b/src/documents/tests/test_bulk_edit.py @@ -1642,6 +1642,17 @@ class TestPDFActions(DirectoriesMixin, TestCase): mock_group.assert_not_called() mock_consume_file.assert_not_called() + @mock.patch("pikepdf.open") + def test_edit_pdf_rejects_out_of_bounds_output_index(self, mock_open) -> None: + with self.assertLogs("paperless.bulk_edit", level="ERROR"): + with self.assertRaisesRegex(ValueError, "index is out of bounds"): + bulk_edit.edit_pdf( + [self.doc2.id], + [{"page": 1, "doc": 2**32}], + ) + + mock_open.assert_not_called() + @mock.patch("documents.bulk_edit.update_document_content_maybe_archive_file.delay") @mock.patch("documents.tasks.consume_file.apply_async") @mock.patch("documents.bulk_edit.tempfile.mkdtemp")