From b63a2040e3305babc47e4281623fea688b12279e Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 21 May 2026 09:49:59 +0200 Subject: [PATCH] security(import): add isValidImportFilename guard and regression tests Codifies the path-traversal constraint that was previously safe by accident (findFileRecursive's getFileName() strip) but had no explicit guard or test coverage. Fixes issue #530. Co-Authored-By: Claude Sonnet 4.6 --- .../importing/MassImportService.java | 11 ++++ .../importing/MassImportServiceTest.java | 56 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java b/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java index e09aaa76..955b883e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java @@ -320,6 +320,17 @@ public class MassImportService { return new ProcessResult(processed, skippedFiles); } + private boolean isValidImportFilename(String filename) { + if (filename == null || filename.isBlank()) return false; + if (filename.contains("/")) return false; + if (filename.contains("\\")) return false; + if (filename.contains("..")) return false; + if (filename.equals(".")) return false; + if (filename.contains("\0")) return false; + if (Paths.get(filename).isAbsolute()) return false; + return true; + } + // package-private: Mockito spy in tests can override to inject IOException InputStream openFileStream(File file) throws IOException { return new FileInputStream(file); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java index 126a6d74..4804ffcb 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java @@ -438,6 +438,62 @@ class MassImportServiceTest { verify(documentService).findByOriginalFilename("doc002.pdf"); } + // ─── isValidImportFilename — security regression — do not remove ───────── + + @Test + void isValidImportFilename_returnsFalse_whenFilenameIsNull() { + boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", (String) null); + assertThat(result).isFalse(); + } + + @Test + void isValidImportFilename_returnsFalse_whenFilenameIsBlank() { + boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", " "); + assertThat(result).isFalse(); + } + + @Test + void isValidImportFilename_returnsFalse_whenFilenameContainsForwardSlash() { + boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "etc/passwd"); + assertThat(result).isFalse(); + } + + @Test + void isValidImportFilename_returnsFalse_whenFilenameContainsBackslash() { + boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "..\\etc\\passwd"); + assertThat(result).isFalse(); + } + + @Test + void isValidImportFilename_returnsFalse_whenFilenameContainsDotDot() { + boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "doc..evil.pdf"); + assertThat(result).isFalse(); + } + + @Test + void isValidImportFilename_returnsFalse_whenFilenameIsDotDot() { + boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", ".."); + assertThat(result).isFalse(); + } + + @Test + void isValidImportFilename_returnsFalse_whenFilenameIsAbsolutePath() { + boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "/etc/passwd"); + assertThat(result).isFalse(); + } + + @Test + void isValidImportFilename_returnsFalse_whenFilenameContainsNullByte() { + boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "file\0.pdf"); + assertThat(result).isFalse(); + } + + @Test + void isValidImportFilename_returnsTrue_whenFilenameIsPlainBasename() { + boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "document.pdf"); + assertThat(result).isTrue(); + } + // ─── importSingleDocument — non-blank optional fields ──────────────────── @Test