Skip to content
Snippets Groups Projects
Commit 5f879d4b authored by Jan Zickermann's avatar Jan Zickermann
Browse files

OZG-5412 xta-adapter: Mitigate ZipBomb attack

parent fa62a17c
No related branches found
No related tags found
No related merge requests found
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;
}
}
...@@ -20,7 +20,7 @@ import de.ozgcloud.eingang.common.formdata.IncomingFile; ...@@ -20,7 +20,7 @@ import de.ozgcloud.eingang.common.formdata.IncomingFile;
// TODO Resolve code duplication with ZipAttachmentReader in semantik-adapter common.zip // 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 // 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, there is no ZIP_MAX_THRESHOLD detection, instead the reader will abort if the inputstream exceeds zipFile.getSize().
@Component @Component
public class ZipFileExtractor { public class ZipFileExtractor {
...@@ -35,11 +35,15 @@ public class ZipFileExtractor { ...@@ -35,11 +35,15 @@ public class ZipFileExtractor {
void verifySizeLimit(File zipFile) { void verifySizeLimit(File zipFile) {
var totalSize = sumUncompressedEntrySizes(zipFile); var totalSize = sumUncompressedEntrySizes(zipFile);
if (totalSize > ZIP_MAX_TOTAL_SIZE) { if (totalSize > getZipMaxTotalSize()) {
throw new TechnicalException("Expect uncompressed size %s to be smaller than %d!".formatted(totalSize, ZIP_MAX_TOTAL_SIZE)); throw new TechnicalException("Expect uncompressed size %s to be smaller than %d!".formatted(totalSize, getZipMaxTotalSize()));
} }
} }
int getZipMaxTotalSize() {
return ZIP_MAX_TOTAL_SIZE;
}
Long sumUncompressedEntrySizes(File zipFile) { Long sumUncompressedEntrySizes(File zipFile) {
return mapZipEntries(zipFile, ReadableZipEntry::getSize) return mapZipEntries(zipFile, ReadableZipEntry::getSize)
.stream() .stream()
...@@ -53,9 +57,9 @@ public class ZipFileExtractor { ...@@ -53,9 +57,9 @@ public class ZipFileExtractor {
private IncomingFile mapReadableEntryToIncomingFile(ReadableZipEntry entry) { private IncomingFile mapReadableEntryToIncomingFile(ReadableZipEntry entry) {
File file; File file;
try (var inputStream = entry.getInputStream()) { try (var inputStream = new LimitedInputStream(entry.getInputStream(), entry.getSize())) {
file = TempFileUtils.writeTmpFile(inputStream); 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); throw new TechnicalException("Failed reading zip file element %s!".formatted(entry.getName()), e);
} }
return createIncomingFile(file, entry.zipEntry()); 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; package de.ozgcloud.eingang.xta.zip;
import java.io.File; import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream; import java.io.FileOutputStream;
import java.io.IOException; import java.io.IOException;
import java.util.List; import java.util.List;
import java.util.zip.ZipEntry; import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream; import java.util.zip.ZipOutputStream;
import org.apache.commons.io.IOUtils;
import de.ozgcloud.common.binaryfile.TempFileUtils; import de.ozgcloud.common.binaryfile.TempFileUtils;
import lombok.Builder; import lombok.Builder;
import lombok.Getter; import lombok.Getter;
public class TestZipFileFactory { public class TestZipFileFactory {
private static final String EXPANDED_ENTRY_NAME = "bomb.txt";
@Builder @Builder
@Getter @Getter
public static class TestZipEntry { public static class TestZipEntry {
...@@ -33,4 +38,66 @@ public class TestZipFileFactory { ...@@ -33,4 +38,66 @@ public class TestZipFileFactory {
throw new RuntimeException("Failed to create temporary zip file", e); throw new RuntimeException("Failed to create temporary zip file", e);
} }
} }
public static File createTempZipBomb(int maxTotalSize) {
var file = TempFileUtils.createTmpFile().toFile();
try (var zipOutputStream = new ZipOutputStream(new FileOutputStream(file))) {
var entry = new ZipEntry(EXPANDED_ENTRY_NAME);
var content = "A".repeat(2 * maxTotalSize).getBytes();
zipOutputStream.putNextEntry(entry);
zipOutputStream.write(content);
zipOutputStream.closeEntry();
} catch (IOException e) {
throw new RuntimeException("Failed to create temporary zip file", e);
}
try {
var zipFileBytes = IOUtils.toByteArray(new FileInputStream(file));
overwriteZipEntrySize(zipFileBytes, EXPANDED_ENTRY_NAME, maxTotalSize);
// 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; package de.ozgcloud.eingang.xta.zip;
import static de.ozgcloud.eingang.xta.zip.TestZipFileFactory.*; 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.assertj.core.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*; import static org.mockito.Mockito.*;
...@@ -26,6 +27,7 @@ import org.springframework.util.MimeTypeUtils; ...@@ -26,6 +27,7 @@ import org.springframework.util.MimeTypeUtils;
import de.ozgcloud.eingang.common.errorhandling.TechnicalException; import de.ozgcloud.eingang.common.errorhandling.TechnicalException;
import de.ozgcloud.eingang.common.formdata.IncomingFile; import de.ozgcloud.eingang.common.formdata.IncomingFile;
import lombok.SneakyThrows;
class ZipFileExtractorTest { class ZipFileExtractorTest {
...@@ -154,6 +156,57 @@ class ZipFileExtractorTest { ...@@ -154,6 +156,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 = 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(createIncomingFile(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 = createTempZipBomb(SMALLER_MAX_ZIP_FILE_SIZE);
assertThatThrownBy(() -> extractor.extractIncomingFilesSafely(createIncomingFile(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 = createTempZipFile(List.of(TestZipEntry.builder()
.name("toolargefile.txt")
.content(toBytes("A".repeat(SMALLER_MAX_ZIP_FILE_SIZE + 1)))
.build()
));
assertThatThrownBy(() -> extractor.extractIncomingFilesSafely(createIncomingFile(zipFile)))
.isInstanceOf(TechnicalException.class);
}
private IncomingFile createIncomingFile(File file) {
return IncomingFile.builder()
.file(file)
.build();
}
}
@DisplayName("create incoming file") @DisplayName("create incoming file")
@Nested @Nested
class TestCreateIncomingFile { class TestCreateIncomingFile {
...@@ -271,7 +324,7 @@ class ZipFileExtractorTest { ...@@ -271,7 +324,7 @@ class ZipFileExtractorTest {
@DisplayName("should throw if max entries exceeded") @DisplayName("should throw if max entries exceeded")
@Test @Test
void shouldThrowIfMaxEntriesExceeded() { 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() .mapToObj(i -> TestZipEntry.builder()
.name("test%d.txt".formatted(i)) .name("test%d.txt".formatted(i))
.content(toBytes("test file %d".formatted(i))) .content(toBytes("test file %d".formatted(i)))
...@@ -285,7 +338,7 @@ class ZipFileExtractorTest { ...@@ -285,7 +338,7 @@ class ZipFileExtractorTest {
@DisplayName("should map with mapping function") @DisplayName("should map with mapping function")
@Test @Test
void shouldMapWithMappingFunction() { 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() var zipFile = createTempZipFile(expectedNumberList.stream()
.map(i -> TestZipEntry.builder() .map(i -> TestZipEntry.builder()
.name("%d".formatted(i)) .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