security(import): add canonical path escape guard in findFileRecursive
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m27s
CI / OCR Service Tests (pull_request) Successful in 20s
CI / Backend Unit Tests (pull_request) Successful in 3m41s
CI / fail2ban Regex (pull_request) Successful in 42s
CI / Semgrep Security Scan (pull_request) Successful in 19s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m0s
CI / Unit & Component Tests (push) Successful in 3m26s
CI / OCR Service Tests (push) Successful in 20s
CI / Backend Unit Tests (push) Successful in 3m24s
CI / fail2ban Regex (push) Successful in 41s
CI / Semgrep Security Scan (push) Successful in 18s
CI / Compose Bucket Idempotency (push) Successful in 1m0s
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m27s
CI / OCR Service Tests (pull_request) Successful in 20s
CI / Backend Unit Tests (pull_request) Successful in 3m41s
CI / fail2ban Regex (pull_request) Successful in 42s
CI / Semgrep Security Scan (pull_request) Successful in 19s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m0s
CI / Unit & Component Tests (push) Successful in 3m26s
CI / OCR Service Tests (push) Successful in 20s
CI / Backend Unit Tests (push) Successful in 3m24s
CI / fail2ban Regex (push) Successful in 41s
CI / Semgrep Security Scan (push) Successful in 18s
CI / Compose Bucket Idempotency (push) Successful in 1m0s
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 <noreply@anthropic.com>
This commit was merged in pull request #650.
This commit is contained in:
@@ -490,11 +490,18 @@ public class MassImportService {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private Optional<File> findFileRecursive(String filename) {
|
private Optional<File> findFileRecursive(String filename) {
|
||||||
try (Stream<Path> walk = Files.walk(Paths.get(importDir))) {
|
File baseDir = new File(importDir);
|
||||||
return walk.filter(p -> !Files.isDirectory(p))
|
try (Stream<Path> walk = Files.walk(baseDir.toPath())) {
|
||||||
|
Optional<Path> match = walk.filter(p -> !Files.isDirectory(p))
|
||||||
.filter(p -> p.getFileName().toString().equals(filename))
|
.filter(p -> p.getFileName().toString().equals(filename))
|
||||||
.map(Path::toFile)
|
|
||||||
.findFirst();
|
.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) {
|
} catch (IOException e) {
|
||||||
return Optional.empty();
|
return Optional.empty();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -758,6 +758,21 @@ class MassImportServiceTest {
|
|||||||
.containsExactly(MassImportService.SkipReason.FILE_READ_ERROR);
|
.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 ───────────────────────────────────
|
// ─── readOds — XXE security regression ───────────────────────────────────
|
||||||
|
|
||||||
// Security regression — do not remove.
|
// Security regression — do not remove.
|
||||||
|
|||||||
Reference in New Issue
Block a user