From eca4f1f0e826571ab24af4f28410dc1bab6a2e4e Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 21 May 2026 10:16:18 +0200 Subject: [PATCH] security(import): add canonical path escape guard in findFileRecursive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A symlink placed inside importDir pointing to a file outside it would pass isValidImportFilename (no forbidden chars in the symlink name) and be found by Files.walk. Now checks candidate.getCanonicalPath() against baseDir.getCanonicalPath() — if the resolved path escapes importDir, throws DomainException.internal and aborts the import. Adds regression test using @TempDir + Files.createSymbolicLink. Co-Authored-By: Claude Sonnet 4.6 --- .../importing/MassImportService.java | 13 ++++++++++--- .../importing/MassImportServiceTest.java | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) 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 ccd62649..975517e7 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java @@ -490,11 +490,18 @@ public class MassImportService { } private Optional findFileRecursive(String filename) { - try (Stream walk = Files.walk(Paths.get(importDir))) { - return walk.filter(p -> !Files.isDirectory(p)) + File baseDir = new File(importDir); + try (Stream walk = Files.walk(baseDir.toPath())) { + Optional match = walk.filter(p -> !Files.isDirectory(p)) .filter(p -> p.getFileName().toString().equals(filename)) - .map(Path::toFile) .findFirst(); + if (match.isEmpty()) return Optional.empty(); + File candidate = match.get().toFile(); + String baseDirCanonical = baseDir.getCanonicalPath(); + if (!candidate.getCanonicalPath().startsWith(baseDirCanonical + File.separator)) { + throw DomainException.internal(ErrorCode.INTERNAL_ERROR, "Path escape detected: " + candidate); + } + return Optional.of(candidate); } catch (IOException e) { return Optional.empty(); } 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 575f568c..d87d28c1 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java @@ -758,6 +758,21 @@ class MassImportServiceTest { .containsExactly(MassImportService.SkipReason.FILE_READ_ERROR); } + // ─── findFileRecursive — symlink escape security regression — do not remove ─ + + @Test + void findFileRecursive_throwsDomainException_whenSymlinkEscapesImportDir( + @TempDir Path importDirPath, @TempDir Path outsideDir) throws Exception { + Path outsideFile = outsideDir.resolve("secret.pdf"); + Files.writeString(outsideFile, "sensitive content"); + Files.createSymbolicLink(importDirPath.resolve("secret.pdf"), outsideFile); + + ReflectionTestUtils.setField(service, "importDir", importDirPath.toString()); + + assertThatThrownBy(() -> ReflectionTestUtils.invokeMethod(service, "findFileRecursive", "secret.pdf")) + .isInstanceOf(DomainException.class); + } + // ─── readOds — XXE security regression ─────────────────────────────────── // Security regression — do not remove.