diff --git a/xta-adapter/src/main/java/de/ozgcloud/eingang/xta/zip/LimitedInputStream.java b/xta-adapter/src/main/java/de/ozgcloud/eingang/xta/zip/LimitedInputStream.java new file mode 100644 index 0000000000000000000000000000000000000000..ee5dc7a8b70e7fe0241a60d94e98ed783f47f66e --- /dev/null +++ b/xta-adapter/src/main/java/de/ozgcloud/eingang/xta/zip/LimitedInputStream.java @@ -0,0 +1,42 @@ +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; + } +} diff --git a/xta-adapter/src/main/java/de/ozgcloud/eingang/xta/zip/ZipFileExtractor.java b/xta-adapter/src/main/java/de/ozgcloud/eingang/xta/zip/ZipFileExtractor.java index 255f7eac2b3562f77724382c1216529b2c06ff0d..66480bc98bab421e4581ae32dfa72a7e8698d8e6 100644 --- a/xta-adapter/src/main/java/de/ozgcloud/eingang/xta/zip/ZipFileExtractor.java +++ b/xta-adapter/src/main/java/de/ozgcloud/eingang/xta/zip/ZipFileExtractor.java @@ -20,7 +20,7 @@ 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, there is no ZIP_MAX_THRESHOLD detection, instead the reader will abort if the inputstream exceeds zipFile.getSize(). @Component public class ZipFileExtractor { @@ -35,11 +35,15 @@ public class ZipFileExtractor { 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)); + if (totalSize > getZipMaxTotalSize()) { + 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) { return mapZipEntries(zipFile, ReadableZipEntry::getSize) .stream() @@ -53,9 +57,9 @@ public class ZipFileExtractor { private IncomingFile mapReadableEntryToIncomingFile(ReadableZipEntry entry) { File file; - try (var inputStream = entry.getInputStream()) { + try (var inputStream = new LimitedInputStream(entry.getInputStream(), entry.getSize())) { 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()); diff --git a/xta-adapter/src/test/java/de/ozgcloud/eingang/xta/zip/LimitedInputStreamTest.java b/xta-adapter/src/test/java/de/ozgcloud/eingang/xta/zip/LimitedInputStreamTest.java new file mode 100644 index 0000000000000000000000000000000000000000..b01c316d94f4bc994601401daaabacb4e0134efc --- /dev/null +++ b/xta-adapter/src/test/java/de/ozgcloud/eingang/xta/zip/LimitedInputStreamTest.java @@ -0,0 +1,141 @@ +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); + } +} diff --git a/xta-adapter/src/test/java/de/ozgcloud/eingang/xta/zip/TestZipFileFactory.java b/xta-adapter/src/test/java/de/ozgcloud/eingang/xta/zip/TestZipFileFactory.java index 1cb3039d44b1acc2767ae36159c4802457cff8e4..cb721d211197124b63aff2a66b75d999a5011678 100644 --- a/xta-adapter/src/test/java/de/ozgcloud/eingang/xta/zip/TestZipFileFactory.java +++ b/xta-adapter/src/test/java/de/ozgcloud/eingang/xta/zip/TestZipFileFactory.java @@ -1,18 +1,23 @@ 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,66 @@ public class TestZipFileFactory { 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; + } } diff --git a/xta-adapter/src/test/java/de/ozgcloud/eingang/xta/zip/ZipFileExtractorTest.java b/xta-adapter/src/test/java/de/ozgcloud/eingang/xta/zip/ZipFileExtractorTest.java index 5a79c2a8e2fbd90ba33ac64012991085b098282f..aeb945cc8fec2c5f3111d4cf2a93ee8c983d6ecc 100644 --- a/xta-adapter/src/test/java/de/ozgcloud/eingang/xta/zip/ZipFileExtractorTest.java +++ b/xta-adapter/src/test/java/de/ozgcloud/eingang/xta/zip/ZipFileExtractorTest.java @@ -1,6 +1,7 @@ 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 { @@ -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") @Nested class TestCreateIncomingFile { @@ -271,7 +324,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 +338,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))