Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2026-06-28 - [Null Byte Injection Bypass in File Extension Validation]
**Vulnerability:** A file named `malicious.hwp\0.pdf` bypasses the `blockedExtensions` check (which verifies against `.pdf`) in `DefaultDocumentValidationService`, but could be subsequently processed as `.hwp` by systems or libraries written in C/C++ that terminate strings at the null byte.
**Learning:** Checking the extension after the last dot does not secure the system if the file name contains a null byte, which is ignored in Java but terminates strings in C/C++.
**Prevention:** Explicitly validate all uploaded file names against null bytes before performing extension extraction or other validation checks.
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.clearfolio.viewer.service;

import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
Expand All @@ -16,13 +18,25 @@
import com.clearfolio.viewer.exception.UnsupportedDocumentFormatException;

/**
* Default document validator that enforces extension and size constraints.
* Default document validator that enforces extension, size, and content signature constraints.
*/
@Service
public class DefaultDocumentValidationService implements DocumentValidationService {

private static final Logger LOGGER = LoggerFactory.getLogger(DefaultDocumentValidationService.class);
private static final int FINGERPRINT_TRUNCATE_BYTES = 8;
private static final int SIGNATURE_BYTES = 8;
private static final byte[] PDF_SIGNATURE = "%PDF-".getBytes(StandardCharsets.US_ASCII);
private static final byte[] ZIP_LOCAL_FILE_SIGNATURE = new byte[] {0x50, 0x4B, 0x03, 0x04};
private static final byte[] ZIP_EMPTY_ARCHIVE_SIGNATURE = new byte[] {0x50, 0x4B, 0x05, 0x06};
private static final byte[] ZIP_SPANNED_ARCHIVE_SIGNATURE = new byte[] {0x50, 0x4B, 0x07, 0x08};
private static final byte[] OLE_COMPOUND_FILE_SIGNATURE = new byte[] {
(byte) 0xD0, (byte) 0xCF, 0x11, (byte) 0xE0, (byte) 0xA1, (byte) 0xB1, 0x1A, (byte) 0xE1
};
private static final byte[] RTF_SIGNATURE = "{\\rtf".getBytes(StandardCharsets.US_ASCII);
private static final Set<String> ZIP_DOCUMENT_EXTENSIONS = Set.of("docx", "pptx", "xlsx", "odt", "odp", "ods");
private static final Set<String> OLE_DOCUMENT_EXTENSIONS = Set.of("doc", "ppt", "xls");
private static final Set<String> TEXT_DOCUMENT_EXTENSIONS = Set.of("csv", "md", "txt");

private final Set<String> blockedExtensions;
private final long maxUploadSizeBytes;
Expand Down Expand Up @@ -55,6 +69,10 @@ public void validateOrThrow(MultipartFile file, PolicyOverrideRequest overrideRe
}

String fileName = file.getOriginalFilename();
if (fileName != null && fileName.indexOf('\0') != -1) {
throw new IllegalArgumentException("File name is invalid.");
}

String extension = extensionOf(fileName);
if (extension.isEmpty()) {
throw new IllegalArgumentException("File extension is required.");
Expand Down Expand Up @@ -87,6 +105,10 @@ public void validateOrThrow(MultipartFile file, PolicyOverrideRequest overrideRe
throw new IllegalArgumentException("File is too large.");
}

if (!blockedExtension) {
validateContentSignature(file, extension);
}

if (blockedExtension) {
LOGGER.info(
"Blocked-format override accepted extension={} approverId={} tokenFingerprint={}",
Expand All @@ -97,6 +119,70 @@ public void validateOrThrow(MultipartFile file, PolicyOverrideRequest overrideRe
}
}

private void validateContentSignature(MultipartFile file, String extension) {
byte[] header = readHeader(file);
boolean valid = switch (extension) {
case "pdf" -> startsWith(header, PDF_SIGNATURE);
case "rtf" -> startsWith(header, RTF_SIGNATURE);
default -> {
if (ZIP_DOCUMENT_EXTENSIONS.contains(extension)) {
yield startsWith(header, ZIP_LOCAL_FILE_SIGNATURE)
|| startsWith(header, ZIP_EMPTY_ARCHIVE_SIGNATURE)
|| startsWith(header, ZIP_SPANNED_ARCHIVE_SIGNATURE);
}
if (OLE_DOCUMENT_EXTENSIONS.contains(extension)) {
yield startsWith(header, OLE_COMPOUND_FILE_SIGNATURE);
}
if (TEXT_DOCUMENT_EXTENSIONS.contains(extension)) {
yield isTextLike(header);
}
throw new IllegalArgumentException("File extension is not supported.");
}
};

if (!valid) {
throw new IllegalArgumentException("File content does not match file extension.");
}
}

private byte[] readHeader(MultipartFile file) {
byte[] header = new byte[SIGNATURE_BYTES];
try (InputStream inputStream = file.getInputStream()) {
int bytesRead = inputStream.readNBytes(header, 0, header.length);
if (bytesRead == header.length) {
return header;
}

byte[] truncated = new byte[bytesRead];
System.arraycopy(header, 0, truncated, 0, bytesRead);
return truncated;
} catch (IOException ex) {
throw new IllegalArgumentException("File content could not be read.", ex);
}
}

private boolean startsWith(byte[] actual, byte[] expected) {
if (actual.length < expected.length) {
return false;
}
for (int index = 0; index < expected.length; index++) {
if (actual[index] != expected[index]) {
return false;
}
}
return true;
}

private boolean isTextLike(byte[] header) {
for (byte value : header) {
int unsigned = Byte.toUnsignedInt(value);
if (unsigned == 0 || unsigned < 0x09 || (unsigned > 0x0D && unsigned < 0x20)) {
return false;
}
}
return true;
}

private String extensionOf(String fileName) {
if (fileName == null || fileName.isBlank()) {
return "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ void submitReturnsBadRequestWhenServiceUploadLimitIsExceeded() {

@Test
void submitAcceptsDocumentAtServiceUploadLimitBoundary() {
byte[] payload = new byte[1024];
byte[] payload = docxBytes(1024);

submit("report.docx", payload)
.expectStatus().isAccepted()
Expand Down Expand Up @@ -173,4 +173,13 @@ private static void assertNonBlankTraceId(Object value) {
String traceId = (String) value;
assertFalse(traceId.isBlank());
}

private static byte[] docxBytes(int size) {
byte[] bytes = new byte[size];
bytes[0] = 0x50;
bytes[1] = 0x4B;
bytes[2] = 0x03;
bytes[3] = 0x04;
return bytes;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void validateOrThrow(MultipartFile file, PolicyOverrideRequest overrideRe
"file",
"contract.docx",
"application/octet-stream",
"hello-viewer".getBytes()
docxBytes("hello-viewer")
);

UUID jobId = service.submit(file, null);
Expand All @@ -133,7 +133,7 @@ void returnsSameJobIdForDuplicatePayloads() {
"file",
"contract.docx",
"application/octet-stream",
"hello-viewer".getBytes()
docxBytes("hello-viewer")
);

UUID first = service.submit(file);
Expand All @@ -158,13 +158,13 @@ void returnsDifferentJobIdsForDifferentPayloads() {
"file",
"contract.docx",
"application/octet-stream",
"hello-viewer".getBytes()
docxBytes("hello-viewer")
);
MockMultipartFile secondFile = new MockMultipartFile(
"file",
"contract.docx",
"application/octet-stream",
"different-content".getBytes()
docxBytes("different-content")
);

UUID first = service.submit(firstFile);
Expand All @@ -189,7 +189,7 @@ void returnsSameJobForConcurrentDuplicatePayloads() throws Exception {
"file",
"contract.docx",
"application/octet-stream",
"hello-viewer-concurrent".getBytes()
docxBytes("hello-viewer-concurrent")
);

ExecutorService executor = Executors.newFixedThreadPool(8);
Expand Down Expand Up @@ -552,6 +552,17 @@ UUID lastEnqueuedJobId() {
}
}

private static byte[] docxBytes(String payload) {
byte[] suffix = payload.getBytes(StandardCharsets.UTF_8);
byte[] bytes = new byte[suffix.length + 4];
bytes[0] = 0x50;
bytes[1] = 0x4B;
bytes[2] = 0x03;
bytes[3] = 0x04;
System.arraycopy(suffix, 0, bytes, 4, suffix.length);
return bytes;
}

private static class FindOrStoreOnlyRepository implements ConversionJobRepository {
private final java.util.function.Function<ConversionJob, ConversionJobRepository.FindOrStoreResult> finder;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import static org.mockito.Mockito.when;

import java.lang.reflect.Method;
import java.nio.charset.StandardCharsets;
import java.security.Provider;
import java.security.Security;
import java.util.Set;
Expand Down Expand Up @@ -159,7 +160,7 @@ void ignoresInvalidOverrideFlagForSupportedExtension() {
DefaultDocumentValidationService validationService = new DefaultDocumentValidationService(conversionProperties);

assertDoesNotThrow(() -> validationService.validateOrThrow(
new MockMultipartFile("file", "contract.docx", "application/octet-stream", new byte[] {1}),
new MockMultipartFile("file", "contract.docx", "application/octet-stream", docxBytes("override")),
PolicyOverrideRequest.of("invalid", null, null)
));
}
Expand All @@ -171,7 +172,70 @@ void allowsSupportedExtensions() {
DefaultDocumentValidationService validationService = new DefaultDocumentValidationService(conversionProperties);

assertDoesNotThrow(() -> validationService.validateOrThrow(
new MockMultipartFile("file", "contract.docx", "application/vnd.openxmlformats-officedocument.wordprocessingml.document", new byte[] {1})
new MockMultipartFile(
"file",
"contract.docx",
"application/vnd.openxmlformats-officedocument.wordprocessingml.document",
docxBytes("contract")
)
));
}

@Test
void rejectsSupportedExtensionWhenContentSignatureDoesNotMatch() {
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.php.docx",
"application/vnd.openxmlformats-officedocument.wordprocessingml.document",
"<?php echo 'owned'; ?>".getBytes(StandardCharsets.UTF_8)
)
)
);

assertEquals("File content does not match file extension.", ex.getMessage());
}

@Test
void rejectsUnsupportedDoubleExtension() {
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.php.jpg",
"image/jpeg",
"<?php".getBytes(StandardCharsets.UTF_8)
)
)
);

assertEquals("File extension is not supported.", ex.getMessage());
}

@Test
void allowsPdfWhenContentSignatureMatches() {
ConversionProperties conversionProperties = new ConversionProperties();
conversionProperties.setBlockedExtensions(Set.of("hwp", "hwpx"));
DefaultDocumentValidationService validationService = new DefaultDocumentValidationService(conversionProperties);

assertDoesNotThrow(() -> validationService.validateOrThrow(
new MockMultipartFile(
"file",
"report.pdf",
"application/pdf",
"%PDF-1.7\n".getBytes(StandardCharsets.UTF_8)
)
));
}

Expand All @@ -191,6 +255,22 @@ void rejectsMissingExtension() {
assertEquals("File extension is required.", ex.getMessage());
}

@Test
void rejectsFilenameWithNullByte() {
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\0.pdf", "application/octet-stream", new byte[] {1})
)
);

assertEquals("File name is invalid.", ex.getMessage());
}

@Test
void rejectsBlankFilenameOrMissingName() {
ConversionProperties conversionProperties = new ConversionProperties();
Expand Down Expand Up @@ -401,4 +481,15 @@ void throwsWhenSha256DigestIsUnavailableForOverrideAuditFingerprint() {
}
}
}

private static byte[] docxBytes(String payload) {
byte[] suffix = payload.getBytes(StandardCharsets.UTF_8);
byte[] bytes = new byte[suffix.length + 4];
bytes[0] = 0x50;
bytes[1] = 0x4B;
bytes[2] = 0x03;
bytes[3] = 0x04;
System.arraycopy(suffix, 0, bytes, 4, suffix.length);
return bytes;
}
}
Loading