Skip to content
Snippets Groups Projects
Commit c07c28a0 authored by Jörg Bolay's avatar Jörg Bolay
Browse files

Merge pull request 'OZG-5412 xta-adapter: Mitigate ZipBomb attack' (#130) from...

Merge pull request 'OZG-5412 xta-adapter: Mitigate ZipBomb attack' (#130) from OZG-5412-Dataport-Mantelantrag into master

Reviewed-on: https://git.ozg-sh.de/ozgcloud-app/eingang-manager/pulls/130


Reviewed-by: default avatarOZGCloud <ozgcloud@mgm-tp.com>
parents d978fafc 505ea897
Branches
Tags
No related merge requests found
Showing with 454 additions and 30 deletions
......@@ -23,16 +23,14 @@
*/
package de.ozgcloud.eingang.semantik.enginebased.afm;
import java.util.List;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import de.ozgcloud.eingang.common.formdata.FormData;
import de.ozgcloud.eingang.common.formdata.FormDataUtils;
import de.ozgcloud.eingang.semantik.enginebased.EngineBasedSemantikAdapter;
import de.ozgcloud.eingang.semantik.enginebased.afm.intelliform.IntelliFormRepresentationAdapter;
import lombok.RequiredArgsConstructor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import java.util.List;
@Component
public class AfmEngineBasedAdapter implements EngineBasedSemantikAdapter {
......
package de.ozgcloud.eingang.xta.zip;
import java.io.FilterInputStream;
import java.io.IOException;
import java.io.InputStream;
public class LimitedInputStream extends FilterInputStream {
static final String LIMITED_EXCEEDED_MESSAGE = "Read limit exceeded";
private final long maxSize;
long bytesRead;
public LimitedInputStream(InputStream in, long maxSize) {
super(in);
this.maxSize = maxSize;
this.bytesRead = 0;
}
@Override
public int read() throws IOException {
var byteValue = super.read();
if (byteValue != -1) {
updateAndVerifyReadLimit(1);
}
return byteValue;
}
@Override
public int read(byte[] b, int off, int len) throws IOException {
return updateAndVerifyReadLimit(super.read(b, off, len));
}
private int updateAndVerifyReadLimit(int newBytesRead) throws IOException {
if (newBytesRead != -1) {
bytesRead += newBytesRead;
if (bytesRead > maxSize) {
throw new IOException(LIMITED_EXCEEDED_MESSAGE);
}
}
return newBytesRead;
}
}
......@@ -5,6 +5,7 @@ import java.io.InputStream;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
import de.ozgcloud.eingang.common.errorhandling.TechnicalException;
import lombok.Builder;
@Builder
......@@ -13,8 +14,12 @@ record ReadableZipEntry(ZipEntry zipEntry, ZipFile parentZip) {
return parentZip.getInputStream(zipEntry);
}
public Long getSize() {
return zipEntry.getSize();
public Long getPositiveSize() {
var size = zipEntry.getSize();
if (size < 0) {
throw new TechnicalException("Size of ZIP entry is unknown.");
}
return size;
}
public String getName() {
......
......@@ -20,28 +20,44 @@ import de.ozgcloud.eingang.common.formdata.IncomingFile;
// TODO Resolve code duplication with ZipAttachmentReader in semantik-adapter common.zip
// Note: In contrast to the ZipAttachmentReader, here, the zip file is not included in list of extracted files
// Further, there is no ZIP_MAX_THRESHOLD detection.
// Further, the suspicious compression ratio ZIP_MAX_THRESHOLD is evaluated on the whole zipFile, instead of individual entries
@Component
public class ZipFileExtractor {
static final double ZIP_MAX_THRESHOLD = 100;
static final int ZIP_MAX_TOTAL_SIZE = 500 * 1024 * 1024;
static final int ZIP_MAX_ENTRIES = 100;
public List<IncomingFile> extractIncomingFilesSafely(IncomingFile zipIncomingFile) {
var zipFile = zipIncomingFile.getFile();
verifySizeLimit(zipFile);
verifyLimits(zipFile);
return extractIncomingFiles(zipFile);
}
void verifySizeLimit(File zipFile) {
var totalSize = sumUncompressedEntrySizes(zipFile);
if (totalSize > ZIP_MAX_TOTAL_SIZE) {
throw new TechnicalException("Expect uncompressed size %s to be smaller than %d!".formatted(totalSize, ZIP_MAX_TOTAL_SIZE));
void verifyLimits(File zipFile) {
var uncompressedSize = sumUncompressedEntrySizes(zipFile);
verifySizeLimit(uncompressedSize);
verifyCompressionRatio(zipFile, uncompressedSize);
}
private void verifySizeLimit(long uncompressedSize) {
if (uncompressedSize > ZIP_MAX_TOTAL_SIZE) {
throw new TechnicalException("Expect uncompressed size %s to be smaller than %d!".formatted(uncompressedSize, ZIP_MAX_TOTAL_SIZE));
}
}
private void verifyCompressionRatio(File zipFile, long totalSize) {
var compressionRatio = (double) totalSize / zipFile.length();
if (compressionRatio > ZIP_MAX_THRESHOLD) {
throw new TechnicalException(
"Expect compression ratio %s to be smaller than %s! A zip bomb attack is suspected!".formatted(compressionRatio,
ZIP_MAX_THRESHOLD));
}
}
Long sumUncompressedEntrySizes(File zipFile) {
return mapZipEntries(zipFile, ReadableZipEntry::getSize)
return mapZipEntries(zipFile, ReadableZipEntry::getPositiveSize)
.stream()
.mapToLong(Long::longValue)
.sum();
......@@ -53,9 +69,9 @@ public class ZipFileExtractor {
private IncomingFile mapReadableEntryToIncomingFile(ReadableZipEntry entry) {
File file;
try (var inputStream = entry.getInputStream()) {
try (var inputStream = new LimitedInputStream(entry.getInputStream(), entry.getPositiveSize())) {
file = TempFileUtils.writeTmpFile(inputStream);
} catch (IOException e) {
} catch (IOException | de.ozgcloud.common.errorhandling.TechnicalException e) {
throw new TechnicalException("Failed reading zip file element %s!".formatted(entry.getName()), e);
}
return createIncomingFile(file, entry.zipEntry());
......
package de.ozgcloud.eingang.xta.zip;
import static de.ozgcloud.eingang.xta.zip.LimitedInputStream.*;
import static org.assertj.core.api.Assertions.*;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.Charset;
import org.apache.commons.io.IOUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import lombok.SneakyThrows;
class LimitedInputStreamTest {
private static final int READ_LIMIT = 10;
private static final String STRING_WITH_READ_LIMIT_LENGTH = "A".repeat(READ_LIMIT);
private static final String STRING_WITH_MORE_THAN_READ_LIMIT_LENGTH = "B".repeat(READ_LIMIT + 1);
private LimitedInputStream limitedInputStream;
private InputStream createStringInputSteam(String string) {
return new ByteArrayInputStream(string.getBytes());
}
@SneakyThrows
private String readInputStreamToString(InputStream inputStream) {
return IOUtils.toString(inputStream, Charset.defaultCharset());
}
@SneakyThrows
@DisplayName("should succeed if read limit is not exceeded")
@Test
void shouldSucceedIfReadLimitIsNotExceeded() {
limitedInputStream = new LimitedInputStream(createStringInputSteam(STRING_WITH_READ_LIMIT_LENGTH), READ_LIMIT);
var outputString = readInputStreamToString(limitedInputStream);
assertThat(outputString).isEqualTo(STRING_WITH_READ_LIMIT_LENGTH);
}
@DisplayName("should fail if read limit is exceeded")
@Test
void shouldFailIfReadLimitIsExceeded() {
limitedInputStream = new LimitedInputStream(createStringInputSteam(STRING_WITH_MORE_THAN_READ_LIMIT_LENGTH), READ_LIMIT);
assertThatThrownBy(() -> readInputStreamToString(limitedInputStream))
.isInstanceOf(IOException.class)
.hasMessage(LIMITED_EXCEEDED_MESSAGE);
}
@DisplayName("read")
@Nested
class TestRead {
@SneakyThrows
@DisplayName("should return")
@Test
void shouldReturn() {
limitedInputStream = createLimitedInputStream();
var result = limitedInputStream.read();
assertThat(result).isEqualTo(STRING_WITH_READ_LIMIT_LENGTH.getBytes()[0]);
}
@SneakyThrows
@DisplayName("should advance bytesRead")
@Test
void shouldAdvanceBytesRead() {
limitedInputStream = createLimitedInputStream();
limitedInputStream.bytesRead = READ_LIMIT - 1;
limitedInputStream.read();
assertThat(limitedInputStream.bytesRead).isEqualTo(READ_LIMIT);
}
@DisplayName("should throw if exceeded")
@Test
void shouldThrowIfExceeded() {
limitedInputStream = createLimitedInputStreamWithExceeding();
limitedInputStream.bytesRead = READ_LIMIT;
assertThatThrownBy(() -> limitedInputStream.read()).isInstanceOf(IOException.class);
}
}
@DisplayName("read into buffer")
@Nested
class TestReadIntoBuffer {
private byte[] buffer;
@BeforeEach
void mock() {
buffer = new byte[READ_LIMIT];
}
@SneakyThrows
@DisplayName("should return")
@Test
void shouldReturn() {
limitedInputStream = createLimitedInputStream();
var result = limitedInputStream.read(buffer);
assertThat(result).isEqualTo(READ_LIMIT);
}
@SneakyThrows
@DisplayName("should advance bytesRead")
@Test
void shouldAdvanceBytesRead() {
limitedInputStream = createLimitedInputStream();
limitedInputStream.read(buffer);
assertThat(limitedInputStream.bytesRead).isEqualTo(READ_LIMIT);
}
@DisplayName("should throw if exceeded")
@Test
void shouldThrowIfExceeded() {
limitedInputStream = createLimitedInputStreamWithExceeding();
limitedInputStream.bytesRead = 1;
assertThatThrownBy(() -> limitedInputStream.read(buffer)).isInstanceOf(IOException.class);
}
}
private LimitedInputStream createLimitedInputStream() {
return new LimitedInputStream(createStringInputSteam(STRING_WITH_READ_LIMIT_LENGTH), READ_LIMIT);
}
private LimitedInputStream createLimitedInputStreamWithExceeding() {
return new LimitedInputStream(createStringInputSteam(STRING_WITH_MORE_THAN_READ_LIMIT_LENGTH), READ_LIMIT);
}
}
package de.ozgcloud.eingang.xta.zip;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;
import java.io.InputStream;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import de.ozgcloud.eingang.common.errorhandling.TechnicalException;
import lombok.SneakyThrows;
class ReadableZipEntryTest {
@Mock
ZipEntry zipEntry;
@Mock
ZipFile zipFile;
private ReadableZipEntry readableZipEntry;
@BeforeEach
void mock() {
readableZipEntry = ReadableZipEntry.builder()
.zipEntry(zipEntry)
.parentZip(zipFile)
.build();
}
@DisplayName("get input stream")
@Nested
class TestGetInputStream {
@Mock
private InputStream inputStream;
@SneakyThrows
@DisplayName("should return input stream")
@Test
void shouldReturnInputStream() {
when(zipFile.getInputStream(zipEntry)).thenReturn(inputStream);
var inputStreamResult = readableZipEntry.getInputStream();
assertThat(inputStreamResult).isEqualTo(inputStream);
}
}
@DisplayName("get positive size")
@Nested
class TestGetPositiveSize {
@DisplayName("should return size")
@Test
void shouldReturnSize() {
var size = 123L;
when(zipEntry.getSize()).thenReturn(size);
var sizeResult = readableZipEntry.getPositiveSize();
assertThat(sizeResult).isEqualTo(size);
}
@DisplayName("should throw if size is negative")
@Test
void shouldThrowIfSizeIsNegative() {
var size = -1L;
when(zipEntry.getSize()).thenReturn(size);
assertThatThrownBy(() -> readableZipEntry.getPositiveSize()).isInstanceOf(TechnicalException.class);
}
}
}
package de.ozgcloud.eingang.xta.zip;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.util.List;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;
import org.apache.commons.io.IOUtils;
import de.ozgcloud.common.binaryfile.TempFileUtils;
import lombok.Builder;
import lombok.Getter;
public class TestZipFileFactory {
private static final String EXPANDED_ENTRY_NAME = "bomb.txt";
@Builder
@Getter
public static class TestZipEntry {
......@@ -33,4 +38,76 @@ public class TestZipFileFactory {
throw new RuntimeException("Failed to create temporary zip file", e);
}
}
public static File createTempZipBomb(int maxTotalSize) {
return overwriteFileWithZipEntrySize(
createTempZipWithSingleEntry(maxTotalSize * 2),
maxTotalSize
);
}
private static File createTempZipWithSingleEntry(int entrySize) {
var file = TempFileUtils.createTmpFile().toFile();
try (var zipOutputStream = new ZipOutputStream(new FileOutputStream(file))) {
var entry = new ZipEntry(EXPANDED_ENTRY_NAME);
var content = "A".repeat(entrySize).getBytes();
zipOutputStream.putNextEntry(entry);
zipOutputStream.write(content);
zipOutputStream.closeEntry();
} catch (IOException e) {
throw new RuntimeException("Failed to create temporary zip file", e);
}
return file;
}
private static File overwriteFileWithZipEntrySize(File file, int newSize) {
try {
var zipFileBytes = IOUtils.toByteArray(new FileInputStream(file));
overwriteZipEntrySize(zipFileBytes, EXPANDED_ENTRY_NAME, newSize);
// Write the adjusted ZIP content back to the file
try (var fos = new FileOutputStream(file)) {
fos.write(zipFileBytes);
}
} catch (IOException e) {
throw new RuntimeException("Failed to adjust size header of zip file", e);
}
return file;
}
private static void overwriteZipEntrySize(byte[] zipFileBytes, String entryName, int newSize) throws IOException {
// Modify the uncompressed size entry size in the central directory structure (which is located at the end)
// Zip structure spec: https://www.iana.org/assignments/media-types/application/zip
var entryNameBytes = entryName.getBytes();
var lastIndexOfEntryName = findLastStartIndex(zipFileBytes, entryNameBytes);
if (lastIndexOfEntryName == -1) {
throw new IOException("ZIP entry not found: " + entryName);
}
var uncompressedSizeFieldStartOffset = lastIndexOfEntryName - (4 * 2 + 5 * 2 + 4);
writeIntToByteArray(newSize, zipFileBytes, uncompressedSizeFieldStartOffset);
}
private static void writeIntToByteArray(int value, byte[] array, int offset) {
array[offset] = (byte) (value & 0xFF);
array[offset + 1] = (byte) ((value >> 8) & 0xFF);
array[offset + 2] = (byte) ((value >> 16) & 0xFF);
array[offset + 3] = (byte) ((value >> 24) & 0xFF);
}
private static int findLastStartIndex(byte[] haystack, byte[] needle) {
var matchOffset = 0;
for (var i = haystack.length - 1; i >= needle.length; i--) {
if (haystack[i] == needle[needle.length - 1 - matchOffset]) {
if (++matchOffset == needle.length) {
return i;
}
} else {
matchOffset = 0;
}
}
return -1;
}
}
package de.ozgcloud.eingang.xta.zip;
import static de.ozgcloud.eingang.xta.zip.TestZipFileFactory.*;
import static de.ozgcloud.eingang.xta.zip.ZipFileExtractor.*;
import static org.assertj.core.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*;
......@@ -26,6 +27,7 @@ import org.springframework.util.MimeTypeUtils;
import de.ozgcloud.eingang.common.errorhandling.TechnicalException;
import de.ozgcloud.eingang.common.formdata.IncomingFile;
import lombok.SneakyThrows;
class ZipFileExtractorTest {
......@@ -52,16 +54,17 @@ class ZipFileExtractorTest {
outIncomingFiles = List.of(outIncomingFile);
when(incomingZipFile.getFile()).thenReturn(zipFile);
doNothing().when(extractor).verifySizeLimit(zipFile);
doNothing().when(extractor).verifyLimits(zipFile);
doReturn(outIncomingFiles).when(extractor).extractIncomingFiles(zipFile);
}
@DisplayName("should call verify size limit")
@DisplayName("should call verify limits")
@Test
void shouldCallVerifySizeLimit() {
extractor.extractIncomingFilesSafely(incomingZipFile);
verify(extractor).verifySizeLimit(zipFile);
verify(extractor).verifyLimits(zipFile);
}
@DisplayName("should return")
......@@ -73,26 +76,37 @@ class ZipFileExtractorTest {
}
}
@DisplayName("verify size limit")
@DisplayName("verify limits")
@Nested
class TestVerifySizeLimit {
class TestVerifyLimits {
@Mock
File zipFile;
@DisplayName("should return")
@Test
void shouldReturn() {
doReturn((long) ZipFileExtractor.ZIP_MAX_TOTAL_SIZE).when(extractor).sumUncompressedEntrySizes(zipFile);
when(zipFile.length()).thenReturn((long) ZIP_MAX_TOTAL_SIZE / 2);
doReturn((long) ZIP_MAX_TOTAL_SIZE).when(extractor).sumUncompressedEntrySizes(zipFile);
extractor.verifySizeLimit(zipFile);
extractor.verifyLimits(zipFile);
}
@DisplayName("should throw if limit exceeded")
@DisplayName("should throw if size limit exceeded")
@Test
void shouldThrowIfLimitExceeded() {
doReturn((long) ZipFileExtractor.ZIP_MAX_TOTAL_SIZE + 1).when(extractor).sumUncompressedEntrySizes(zipFile);
void shouldThrowIfSizeLimitExceeded() {
doReturn((long) ZIP_MAX_TOTAL_SIZE + 1).when(extractor).sumUncompressedEntrySizes(zipFile);
assertThatThrownBy(() -> extractor.verifySizeLimit(zipFile))
assertThatThrownBy(() -> extractor.verifyLimits(zipFile))
.isInstanceOf(TechnicalException.class);
}
@DisplayName("should throw if ratio exceeded")
@Test
void shouldThrowIfRatioExceeded() {
when(zipFile.length()).thenReturn(1L);
doReturn((long) ZIP_MAX_THRESHOLD + 1).when(extractor).sumUncompressedEntrySizes(zipFile);
assertThatThrownBy(() -> extractor.verifyLimits(zipFile))
.isInstanceOf(TechnicalException.class);
}
}
......@@ -154,6 +168,57 @@ class ZipFileExtractorTest {
}
}
@DisplayName("extract zip bomb")
@Nested
class TestExtractZipBomb {
private static final int SMALLER_MAX_ZIP_FILE_SIZE = 2 * 1024;
@DisplayName("should throw with too many entries")
@Test
void shouldThrow() {
var zipFile = createIncomingFile(createTempZipFile(IntStream.range(0, ZIP_MAX_ENTRIES + 1).mapToObj(i -> TestZipEntry.builder()
.name("file%d.txt".formatted(i))
.content(toBytes("A".repeat(2)))
.build()
).toList()));
assertThatThrownBy(() -> extractor.extractIncomingFilesSafely(zipFile))
.isInstanceOf(TechnicalException.class);
}
@SneakyThrows
@DisplayName("should throw with fake getSize")
@Test
void shouldThrowWithFakeGetSize() {
doReturn(SMALLER_MAX_ZIP_FILE_SIZE).when(extractor).getZipMaxTotalSize();
var zipBomb = createIncomingFile(createTempZipBomb(SMALLER_MAX_ZIP_FILE_SIZE));
assertThatThrownBy(() -> extractor.extractIncomingFilesSafely(zipBomb))
.isInstanceOf(TechnicalException.class)
.hasRootCauseMessage(LimitedInputStream.LIMITED_EXCEEDED_MESSAGE);
}
@DisplayName("should throw with too large size")
@Test
void shouldThrowWithTooLargeSize() {
doReturn(SMALLER_MAX_ZIP_FILE_SIZE).when(extractor).getZipMaxTotalSize();
var zipFile = createIncomingFile(createTempZipFile(List.of(TestZipEntry.builder()
.name("toolargefile.txt")
.content(toBytes("A".repeat(SMALLER_MAX_ZIP_FILE_SIZE + 1)))
.build()
)));
assertThatThrownBy(() -> extractor.extractIncomingFilesSafely(zipFile))
.isInstanceOf(TechnicalException.class);
}
private IncomingFile createIncomingFile(File file) {
return IncomingFile.builder()
.file(file)
.build();
}
}
@DisplayName("create incoming file")
@Nested
class TestCreateIncomingFile {
......@@ -271,7 +336,7 @@ class ZipFileExtractorTest {
@DisplayName("should throw if max entries exceeded")
@Test
void shouldThrowIfMaxEntriesExceeded() {
var zipWithTooManyEntries = createTempZipFile(IntStream.range(0, ZipFileExtractor.ZIP_MAX_ENTRIES + 1)
var zipWithTooManyEntries = createTempZipFile(IntStream.range(0, ZIP_MAX_ENTRIES + 1)
.mapToObj(i -> TestZipEntry.builder()
.name("test%d.txt".formatted(i))
.content(toBytes("test file %d".formatted(i)))
......@@ -285,7 +350,7 @@ class ZipFileExtractorTest {
@DisplayName("should map with mapping function")
@Test
void shouldMapWithMappingFunction() {
var expectedNumberList = IntStream.range(0, ZipFileExtractor.ZIP_MAX_ENTRIES).boxed().toList();
var expectedNumberList = IntStream.range(0, ZIP_MAX_ENTRIES).boxed().toList();
var zipFile = createTempZipFile(expectedNumberList.stream()
.map(i -> TestZipEntry.builder()
.name("%d".formatted(i))
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment