diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..ecc9f2c --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-06-26 - [Null Byte Bypass in File Extension Validation] +**Vulnerability:** File extension validation was vulnerable to null byte injection (e.g. `file.hwp\u0000`). +**Learning:** `String.strip()` does not remove null characters, leaving them at the end of the extracted extension, bypassing exact match checks in sets (like `blockedExtensions.contains(extension)`). +**Prevention:** Always explicitly remove or reject null bytes (`\u0000`) before performing file extension extraction or validation, as many string normalization functions only target whitespace. diff --git a/pom.xml b/pom.xml index 6b7e40d..a5f3cb2 100644 --- a/pom.xml +++ b/pom.xml @@ -89,6 +89,25 @@ @{argLine} -Xshare:off -XX:+EnableDynamicAgentLoading --enable-native-access=ALL-UNNAMED + + org.jacoco + jacoco-maven-plugin + 0.8.12 + + + + prepare-agent + + + + report + test + + report + + + + org.springframework.boot spring-boot-maven-plugin diff --git a/src/main/java/com/clearfolio/viewer/service/DefaultDocumentValidationService.java b/src/main/java/com/clearfolio/viewer/service/DefaultDocumentValidationService.java index b898f7a..b753202 100644 --- a/src/main/java/com/clearfolio/viewer/service/DefaultDocumentValidationService.java +++ b/src/main/java/com/clearfolio/viewer/service/DefaultDocumentValidationService.java @@ -55,6 +55,11 @@ public void validateOrThrow(MultipartFile file, PolicyOverrideRequest overrideRe } String fileName = file.getOriginalFilename(); + if (fileName != null && fileName.indexOf('\u0000') != -1) { + throw new IllegalArgumentException( + "File name contains invalid characters."); + } + String extension = extensionOf(fileName); if (extension.isEmpty()) { throw new IllegalArgumentException("File extension is required."); diff --git a/src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationServiceTest.java b/src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationServiceTest.java index dc1df79..5b27067 100644 --- a/src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationServiceTest.java +++ b/src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationServiceTest.java @@ -38,6 +38,38 @@ void rejectsHwpAndHwpxByDefault() { assertEquals("hwp", ex.getExtension()); } + @Test + void rejectsFilenameWithNullByteBypass() { + ConversionProperties conversionProperties = new ConversionProperties(); + conversionProperties.setBlockedExtensions(Set.of("hwp", "hwpx")); + DefaultDocumentValidationService validationService = new DefaultDocumentValidationService(conversionProperties); + + IllegalArgumentException ex = assertThrows( + IllegalArgumentException.class, + () -> validationService.validateOrThrow( + new MockMultipartFile("file", "contract.hwp\u0000", "application/octet-stream", new byte[] {1}) + ) + ); + + assertEquals("File name contains invalid characters.", ex.getMessage()); + } + + @Test + void rejectsFilenameWithNullByteTruncationAttack() { + ConversionProperties conversionProperties = new ConversionProperties(); + conversionProperties.setBlockedExtensions(Set.of("hwp", "hwpx")); + DefaultDocumentValidationService validationService = new DefaultDocumentValidationService(conversionProperties); + + IllegalArgumentException ex = assertThrows( + IllegalArgumentException.class, + () -> validationService.validateOrThrow( + new MockMultipartFile("file", "malicious.sh\u0000.pdf", "application/pdf", new byte[] {1}) + ) + ); + + assertEquals("File name contains invalid characters.", ex.getMessage()); + } + @Test void allowsBlockedExtensionWhenOverrideHeadersAreValid() { ConversionProperties conversionProperties = new ConversionProperties();