RED-10731: Zip Bombing (~10.000 files in one zip within file limit) should be detected and aborted
This commit is contained in:
maverickstuder
parent
fdfdba550e
commit
4746d50627
@ -23,12 +23,11 @@ import org.springframework.web.bind.annotation.RequestPart;
|
||||
import org.springframework.web.bind.annotation.RestController;
|
||||
import org.springframework.web.multipart.MultipartFile;
|
||||
|
||||
import com.iqser.red.service.persistence.management.v1.processor.service.FileFormatValidationService;
|
||||
import com.iqser.red.service.persistence.management.v1.processor.exception.NotAllowedException;
|
||||
import com.knecon.fforesight.keycloakcommons.security.KeycloakSecurity;
|
||||
import com.iqser.red.service.pdftron.redaction.v1.api.model.ByteContentDocument;
|
||||
import com.iqser.red.service.persistence.management.v1.processor.exception.BadRequestException;
|
||||
import com.iqser.red.service.persistence.management.v1.processor.exception.NotAllowedException;
|
||||
import com.iqser.red.service.persistence.management.v1.processor.service.AccessControlService;
|
||||
import com.iqser.red.service.persistence.management.v1.processor.service.FileFormatValidationService;
|
||||
import com.iqser.red.service.persistence.management.v1.processor.service.ReanalysisService;
|
||||
import com.iqser.red.service.persistence.management.v1.processor.service.UploadService;
|
||||
import com.iqser.red.service.persistence.management.v1.processor.service.persistence.AuditPersistenceService;
|
||||
@ -37,6 +36,7 @@ import com.iqser.red.service.persistence.service.v1.api.external.resource.Upload
|
||||
import com.iqser.red.service.persistence.service.v1.api.shared.model.AuditCategory;
|
||||
import com.iqser.red.service.persistence.service.v1.api.shared.model.FileUploadResult;
|
||||
import com.iqser.red.service.persistence.service.v1.api.shared.model.audit.AuditRequest;
|
||||
import com.knecon.fforesight.keycloakcommons.security.KeycloakSecurity;
|
||||
import com.knecon.fforesight.tenantcommons.TenantContext;
|
||||
|
||||
import feign.FeignException;
|
||||
@ -53,9 +53,9 @@ import lombok.extern.slf4j.Slf4j;
|
||||
@SuppressWarnings("PMD")
|
||||
public class UploadController implements UploadResource {
|
||||
|
||||
private static final int THRESHOLD_ENTRIES = 10000;
|
||||
private static final int THRESHOLD_SIZE = 1000000000; // 1 GB
|
||||
private static final double THRESHOLD_RATIO = 10;
|
||||
private static final int THRESHOLD_ENTRIES = 10000; // Maximum number of files allowed
|
||||
private static final int THRESHOLD_SIZE = 1000000000; // 1 GB total unzipped data
|
||||
private static final double THRESHOLD_RATIO = 10; // Max allowed compression ratio
|
||||
|
||||
private final UploadService uploadService;
|
||||
private final ReanalysisService reanalysisService;
|
||||
@ -72,31 +72,25 @@ public class UploadController implements UploadResource {
|
||||
@Parameter(name = DISABLE_AUTOMATIC_ANALYSIS_PARAM, description = "Disables automatic redaction for the uploaded file, imports only imported redactions") @RequestParam(value = DISABLE_AUTOMATIC_ANALYSIS_PARAM, required = false, defaultValue = "false") boolean disableAutomaticAnalysis) {
|
||||
|
||||
accessControlService.checkAccessPermissionsToDossier(dossierId);
|
||||
if (file.getOriginalFilename() == null) {
|
||||
|
||||
String originalFilename = file.getOriginalFilename();
|
||||
if (originalFilename == null) {
|
||||
throw new BadRequestException("Could not upload file, no filename provided.");
|
||||
}
|
||||
|
||||
var extension = getExtension(file.getOriginalFilename());
|
||||
|
||||
String extension = getExtension(originalFilename);
|
||||
try {
|
||||
switch (extension) {
|
||||
case "zip":
|
||||
return handleZip(dossierId, file.getBytes(), keepManualRedactions, disableAutomaticAnalysis);
|
||||
case "csv":
|
||||
return uploadService.importCsv(dossierId, file.getBytes());
|
||||
default:
|
||||
if (!fileFormatValidationService.getAllFileFormats().contains(extension)) {
|
||||
throw new BadRequestException("Invalid file uploaded");
|
||||
}
|
||||
if (!fileFormatValidationService.getValidFileFormatsForTenant(TenantContext.getTenantId()).contains(extension)) {
|
||||
throw new NotAllowedException("Insufficient permissions");
|
||||
}
|
||||
return uploadService.processSingleFile(dossierId, file.getOriginalFilename(), file.getBytes(), keepManualRedactions, disableAutomaticAnalysis);
|
||||
}
|
||||
return switch (extension) {
|
||||
case "zip" -> handleZip(dossierId, file.getBytes(), keepManualRedactions, disableAutomaticAnalysis);
|
||||
case "csv" -> uploadService.importCsv(dossierId, file.getBytes());
|
||||
default -> {
|
||||
validateExtensionOrThrow(extension);
|
||||
yield uploadService.processSingleFile(dossierId, originalFilename, file.getBytes(), keepManualRedactions, disableAutomaticAnalysis);
|
||||
}
|
||||
};
|
||||
} catch (IOException e) {
|
||||
throw new BadRequestException(e.getMessage(), e);
|
||||
throw new BadRequestException("Failed to process file: " + e.getMessage(), e);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
|
||||
@ -111,7 +105,6 @@ public class UploadController implements UploadResource {
|
||||
accessControlService.verifyUserIsReviewerOrApprover(dossierId, fileId);
|
||||
|
||||
try {
|
||||
|
||||
reanalysisService.importRedactions(ByteContentDocument.builder().dossierId(dossierId).fileId(fileId).document(file.getBytes()).pages(pageInclusionRequest).build());
|
||||
|
||||
auditPersistenceService.audit(AuditRequest.builder()
|
||||
@ -122,84 +115,116 @@ public class UploadController implements UploadResource {
|
||||
.details(Map.of("dossierId", dossierId))
|
||||
.build());
|
||||
} catch (IOException e) {
|
||||
throw new BadRequestException(e.getMessage(), e);
|
||||
throw new BadRequestException("Failed to import redactions: " + e.getMessage(), e);
|
||||
} catch (FeignException e) {
|
||||
throw processFeignException(e);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
private String getExtension(String fileName) {
|
||||
private void validateExtensionOrThrow(String extension) {
|
||||
|
||||
return fileName.substring(fileName.lastIndexOf(".") + 1).toLowerCase(Locale.ROOT);
|
||||
if (!fileFormatValidationService.getAllFileFormats().contains(extension)) {
|
||||
throw new BadRequestException("Invalid file uploaded (unrecognized extension).");
|
||||
}
|
||||
if (!fileFormatValidationService.getValidFileFormatsForTenant(TenantContext.getTenantId()).contains(extension)) {
|
||||
throw new NotAllowedException("Insufficient permissions for this file type.");
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* 1. Write the uploaded content to a temp ZIP file
|
||||
* 2. Check the number of entries and reject if too big or if symlinks found
|
||||
* 3. Unzip and process each file, while checking size and ratio.
|
||||
*/
|
||||
private FileUploadResult handleZip(String dossierId, byte[] fileContent, boolean keepManualRedactions, boolean disableAutomaticAnalysis) throws IOException {
|
||||
|
||||
File tempFile = FileUtils.createTempFile(UUID.randomUUID().toString(), ".zip");
|
||||
try (var fileOutputStream = new FileOutputStream(tempFile)) {
|
||||
IOUtils.write(fileContent, fileOutputStream);
|
||||
File tempZip = FileUtils.createTempFile(UUID.randomUUID().toString(), ".zip");
|
||||
try (FileOutputStream fos = new FileOutputStream(tempZip)) {
|
||||
IOUtils.write(fileContent, fos);
|
||||
}
|
||||
|
||||
try {
|
||||
checkForSymlinks(tempFile);
|
||||
validateZipEntries(tempZip);
|
||||
|
||||
var zipData = unzip(tempFile, dossierId, keepManualRedactions, disableAutomaticAnalysis);
|
||||
try {
|
||||
ZipData zipData = processZipContents(tempZip, dossierId, keepManualRedactions, disableAutomaticAnalysis);
|
||||
|
||||
if (zipData.csvBytes != null) {
|
||||
try {
|
||||
var importResult = uploadService.importCsv(dossierId, zipData.csvBytes);
|
||||
zipData.fileUploadResult.getProcessedAttributes().addAll(importResult.getProcessedAttributes());
|
||||
zipData.fileUploadResult.getProcessedFileIds().addAll(importResult.getProcessedFileIds());
|
||||
FileUploadResult csvResult = uploadService.importCsv(dossierId, zipData.csvBytes);
|
||||
zipData.fileUploadResult.getProcessedAttributes().addAll(csvResult.getProcessedAttributes());
|
||||
zipData.fileUploadResult.getProcessedFileIds().addAll(csvResult.getProcessedFileIds());
|
||||
} catch (Exception e) {
|
||||
log.debug("CSV file inside ZIP failed", e);
|
||||
// TODO return un-processed files to client
|
||||
log.debug("CSV file inside ZIP failed to import", e);
|
||||
}
|
||||
} else if (zipData.fileUploadResult.getFileIds().isEmpty()) {
|
||||
if (zipData.containedUnpermittedFiles) {
|
||||
throw new NotAllowedException("Zip file contains unpermitted files");
|
||||
throw new NotAllowedException("Zip file contains unpermitted files.");
|
||||
} else {
|
||||
throw new BadRequestException("Only unsupported files in zip file");
|
||||
throw new BadRequestException("Only unsupported files in the ZIP.");
|
||||
}
|
||||
}
|
||||
|
||||
return zipData.fileUploadResult;
|
||||
|
||||
} finally {
|
||||
boolean isDeleted = tempFile.delete();
|
||||
if (!isDeleted) {
|
||||
log.warn("tempFile could not be deleted");
|
||||
|
||||
if (!tempZip.delete()) {
|
||||
log.warn("Could not delete temporary ZIP file: {}", tempZip);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
private void checkForSymlinks(File tempFile) throws IOException {
|
||||
private void validateZipEntries(File tempZip) throws IOException {
|
||||
|
||||
try (FileInputStream fis = new FileInputStream(tempZip); ZipFile zipFile = new ZipFile(fis.getChannel())) {
|
||||
|
||||
int count = 0;
|
||||
var entries = zipFile.getEntries();
|
||||
while (entries.hasMoreElements()) {
|
||||
ZipArchiveEntry ze = entries.nextElement();
|
||||
|
||||
try (var fis = new FileInputStream(tempFile); var zipFile = new ZipFile(fis.getChannel())) {
|
||||
for (var entryEnum = zipFile.getEntries(); entryEnum.hasMoreElements(); ) {
|
||||
var ze = entryEnum.nextElement();
|
||||
if (ze.isUnixSymlink()) {
|
||||
throw new BadRequestException("ZIP-files with symlinks are not allowed");
|
||||
throw new BadRequestException("ZIP-files with symlinks are not allowed.");
|
||||
}
|
||||
|
||||
if (!ze.isDirectory() && !ze.getName().startsWith(".")) {
|
||||
count++;
|
||||
if (count > THRESHOLD_ENTRIES) {
|
||||
throw new BadRequestException("ZIP-Bomb detected: too many entries.");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
private ZipData unzip(File tempFile, String dossierId, boolean keepManualRedactions, boolean disableAutomaticAnalysis) throws IOException {
|
||||
private ZipData processZipContents(File tempZip, String dossierId, boolean keepManualRedactions, boolean disableAutomaticAnalysis) throws IOException {
|
||||
|
||||
var zipData = new ZipData();
|
||||
ZipData zipData = new ZipData();
|
||||
|
||||
try (var fis = new FileInputStream(tempFile); var zipFile = new ZipFile(fis.getChannel())) {
|
||||
try (FileInputStream fis = new FileInputStream(tempZip); ZipFile zipFile = new ZipFile(fis.getChannel())) {
|
||||
|
||||
for (var entryEnum = zipFile.getEntries(); entryEnum.hasMoreElements(); ) {
|
||||
var ze = entryEnum.nextElement();
|
||||
zipData.totalEntryArchive++;
|
||||
var entries = zipFile.getEntries();
|
||||
while (entries.hasMoreElements()) {
|
||||
ZipArchiveEntry entry = entries.nextElement();
|
||||
|
||||
if (!ze.isDirectory()) {
|
||||
processFileZipEntry(ze, zipFile, dossierId, keepManualRedactions, zipData, disableAutomaticAnalysis);
|
||||
if (entry.isDirectory() || entry.getName().startsWith(".")) {
|
||||
continue;
|
||||
}
|
||||
|
||||
byte[] entryBytes = readEntryWithRatioCheck(entry, zipFile);
|
||||
zipData.totalSizeArchive += entryBytes.length;
|
||||
if (zipData.totalSizeArchive > THRESHOLD_SIZE) {
|
||||
throw new BadRequestException("ZIP-Bomb detected (exceeds total size limit).");
|
||||
}
|
||||
|
||||
String extension = getExtension(entry.getName());
|
||||
if ("csv".equalsIgnoreCase(extension)) {
|
||||
zipData.csvBytes = entryBytes;
|
||||
} else {
|
||||
handleRegularFile(dossierId, entryBytes, extension, extractFileName(entry.getName()), zipData, keepManualRedactions, disableAutomaticAnalysis);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -207,73 +232,70 @@ public class UploadController implements UploadResource {
|
||||
}
|
||||
|
||||
|
||||
private void processFileZipEntry(ZipArchiveEntry ze, ZipFile zipFile, String dossierId, boolean keepManualRedactions, ZipData zipData, boolean disableAutomaticAnalysis) throws IOException {
|
||||
private byte[] readEntryWithRatioCheck(ZipArchiveEntry entry, ZipFile zipFile) throws IOException {
|
||||
|
||||
var extension = getExtension(ze.getName());
|
||||
long compressedSize = entry.getCompressedSize() > 0 ? entry.getCompressedSize() : 1;
|
||||
try (var is = zipFile.getInputStream(entry); var bos = new ByteArrayOutputStream()) {
|
||||
|
||||
final String fileName;
|
||||
if (ze.getName().lastIndexOf("/") >= 0) {
|
||||
fileName = ze.getName().substring(ze.getName().lastIndexOf("/") + 1);
|
||||
} else {
|
||||
fileName = ze.getName();
|
||||
}
|
||||
byte[] buffer = new byte[4096];
|
||||
int bytesRead;
|
||||
int totalUncompressed = 0;
|
||||
|
||||
if (fileName.startsWith(".")) {
|
||||
return;
|
||||
}
|
||||
while ((bytesRead = is.read(buffer)) != -1) {
|
||||
bos.write(buffer, 0, bytesRead);
|
||||
totalUncompressed += bytesRead;
|
||||
|
||||
var entryAsBytes = readCurrentZipEntry(ze, zipFile);
|
||||
zipData.totalSizeArchive += entryAsBytes.length;
|
||||
|
||||
// 1. the uncompressed data size is too much for the application resource capacity
|
||||
// 2. too many entries in the archive can lead to inode exhaustion of the file-system
|
||||
if (zipData.totalSizeArchive > THRESHOLD_SIZE || zipData.totalEntryArchive > THRESHOLD_ENTRIES) {
|
||||
throw new BadRequestException("ZIP-Bomb detected.");
|
||||
}
|
||||
|
||||
if ("csv".equals(extension)) {
|
||||
zipData.csvBytes = entryAsBytes;
|
||||
} else if (fileFormatValidationService.getAllFileFormats().contains(extension)) {
|
||||
|
||||
if (!fileFormatValidationService.getValidFileFormatsForTenant(TenantContext.getTenantId()).contains(extension)) {
|
||||
zipData.containedUnpermittedFiles = true;
|
||||
return;
|
||||
}
|
||||
zipData.containedUnpermittedFiles = false;
|
||||
|
||||
try {
|
||||
var result = uploadService.processSingleFile(dossierId, fileName, entryAsBytes, keepManualRedactions, disableAutomaticAnalysis);
|
||||
zipData.fileUploadResult.getFileIds().addAll(result.getFileIds());
|
||||
} catch (Exception e) {
|
||||
log.debug("PDF File inside ZIP failed", e);
|
||||
// TODO return un-processed files to client
|
||||
double ratio = (double) totalUncompressed / compressedSize;
|
||||
if (ratio > THRESHOLD_RATIO) {
|
||||
throw new BadRequestException("ZIP-Bomb detected (compression ratio too high).");
|
||||
}
|
||||
}
|
||||
return bos.toByteArray();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
private byte[] readCurrentZipEntry(ZipArchiveEntry ze, ZipFile zipFile) throws IOException {
|
||||
private void handleRegularFile(String dossierId,
|
||||
byte[] fileBytes,
|
||||
String extension,
|
||||
String fileName,
|
||||
ZipData zipData,
|
||||
boolean keepManualRedactions,
|
||||
boolean disableAutomaticAnalysis) {
|
||||
|
||||
var bos = new ByteArrayOutputStream();
|
||||
|
||||
try (var entryStream = zipFile.getInputStream(ze)) {
|
||||
var buffer = new byte[2048];
|
||||
var nBytes = 0;
|
||||
int totalSizeEntry = 0;
|
||||
|
||||
while ((nBytes = entryStream.read(buffer)) > 0) {
|
||||
bos.write(buffer, 0, nBytes);
|
||||
totalSizeEntry += nBytes;
|
||||
|
||||
double compressionRatio = (float) totalSizeEntry / ze.getCompressedSize();
|
||||
if (compressionRatio > THRESHOLD_RATIO) {
|
||||
// ratio between compressed and uncompressed data is highly suspicious, looks like a Zip Bomb Attack
|
||||
throw new BadRequestException("ZIP-Bomb detected.");
|
||||
}
|
||||
}
|
||||
if (!fileFormatValidationService.getAllFileFormats().contains(extension)) {
|
||||
zipData.containedUnpermittedFiles = false;
|
||||
return;
|
||||
}
|
||||
|
||||
return bos.toByteArray();
|
||||
if (!fileFormatValidationService.getValidFileFormatsForTenant(TenantContext.getTenantId()).contains(extension)) {
|
||||
zipData.containedUnpermittedFiles = true;
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
FileUploadResult result = uploadService.processSingleFile(dossierId, fileName, fileBytes, keepManualRedactions, disableAutomaticAnalysis);
|
||||
zipData.fileUploadResult.getFileIds().addAll(result.getFileIds());
|
||||
} catch (Exception e) {
|
||||
log.debug("Failed to process file '{}' in ZIP: {}", fileName, e.getMessage(), e);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
private String extractFileName(String path) {
|
||||
|
||||
int idx = path.lastIndexOf('/');
|
||||
return (idx >= 0) ? path.substring(idx + 1) : path;
|
||||
}
|
||||
|
||||
|
||||
private String getExtension(String fileName) {
|
||||
|
||||
int idx = fileName.lastIndexOf('.');
|
||||
if (idx < 0) {
|
||||
return "";
|
||||
}
|
||||
return fileName.substring(idx + 1).toLowerCase(Locale.ROOT);
|
||||
}
|
||||
|
||||
|
||||
@ -282,7 +304,6 @@ public class UploadController implements UploadResource {
|
||||
|
||||
byte[] csvBytes;
|
||||
int totalSizeArchive;
|
||||
int totalEntryArchive;
|
||||
FileUploadResult fileUploadResult = new FileUploadResult();
|
||||
boolean containedUnpermittedFiles;
|
||||
|
||||
|
||||
@ -0,0 +1,79 @@
|
||||
package com.iqser.red.service.peristence.v1.server.integration.tests;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
import org.apache.commons.io.IOUtils;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.core.io.ClassPathResource;
|
||||
import org.springframework.mock.web.MockMultipartFile;
|
||||
import org.springframework.test.context.TestPropertySource;
|
||||
|
||||
import com.iqser.red.service.peristence.v1.server.integration.client.FileClient;
|
||||
import com.iqser.red.service.peristence.v1.server.integration.client.UploadClient;
|
||||
import com.iqser.red.service.peristence.v1.server.integration.service.DossierTesterAndProvider;
|
||||
import com.iqser.red.service.peristence.v1.server.integration.utils.AbstractPersistenceServerServiceTest;
|
||||
|
||||
import feign.FeignException;
|
||||
import lombok.SneakyThrows;
|
||||
|
||||
@TestPropertySource(properties = {
|
||||
"spring.servlet.multipart.max-file-size=50MB",
|
||||
"spring.servlet.multipart.max-request-size=50MB"
|
||||
})
|
||||
public class ZipFileUploadTest extends AbstractPersistenceServerServiceTest {
|
||||
|
||||
@Autowired
|
||||
private DossierTesterAndProvider dossierTesterAndProvider;
|
||||
|
||||
@Autowired
|
||||
private UploadClient uploadClient;
|
||||
|
||||
@Autowired
|
||||
private FileClient fileClient;
|
||||
|
||||
@SneakyThrows
|
||||
@Test
|
||||
void testZipUploadWithEntryCountCheck() {
|
||||
|
||||
var dossier = dossierTesterAndProvider.provideTestDossier();
|
||||
|
||||
var smallZipResource = new ClassPathResource("files/zip/ArchiveWithManyFiles.zip");
|
||||
var smallZip = new MockMultipartFile(
|
||||
"ArchiveWithManyFiles.zip",
|
||||
"ArchiveWithManyFiles.zip",
|
||||
"application/zip",
|
||||
IOUtils.toByteArray(smallZipResource.getInputStream())
|
||||
);
|
||||
|
||||
var uploadResult = uploadClient.upload(smallZip, dossier.getId(), false, false);
|
||||
|
||||
assertNotNull(uploadResult, "Upload result for small zip should not be null.");
|
||||
assertEquals(9993, uploadResult.getFileIds().size());
|
||||
|
||||
var largeZipResource = new ClassPathResource("files/zip/ArchiveWithTooManyFiles.zip");
|
||||
var largeZip = new MockMultipartFile(
|
||||
"ArchiveWithTooManyFiles.zip",
|
||||
"ArchiveWithTooManyFiles.zip",
|
||||
"application/zip",
|
||||
IOUtils.toByteArray(largeZipResource.getInputStream())
|
||||
);
|
||||
|
||||
FeignException ex = assertThrows(
|
||||
FeignException.class,
|
||||
() -> uploadClient.upload(largeZip, dossier.getId(), false, false),
|
||||
"Uploading a zip with more than 10000 entries should throw a FeignException."
|
||||
);
|
||||
|
||||
assertEquals(400, ex.status(), "Expected HTTP 400 (Bad Request) for a ZIP bomb scenario");
|
||||
assertTrue(ex.getMessage().contains("ZIP-Bomb detected"),
|
||||
"Exception message should contain 'ZIP-Bomb detected' or similar.");
|
||||
|
||||
var filesInDossier = fileClient.getDossierStatus(dossier.getId());
|
||||
assertEquals(9993, filesInDossier.size());
|
||||
}
|
||||
}
|
||||
|
||||
Binary file not shown.
Binary file not shown.
Loading…
x
Reference in New Issue
Block a user