From 4a5b0b642ffda9dd7c958d23e058b64f01936a6f Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Tue, 20 Oct 2026 16:33:45 -0400 Subject: [PATCH 1/2] feat(sdk): read deflate-compressed ZIP entries in TDF archives OpenTDF writers store TDF entries uncompressed, but other ZTDF/TDF writers may deflate the manifest or the payload. ZipReader ignored the compression method and returned a deflated entry's raw compressed bytes as its contents, so such a TDF failed later with a misleading manifest parse or segment error. ZipReader now inflates entries that use method 8 (raw DEFLATE) and refuses every method other than 0 and 8 with an InvalidZipException that names the entry and the method. Entries keep both sizes from the central directory, zip64 extra fields included: the compressed size bounds what is read from the archive and the uncompressed size bounds what is inflated. A deflated entry that inflates to more or less than its declared size is rejected, so a small entry cannot expand past what it declares. Inflation is incremental, so a deflated payload is never held in memory as a whole. Writers are unchanged and still emit stored entries. The CRC-32 is not checked: TDF integrity is verified by the segment hashes and the root signature over the inflated bytes. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Paul Flynn --- .../platform/sdk/InvalidZipException.java | 4 + .../io/opentdf/platform/sdk/TDFReader.java | 5 +- .../io/opentdf/platform/sdk/ZipReader.java | 218 ++++++- .../opentdf/platform/sdk/TDFReaderTest.java | 59 ++ .../java/io/opentdf/platform/sdk/TDFTest.java | 95 +++ .../opentdf/platform/sdk/ZipReaderTest.java | 545 ++++++++++++++++++ 6 files changed, 915 insertions(+), 11 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/InvalidZipException.java b/sdk/src/main/java/io/opentdf/platform/sdk/InvalidZipException.java index 8bba58bdf..82e5b2da9 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/InvalidZipException.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/InvalidZipException.java @@ -9,4 +9,8 @@ public class InvalidZipException extends RuntimeException { public InvalidZipException(String message) { super(message); } + + public InvalidZipException(String message, Throwable cause) { + super(message, cause); + } } diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java b/sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java index 93548d678..40e4f468a 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java @@ -51,8 +51,9 @@ public TDFReader(SeekableByteChannel tdf) throws SDKException, IOException { String manifest() { var out = new ByteArrayOutputStream(); - try { - manifestEntry.getData().transferTo(out); + // closing releases the inflater when the manifest entry is deflated + try (var manifestData = manifestEntry.getData()) { + manifestData.transferTo(out); } catch (IOException e) { throw new SDKException("error retrieving manifest from zip file", e); } diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java index 576ba5dee..9e038434c 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java @@ -12,12 +12,16 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; +import java.util.Objects; +import java.util.zip.DataFormatException; +import java.util.zip.Inflater; /** * The ZipReader class provides functionality to read basic ZIP file * structures, such as the End of Central Directory Record and the * Local File Header. This class supports standard ZIP archives as well - * as ZIP64 format. + * as ZIP64 format, with entries that are stored (method 0) or deflated + * (method 8). Entry sizes are taken from the central directory. */ public class ZipReader { @@ -101,6 +105,10 @@ public CentralDirectoryRecord(long numEntries, long offsetToStart) { /** The same sentinel for a 16-bit field, which is only two bytes wide. */ private static final int ZIP64_MAGIC_SHORT = 0xFFFF; private static final int ZIP64_EXTID= 0x0001; + /** Compression method 0: the entry's bytes are written as they are. All OpenTDF writers use it. */ + private static final int STORED = 0; + /** Compression method 8: raw DEFLATE (RFC 1951), which some other TDF writers use. */ + private static final int DEFLATED = 8; CentralDirectoryRecord readEndOfCentralDirectory() throws IOException { long eoCDRStart = zipChannel.size() - END_OF_CENTRAL_DIRECTORY_SIZE; // 22 is the minimum size of the EOCDR @@ -176,19 +184,36 @@ private CentralDirectoryRecord extractZIP64CentralDirectoryInfo() throws IOExcep public class Entry { private final long fileSize; + private final long compressedSize; + private final int compressionMethod; private final String fileName; final long offsetToLocalHeader; - private Entry(byte[] fileName, long offsetToLocalHeader, long fileSize) { + private Entry(byte[] fileName, long offsetToLocalHeader, int compressionMethod, long compressedSize, + long uncompressedSize) { this.fileName = new String(fileName, StandardCharsets.UTF_8); this.offsetToLocalHeader = offsetToLocalHeader; - this.fileSize = fileSize; + this.compressionMethod = compressionMethod; + this.compressedSize = compressedSize; + this.fileSize = uncompressedSize; } public String getName() { return fileName; } + int getCompressionMethod() { + return compressionMethod; + } + + long getCompressedSize() { + return compressedSize; + } + + long getUncompressedSize() { + return fileSize; + } + /** * Checks that this entry's local header offset points inside the archive, so a corrupt * or truncated central directory fails here rather than at an arbitrary position. @@ -228,9 +253,44 @@ private long findStartOfData() throws IOException { return zipChannel.position() + filenameLength + extrafieldLength; } + /** + * Opens this entry's contents. A stored entry is read straight from the archive; a + * deflated one is inflated as it is read, never past the uncompressed size the central + * directory declares. Any other compression method is refused here, so an archive whose + * other entries use one can still be listed and those entries left unread. + *

+ * Entries share the archive's channel. Each stream positions the channel itself on every + * read, so streams over different entries may be interleaved, but a stream must not be + * used from more than one thread at a time. + * + * @return the entry's uncompressed bytes + * @throws InvalidZipException if the entry uses a compression method other than stored + * or deflate, or its local header is invalid. A deflated stream also throws it from + * {@code read} when its data is corrupt or does not inflate to exactly the declared + * uncompressed size. + */ public InputStream getData() throws IOException { - final long startPosition = findStartOfData(); - final long endPosition = startPosition + fileSize; + switch (compressionMethod) { + case STORED: + return channelStream(findStartOfData(), fileSize); + case DEFLATED: + // zip64 sizes are read as signed longs, so one at or above 2^63 arrives negative + if (compressedSize < 0 || fileSize < 0) { + throw new InvalidZipException("entry [" + fileName + "] declares a size out of range: compressed " + + Long.toUnsignedString(compressedSize) + ", uncompressed " + Long.toUnsignedString(fileSize)); + } + return new InflatingInputStream(channelStream(findStartOfData(), compressedSize), fileSize, fileName); + default: + throw new InvalidZipException("unsupported compression method " + compressionMethod + + " for entry [" + fileName + "]; only stored (0) and deflate (8) are supported"); + } + } + + /** + * Reads {@code length} bytes of the archive starting at {@code startPosition}, ending early + * only if the archive does. + */ + private InputStream channelStream(final long startPosition, final long length) { final ByteBuffer buf = ByteBuffer.allocate(1); return new InputStream() { long offset = 0; @@ -251,7 +311,7 @@ public int read() throws IOException { } private boolean doneReading() { - return offset >= fileSize; + return offset >= length; } private void setChannelPosition() throws IOException { @@ -267,7 +327,7 @@ public int read(byte[] b, int off, int len) throws IOException { return -1; } setChannelPosition(); - var lenToRead = (int)Math.min(len, fileSize - offset); // cast is always valid because len is an int + var lenToRead = (int)Math.min(len, length - offset); // cast is always valid because len is an int var buf = ByteBuffer.wrap(b, off, lenToRead); var nread = zipChannel.read(buf); if (nread > 0) { @@ -286,7 +346,7 @@ public Entry readCentralDirectoryFileHeader() throws IOException { short versionMadeBy = readShort(); short versionNeededToExtract = readShort(); short generalPurposeBitFlag = readShort(); - short compressionMethod = readShort(); + int compressionMethod = readUnsignedShort(); short lastModFileTime = readShort(); short lastModFileDate = readShort(); int crc32 = readInt(); @@ -335,7 +395,8 @@ public Entry readCentralDirectoryFileHeader() throws IOException { zipChannel.position(zipChannel.position() + fileCommentLength); - return new Entry(fileName.array(), relativeOffsetOfLocalHeader, uncompressedSize); + return new Entry(fileName.array(), relativeOffsetOfLocalHeader, compressionMethod, compressedSize, + uncompressedSize); } public ZipReader(SeekableByteChannel channel) throws IOException { @@ -353,4 +414,143 @@ public ZipReader(SeekableByteChannel channel) throws IOException { public List getEntries() { return entries; } + + /** + * Inflates a raw DEFLATE stream to exactly the uncompressed size the central directory + * declares. Output is produced only as it is read, so a large entry is never held in memory, + * and never past the declared size: once that many bytes have been returned the deflate + * stream has to end there, and an entry that holds more, or ends sooner, is rejected. That + * keeps a small entry from expanding without bound, whatever it claims about itself. + *

+ * The CRC-32 is not checked. A TDF's integrity comes from its segment hashes and root + * signature, which are verified over these bytes. + */ + private static final class InflatingInputStream extends InputStream { + private static final int INPUT_BUFFER_SIZE = 8192; + + private final InputStream compressed; + private final long uncompressedSize; + private final String entryName; + private final Inflater inflater = new Inflater(true); + private final byte[] input = new byte[INPUT_BUFFER_SIZE]; + private final byte[] singleByte = new byte[1]; + private long produced = 0; + private boolean paddingFed = false; + private boolean endVerified = false; + private boolean closed = false; + + InflatingInputStream(InputStream compressed, long uncompressedSize, String entryName) { + this.compressed = compressed; + this.uncompressedSize = uncompressedSize; + this.entryName = entryName; + } + + @Override + public int read() throws IOException { + return read(singleByte, 0, 1) == -1 ? -1 : singleByte[0] & 0xFF; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + Objects.checkFromIndexSize(off, len, b.length); + if (closed) { + throw new IOException("stream closed"); + } + if (len == 0) { + return 0; + } + if (produced == uncompressedSize) { + verifyEnd(); + return -1; + } + // never ask for more than the declared size, so nothing past it reaches the caller + int wanted = (int) Math.min(len, uncompressedSize - produced); + while (true) { + int n = inflate(b, off, wanted); + if (n > 0) { + produced += n; + return n; + } + if (inflater.finished()) { + throw new InvalidZipException("deflated entry [" + entryName + "] ended after " + produced + + " bytes, but its declared uncompressed size is " + uncompressedSize); + } + fill(); + } + } + + /** + * At the declared size the deflate stream must also be at its end. Probes for a single + * further byte: getting one means the entry holds more than it declares. + */ + private void verifyEnd() throws IOException { + if (endVerified) { + return; + } + byte[] probe = new byte[1]; + while (!inflater.finished()) { + if (inflate(probe, 0, 1) > 0) { + throw new InvalidZipException("deflated entry [" + entryName + + "] holds more data than its declared uncompressed size of " + uncompressedSize); + } + if (!inflater.finished()) { + fill(); + } + } + endVerified = true; + } + + private int inflate(byte[] b, int off, int len) { + int n; + try { + n = inflater.inflate(b, off, len); + } catch (DataFormatException e) { + throw new InvalidZipException("invalid deflate data in entry [" + entryName + "]", e); + } + if (n > 0 && paddingFed) { + // the padding byte was taken as data, so the real compressed data ran out first + throw truncated(); + } + return n; + } + + private void fill() throws IOException { + if (inflater.needsDictionary()) { + // raw deflate has no way to name a dictionary, so only corrupt data gets here + throw new InvalidZipException("invalid deflate data in entry [" + entryName + + "]: it asks for a preset dictionary"); + } + if (!inflater.needsInput()) { + return; + } + int n = compressed.read(input, 0, input.length); + if (n > 0) { + inflater.setInput(input, 0, n); + return; + } + if (!paddingFed) { + // without the zlib wrapper, zlib may want one byte past the end of the deflate + // stream before it reports the end (see Inflater(boolean)); ZipFile pads the same way + paddingFed = true; + input[0] = 0; + inflater.setInput(input, 0, 1); + return; + } + throw truncated(); + } + + private InvalidZipException truncated() { + return new InvalidZipException("deflated entry [" + entryName + "] is truncated: its compressed data ran out after " + + produced + " of its declared " + uncompressedSize + " uncompressed bytes"); + } + + @Override + public void close() throws IOException { + if (!closed) { + closed = true; + inflater.end(); + compressed.close(); + } + } + } } diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/TDFReaderTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/TDFReaderTest.java index 8844a0026..0e887bdd2 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFReaderTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFReaderTest.java @@ -1,5 +1,6 @@ package io.opentdf.platform.sdk; +import org.apache.commons.compress.archivers.zip.Zip64Mode; import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -181,4 +182,62 @@ void readsThePayloadAlongsideASpecNamedManifest() throws IOException { assertThat(buf).startsWith(expected); } } + + /** + * The reader opens the payload stream when it is constructed and may read the manifest again + * at any point (for the policy, say) while the payload is part way through. With both + * entries deflated, the payload stream must pick up where it left off. + */ + @ParameterizedTest + @ValueSource(strings = { "manifest.json", "0.manifest.json" }) + void readsADeflatedPayloadAroundRereadsOfADeflatedManifest(String manifestName) throws IOException { + byte[] payload = ZipReaderTest.compressibleBytes(100_000); + byte[] archive = ZipReaderTest.zipOf(Zip64Mode.Never, + ZipReaderTest.deflated(TDFWriter.TDF_PAYLOAD_FILE_NAME, payload), + ZipReaderTest.deflated(manifestName, MANIFEST.getBytes(StandardCharsets.UTF_8))); + try (var tdf = new SeekableInMemoryByteChannel(archive)) { + var reader = new TDFReader(tdf); + var first = new byte[30_000]; + var rest = new byte[payload.length - first.length + 1]; + + assertThat(reader.readPayloadBytes(first)).isEqualTo(first.length); + assertThat(reader.manifest()).isEqualTo(MANIFEST); + assertThat(reader.readPayloadBytes(rest)).isEqualTo(payload.length - first.length); + assertThat(reader.manifest()).isEqualTo(MANIFEST); + + var whole = new ByteArrayOutputStream(); + whole.write(first); + whole.write(rest, 0, payload.length - first.length); + assertThat(whole.toByteArray()).isEqualTo(payload); + } + } + + @Test + void rejectsAPayloadWithAnUnsupportedCompressionMethod() throws IOException { + byte[] archive = ZipReaderTest.withCompressionMethod(ZipReaderTest.zipOf(Zip64Mode.Never, + ZipReaderTest.stored(TDFWriter.TDF_PAYLOAD_FILE_NAME, PAYLOAD.getBytes(StandardCharsets.UTF_8)), + ZipReaderTest.stored("manifest.json", MANIFEST.getBytes(StandardCharsets.UTF_8))), + TDFWriter.TDF_PAYLOAD_FILE_NAME, 12); + try (var tdf = new SeekableInMemoryByteChannel(archive)) { + assertThatThrownBy(() -> new TDFReader(tdf)) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[" + TDFWriter.TDF_PAYLOAD_FILE_NAME + "]") + .hasMessageContaining("unsupported compression method 12"); + } + } + + @Test + void rejectsAManifestWithAnUnsupportedCompressionMethod() throws IOException { + byte[] archive = ZipReaderTest.withCompressionMethod(ZipReaderTest.zipOf(Zip64Mode.Never, + ZipReaderTest.stored(TDFWriter.TDF_PAYLOAD_FILE_NAME, PAYLOAD.getBytes(StandardCharsets.UTF_8)), + ZipReaderTest.stored("manifest.json", MANIFEST.getBytes(StandardCharsets.UTF_8))), + "manifest.json", 12); + try (var tdf = new SeekableInMemoryByteChannel(archive)) { + var reader = new TDFReader(tdf); + assertThatThrownBy(reader::manifest) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[manifest.json]") + .hasMessageContaining("unsupported compression method 12"); + } + } } diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java index aa5b38e6b..773f37952 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; import javax.annotation.Nonnull; @@ -41,6 +42,9 @@ import java.util.function.Predicate; import java.util.regex.Pattern; import java.util.stream.Collectors; +import java.util.zip.CRC32; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; import static io.opentdf.platform.sdk.TDF.GLOBAL_KEY_SALT; import static org.junit.jupiter.api.Assertions.assertArrayEquals; @@ -916,6 +920,97 @@ private static ByteArrayOutputStream readZipEntry(ZipReader reader, String name) return data; } + /** + * OpenTDF writers store both entries, but other TDF writers may deflate either one. Takes a + * TDF this SDK wrote, repacks it with the requested entries deflated, and decrypts it end to + * end. The entry contents are unchanged, so the segment hashes and root signature still have + * to verify against the inflated bytes. The payload spans several segments, so the reader + * walks the inflated stream across segment boundaries. + */ + @ParameterizedTest + @CsvSource({ + "manifest.json, true, true", + "0.manifest.json, true, true", + "manifest.json, true, false", + "0.manifest.json, false, true", + }) + public void testDecryptingATDFWithDeflatedEntries(String manifestName, boolean deflateManifest, + boolean deflatePayload) throws Exception { + String metadata = "metadata for a deflated tdf"; + var data = new byte[(int) (Config.MIN_SEGMENT_SIZE * 3.5)]; + new Random(7741).nextBytes(data); + var tdf = new TDF(new FakeServicesBuilder().setKas(kas) + .setKeyAccessServerRegistryService(kasRegistryService).build()); + var original = new ByteArrayOutputStream(); + var tdfObject = tdf.createTDF(new ByteArrayInputStream(data), original, + Config.newTDFConfig( + Config.withAutoconfigure(false), + Config.withKasInformation(getRSAKASInfos()), + Config.withSegmentSize(Config.MIN_SEGMENT_SIZE), + Config.withMetaData(metadata))); + assertThat(tdfObject.getManifest().encryptionInformation.integrityInformation.segments) + .withFailMessage("the test needs more than one segment to be meaningful") + .hasSizeGreaterThan(1); + + byte[] repacked = repackWithDeflate(original.toByteArray(), manifestName, deflateManifest, deflatePayload); + assertThat(ZipReaderTest.compressionMethodOf(repacked, manifestName)) + .isEqualTo(deflateManifest ? ZipEntry.DEFLATED : ZipEntry.STORED); + assertThat(ZipReaderTest.compressionMethodOf(repacked, TDFWriter.TDF_PAYLOAD_FILE_NAME)) + .isEqualTo(deflatePayload ? ZipEntry.DEFLATED : ZipEntry.STORED); + + var unwrapped = new ByteArrayOutputStream(); + var reader = tdf.loadTDF(new SeekableInMemoryByteChannel(repacked), platformUrl); + reader.readPayload(unwrapped); + + assertThat(unwrapped.toByteArray()) + .withFailMessage("extracted data does not match") + .containsExactly(data); + assertThat(reader.getMetadata()).isEqualTo(metadata); + assertThat(SDK.readManifest(new SeekableInMemoryByteChannel(repacked)).payload.isEncrypted).isTrue(); + } + + /** + * Repacks a TDF this SDK wrote using {@link ZipOutputStream}, deflating the entries asked + * for. That writer streams, so deflated entries carry their sizes in a data descriptor and + * the central directory, with zeros in the local header. + */ + private static byte[] repackWithDeflate(byte[] tdfBytes, String manifestName, boolean deflateManifest, + boolean deflatePayload) throws IOException { + final byte[] manifest; + final byte[] payload; + try (var channel = new SeekableInMemoryByteChannel(tdfBytes)) { + var reader = new ZipReader(channel); + manifest = readZipEntry(reader, TDFWriter.TDF_MANIFEST_FILE_NAME).toByteArray(); + payload = readZipEntry(reader, TDFWriter.TDF_PAYLOAD_FILE_NAME).toByteArray(); + } + + var out = new ByteArrayOutputStream(); + try (var zip = new ZipOutputStream(out)) { + putZipEntry(zip, TDFWriter.TDF_PAYLOAD_FILE_NAME, payload, deflatePayload); + putZipEntry(zip, manifestName, manifest, deflateManifest); + } + return out.toByteArray(); + } + + private static void putZipEntry(ZipOutputStream zip, String name, byte[] content, boolean deflate) + throws IOException { + var entry = new ZipEntry(name); + if (deflate) { + entry.setMethod(ZipEntry.DEFLATED); + } else { + // ZipOutputStream needs a stored entry's size and CRC before its data + var crc = new CRC32(); + crc.update(content); + entry.setMethod(ZipEntry.STORED); + entry.setSize(content.length); + entry.setCompressedSize(content.length); + entry.setCrc(crc.getValue()); + } + zip.putNextEntry(entry); + zip.write(content); + zip.closeEntry(); + } + @Test public void testIvCounterUsesBigEndianInvocationAndDefensiveCopy() { diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java index fa2014bd5..0ff1577ce 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java @@ -7,22 +7,30 @@ import org.apache.commons.compress.archivers.zip.ZipFile; import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.RandomAccessFile; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.channels.SeekableByteChannel; import java.nio.charset.StandardCharsets; +import java.util.Arrays; import java.util.HashMap; import java.util.Map; import java.util.Random; import java.util.stream.Collectors; import java.util.stream.IntStream; +import java.util.zip.DataFormatException; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class ZipReaderTest { @@ -273,4 +281,541 @@ private static String readEntry(ZipFile zip, String name) throws IOException { zip.getInputStream(zip.getEntry(name)).transferTo(data); return data.toString(StandardCharsets.UTF_8); } + + // OpenTDF writers store entries uncompressed, but other TDF writers may deflate them. The + // reader has to inflate method 8, refuse every other method by name, and hold inflation to + // the uncompressed size the central directory declares. + + @Test + public void testReadingDeflatedEntries() throws IOException { + byte[] first = compressibleBytes(10_000); + byte[] second = compressibleBytes(3_000); + byte[] archive = zipOf(Zip64Mode.Never, deflated("first", first), deflated("second", second)); + assertThat(compressionMethodOf(archive, "first")).isEqualTo(ZipEntry.DEFLATED); + assertThat(compressedSizeOf(archive, "first")).isLessThan(first.length); + + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + + assertThat(readEntryBytes(reader, "first")).isEqualTo(first); + assertThat(readEntryBytes(reader, "second")).isEqualTo(second); + } + + @Test + public void testReadingStoredAndDeflatedEntriesInOneArchive() throws IOException { + byte[] storedContent = compressibleBytes(5_000); + byte[] deflatedContent = compressibleBytes(7_000); + byte[] archive = zipOf(Zip64Mode.Never, + stored("stored", storedContent), + deflated("deflated", deflatedContent), + stored("empty stored", new byte[0]), + deflated("empty deflated", new byte[0])); + assertThat(compressionMethodOf(archive, "stored")).isEqualTo(ZipEntry.STORED); + assertThat(compressionMethodOf(archive, "deflated")).isEqualTo(ZipEntry.DEFLATED); + assertThat(compressionMethodOf(archive, "empty deflated")).isEqualTo(ZipEntry.DEFLATED); + + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + + assertThat(readEntryBytes(reader, "stored")).isEqualTo(storedContent); + assertThat(readEntryBytes(reader, "deflated")).isEqualTo(deflatedContent); + assertThat(readEntryBytes(reader, "empty stored")).isEmpty(); + assertThat(readEntryBytes(reader, "empty deflated")).isEmpty(); + } + + /** With zip64 forced on, every size in the central directory lives in the zip64 extra field. */ + @Test + public void testReadingDeflatedZip64Entries() throws IOException { + byte[] content = compressibleBytes(20_000); + byte[] small = compressibleBytes(10); + byte[] archive = zipOf(Zip64Mode.Always, deflated("big", content), stored("small", small)); + + long expectedCompressedSize; + try (var zip = new ZipFile.Builder().setSeekableByteChannel(new SeekableInMemoryByteChannel(archive)).get()) { + expectedCompressedSize = zip.getEntry("big").getCompressedSize(); + } + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + var entry = entryNamed(reader, "big"); + + assertThat(entry.getCompressionMethod()).isEqualTo(ZipEntry.DEFLATED); + assertThat(entry.getCompressedSize()).isEqualTo(expectedCompressedSize).isLessThan(content.length); + assertThat(entry.getUncompressedSize()).isEqualTo(content.length); + assertThat(readEntryBytes(reader, "big")).isEqualTo(content); + assertThat(readEntryBytes(reader, "small")).isEqualTo(small); + } + + /** A stored entry keeps both sizes too, and they are equal. */ + @Test + public void testStoredEntryReportsItsSizes() throws IOException { + byte[] content = compressibleBytes(1_234); + var reader = new ZipReader(new SeekableInMemoryByteChannel(zipOf(Zip64Mode.Never, stored("entry", content)))); + var entry = entryNamed(reader, "entry"); + + assertThat(entry.getCompressionMethod()).isEqualTo(ZipEntry.STORED); + assertThat(entry.getCompressedSize()).isEqualTo(content.length); + assertThat(entry.getUncompressedSize()).isEqualTo(content.length); + } + + /** + * A streaming writer such as {@link ZipOutputStream} does not know a deflated entry's sizes + * until after its data, so it leaves zeros in the local header and puts the sizes in a data + * descriptor and the central directory. The reader must take them from the central directory. + */ + @Test + public void testReadingADeflatedEntryWrittenWithADataDescriptor() throws IOException { + byte[] content = compressibleBytes(9_000); + var out = new ByteArrayOutputStream(); + try (var zip = new ZipOutputStream(out)) { + var entry = new ZipEntry("streamed"); + entry.setMethod(ZipEntry.DEFLATED); + zip.putNextEntry(entry); + zip.write(content); + zip.closeEntry(); + } + byte[] archive = out.toByteArray(); + int localHeader = (int) localHeaderOffsetOf(archive, "streamed"); + assertThat(ByteBuffer.wrap(archive).order(ByteOrder.LITTLE_ENDIAN).getInt(localHeader + LFH_COMPRESSED_SIZE)) + .withFailMessage("the fixture should leave the local header sizes unset") + .isZero(); + + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + + assertThat(readEntryBytes(reader, "streamed")).isEqualTo(content); + } + + @Test + public void testSingleByteReadsThroughADeflatedEntry() throws IOException { + byte[] content = compressibleBytes(2_000); + var reader = new ZipReader(new SeekableInMemoryByteChannel(zipOf(Zip64Mode.Never, deflated("entry", content)))); + var actual = new ByteArrayOutputStream(); + try (var data = entryNamed(reader, "entry").getData()) { + int next; + while ((next = data.read()) != -1) { + actual.write(next); + } + } + + assertThat(actual.toByteArray()).isEqualTo(content); + } + + /** + * Entries share the archive's channel, and a TDF reader keeps the payload stream open while + * it reads the manifest, so each deflated stream has to find its own place in the archive on + * every read rather than trusting where the channel was left. + */ + @Test + public void testInterleavedReadsOfDeflatedEntries() throws IOException { + byte[] first = randomBytes(50_000, 1); + byte[] second = compressibleBytes(30_000); + byte[] archive = zipOf(Zip64Mode.Never, deflated("first", first), deflated("second", second)); + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + + var firstOut = new ByteArrayOutputStream(); + var secondOut = new ByteArrayOutputStream(); + try (var firstData = entryNamed(reader, "first").getData(); + var secondData = entryNamed(reader, "second").getData()) { + byte[] buf = new byte[777]; + boolean firstDone = false; + boolean secondDone = false; + while (!firstDone || !secondDone) { + firstDone = firstDone || copyOnce(firstData, buf, firstOut); + secondDone = secondDone || copyOnce(secondData, buf, secondOut); + } + } + + assertThat(firstOut.toByteArray()).isEqualTo(first); + assertThat(secondOut.toByteArray()).isEqualTo(second); + } + + /** + * 12 is bzip2, 9 deflate64, 14 LZMA, 93 zstd and 99 the AE-x encryption marker: real methods, + * none of which this reader implements. 0xFFFF checks the method is read unsigned. + */ + @ParameterizedTest + @ValueSource(ints = { 1, 9, 12, 14, 93, 99, 0xFFFF }) + public void testUnsupportedCompressionMethodIsRejectedByName(int method) throws IOException { + byte[] archive = withCompressionMethod( + zipOf(Zip64Mode.Never, stored("0.payload", compressibleBytes(100))), "0.payload", method); + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + var entry = entryNamed(reader, "0.payload"); + + assertThatThrownBy(entry::getData) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[0.payload]") + .hasMessageContaining("unsupported compression method " + method); + } + + /** Refused only when read, so an archive can still be listed and its other entries read. */ + @Test + public void testUnsupportedMethodOnOneEntryDoesNotAffectTheOthers() throws IOException { + byte[] good = compressibleBytes(500); + byte[] archive = withCompressionMethod( + zipOf(Zip64Mode.Never, deflated("good", good), stored("bad", compressibleBytes(100))), "bad", 12); + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + + assertThat(reader.getEntries()).extracting(ZipReader.Entry::getName).containsExactly("good", "bad"); + assertThat(readEntryBytes(reader, "good")).isEqualTo(good); + var bad = entryNamed(reader, "bad"); + assertThatThrownBy(bad::getData).isInstanceOf(InvalidZipException.class); + } + + /** + * The declared uncompressed size bounds inflation: the reader hands back exactly that many + * bytes, never writes past them into the caller's buffer, and then refuses the entry because + * the deflate stream holds more. + */ + @ParameterizedTest + @ValueSource(longs = { 0, 1, 100, 9_999 }) + public void testDeflatedEntryLargerThanDeclaredIsRejected(long declared) throws IOException { + byte[] content = compressibleBytes(10_000); + byte[] archive = withDeclaredUncompressedSize( + zipOf(Zip64Mode.Never, deflated("entry", content)), "entry", declared); + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + + var delivered = new ByteArrayOutputStream(); + try (var data = entryNamed(reader, "entry").getData()) { + assertThatThrownBy(() -> drainIntoOversizedBuffer(data, content.length * 2, delivered)) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[entry]") + .hasMessageContaining("more data than its declared uncompressed size of " + declared); + } + + assertThat(delivered.toByteArray()).isEqualTo(Arrays.copyOf(content, (int) declared)); + } + + /** + * Reads {@code in} to the end through a buffer of {@code bufferSize} bytes, failing if a read + * writes into the buffer past the count it returns. + */ + private static void drainIntoOversizedBuffer(InputStream in, int bufferSize, ByteArrayOutputStream out) + throws IOException { + final byte untouched = 0x5A; + byte[] buf = new byte[bufferSize]; + Arrays.fill(buf, untouched); + int n; + while ((n = in.read(buf, 0, buf.length)) != -1) { + out.write(buf, 0, n); + for (int i = n; i < buf.length; i++) { + if (buf[i] != untouched) { + throw new AssertionError("wrote past the " + n + " bytes it returned"); + } + } + Arrays.fill(buf, 0, n, untouched); + } + } + + /** + * A highly compressible entry that understates its size is the shape of a zip bomb. However + * far it would expand, the reader delivers only the declared size before refusing it. + */ + @Test + public void testZipBombIsStoppedAtTheDeclaredSize() throws IOException { + byte[] bomb = new byte[16 * 1024 * 1024]; + byte[] archive = zipOf(Zip64Mode.Never, deflated("bomb", bomb)); + assertThat(archive.length).isLessThan(bomb.length / 500); + archive = withDeclaredUncompressedSize(archive, "bomb", 1024); + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + + var delivered = new ByteArrayOutputStream(); + try (var data = entryNamed(reader, "bomb").getData()) { + assertThatThrownBy(() -> data.transferTo(delivered)) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[bomb]"); + } + + assertThat(delivered.size()).isEqualTo(1024); + } + + @ParameterizedTest + @ValueSource(longs = { 1, 2, 5_000 }) + public void testDeflatedEntrySmallerThanDeclaredIsRejected(long extra) throws IOException { + byte[] content = compressibleBytes(10_000); + long declared = content.length + extra; + byte[] archive = withDeclaredUncompressedSize( + zipOf(Zip64Mode.Never, deflated("entry", content)), "entry", declared); + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + + var delivered = new ByteArrayOutputStream(); + try (var data = entryNamed(reader, "entry").getData()) { + assertThatThrownBy(() -> data.transferTo(delivered)) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[entry]") + .hasMessageContaining("ended after " + content.length + " bytes") + .hasMessageContaining("declared uncompressed size is " + declared); + } + assertThat(delivered.toByteArray()).isEqualTo(content); + } + + /** The compressed size bounds what is read from the archive, so a short one starves the inflater. */ + @Test + public void testDeflatedEntryWithTruncatedCompressedDataIsRejected() throws IOException { + byte[] content = randomBytes(10_000, 2); + byte[] archive = zipOf(Zip64Mode.Never, deflated("entry", content), stored("after", compressibleBytes(5_000))); + archive = withDeclaredCompressedSize(archive, "entry", compressedSizeOf(archive, "entry") / 2); + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + + var delivered = new ByteArrayOutputStream(); + try (var data = entryNamed(reader, "entry").getData()) { + assertThatThrownBy(() -> data.transferTo(delivered)) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[entry]"); + } + assertThat(delivered.toByteArray()).isEqualTo(Arrays.copyOf(content, delivered.size())); + assertThat(delivered.size()).isLessThan(content.length); + } + + /** A first byte of 0xFF opens a deflate block of the reserved type 3, which no inflater accepts. */ + @Test + public void testCorruptDeflateDataIsRejected() throws IOException { + byte[] archive = zipOf(Zip64Mode.Never, deflated("entry", compressibleBytes(1_000))); + archive[(int) dataOffsetOf(archive, "entry")] = (byte) 0xFF; + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + + try (var data = entryNamed(reader, "entry").getData()) { + assertThatThrownBy(() -> data.transferTo(new ByteArrayOutputStream())) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[entry]") + .hasCauseInstanceOf(DataFormatException.class); + } + } + + /** + * A zip64 size is a 64-bit field, so one at or above 2^63 arrives negative once read as a + * signed long. A deflated entry declaring one is refused as invalid, not left to fail on the + * arithmetic with an exception callers don't expect from a bad archive. + * + * @param field 0 for the uncompressed size, 8 for the compressed size: their order within + * the zip64 extra field + */ + @ParameterizedTest + @ValueSource(ints = { 0, 8 }) + public void testDeflatedEntryWithAnOutOfRangeZip64SizeIsRejected(int field) throws IOException { + byte[] archive = zipOf(Zip64Mode.Always, deflated("entry", compressibleBytes(1_000))); + ByteBuffer.wrap(archive).order(ByteOrder.LITTLE_ENDIAN) + .putLong(zip64SizesInCentralDirectoryOf(archive, "entry") + field, Long.MIN_VALUE); + var reader = new ZipReader(new SeekableInMemoryByteChannel(archive)); + var entry = entryNamed(reader, "entry"); + + assertThatThrownBy(entry::getData) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[entry]") + .hasMessageContaining("out of range"); + } + + @Test + public void testReadingAClosedDeflatedStreamFails() throws IOException { + var reader = new ZipReader(new SeekableInMemoryByteChannel( + zipOf(Zip64Mode.Never, deflated("entry", compressibleBytes(1_000))))); + var data = entryNamed(reader, "entry").getData(); + data.close(); + data.close(); + + assertThatThrownBy(() -> data.read(new byte[10])).isInstanceOf(IOException.class); + } + + private static boolean copyOnce(InputStream in, byte[] buf, ByteArrayOutputStream out) throws IOException { + int n = in.read(buf, 0, buf.length); + if (n == -1) { + return true; + } + out.write(buf, 0, n); + return false; + } + + /** Text that deflate shrinks a great deal, so a deflated fixture is visibly compressed. */ + static byte[] compressibleBytes(int size) { + byte[] pattern = "the quick brown fox jumps over the lazy dog. ".getBytes(StandardCharsets.US_ASCII); + byte[] out = new byte[size]; + for (int i = 0; i < size; i++) { + out[i] = pattern[i % pattern.length]; + } + return out; + } + + /** Incompressible bytes, so the deflate stream is about as long as its output. */ + private static byte[] randomBytes(int size, long seed) { + byte[] out = new byte[size]; + new Random(seed).nextBytes(out); + return out; + } + + private static ZipReader.Entry entryNamed(ZipReader reader, String name) { + return reader.getEntries().stream() + .filter(e -> e.getName().equals(name)) + .findFirst() + .orElseThrow(() -> new AssertionError("no entry named " + name)); + } + + private static byte[] readEntryBytes(ZipReader reader, String name) throws IOException { + var data = new ByteArrayOutputStream(); + try (var in = entryNamed(reader, name).getData()) { + in.transferTo(data); + } + return data.toByteArray(); + } + + static final class Item { + final String name; + final int method; + final byte[] content; + + private Item(String name, int method, byte[] content) { + this.name = name; + this.method = method; + this.content = content; + } + } + + static Item stored(String name, byte[] content) { + return new Item(name, ZipEntry.STORED, content); + } + + static Item deflated(String name, byte[] content) { + return new Item(name, ZipEntry.DEFLATED, content); + } + + /** + * Builds an archive with commons-compress, an implementation independent of the one under + * test. It writes to a seekable channel, so every size lands in the local header as well as + * the central directory. + */ + static byte[] zipOf(Zip64Mode zip64Mode, Item... items) throws IOException { + var channel = new SeekableInMemoryByteChannel(); + try (var zip = new ZipArchiveOutputStream(channel)) { + zip.setUseZip64(zip64Mode); + for (var item : items) { + var entry = new ZipArchiveEntry(item.name); + entry.setMethod(item.method); + zip.putArchiveEntry(entry); + zip.write(item.content); + zip.closeArchiveEntry(); + } + zip.finish(); + return Arrays.copyOf(channel.array(), (int) channel.size()); + } + } + + private static final int CENTRAL_FILE_HEADER_SIGNATURE = 0x02014b50; + private static final int LOCAL_FILE_HEADER_SIGNATURE = 0x04034b50; + /** Field offsets within a central directory file header (APPNOTE 4.3.12). */ + private static final int CDH_METHOD = 10; + private static final int CDH_COMPRESSED_SIZE = 20; + private static final int CDH_UNCOMPRESSED_SIZE = 24; + private static final int CDH_NAME_LENGTH = 28; + private static final int CDH_EXTRA_LENGTH = 30; + private static final int CDH_COMMENT_LENGTH = 32; + private static final int CDH_LOCAL_HEADER_OFFSET = 42; + private static final int CDH_FIXED_SIZE = 46; + private static final int ZIP64_EXTRA_ID = 0x0001; + private static final long ZIP64_SIZE_SENTINEL = 0xFFFFFFFFL; + /** Field offsets within a local file header (APPNOTE 4.3.7). */ + private static final int LFH_METHOD = 8; + private static final int LFH_COMPRESSED_SIZE = 18; + private static final int LFH_NAME_LENGTH = 26; + private static final int LFH_EXTRA_LENGTH = 28; + private static final int LFH_FIXED_SIZE = 30; + + /** + * Finds the central directory file header for {@code name}. Walks the central directory from + * the end of central directory record, so it handles only archives that are not zip64 and + * carry no comment -- which is every archive these patching helpers are used on. + */ + private static int centralDirectoryHeaderOf(byte[] archive, String name) { + var buf = ByteBuffer.wrap(archive).order(ByteOrder.LITTLE_ENDIAN); + int eocd = archive.length - EOCD_SIZE; + assertThat(buf.getInt(eocd)).isEqualTo(EOCD_SIGNATURE); + int entries = buf.getShort(eocd + 10) & 0xFFFF; + int header = buf.getInt(eocd + 16); + for (int i = 0; i < entries; i++) { + assertThat(buf.getInt(header)).isEqualTo(CENTRAL_FILE_HEADER_SIGNATURE); + int nameLength = buf.getShort(header + CDH_NAME_LENGTH) & 0xFFFF; + if (new String(archive, header + CDH_FIXED_SIZE, nameLength, StandardCharsets.UTF_8).equals(name)) { + return header; + } + header += CDH_FIXED_SIZE + nameLength + + (buf.getShort(header + CDH_EXTRA_LENGTH) & 0xFFFF) + + (buf.getShort(header + CDH_COMMENT_LENGTH) & 0xFFFF); + } + throw new AssertionError("no central directory entry named " + name); + } + + /** + * Finds the sizes in the zip64 extra field of {@code name}'s central directory header: the + * uncompressed size, then the compressed size. Scans for the header rather than walking from + * the end of central directory record, so it works on a zip64 archive. + */ + private static int zip64SizesInCentralDirectoryOf(byte[] archive, String name) { + var buf = ByteBuffer.wrap(archive).order(ByteOrder.LITTLE_ENDIAN); + byte[] nameBytes = name.getBytes(StandardCharsets.UTF_8); + for (int header = 0; header + CDH_FIXED_SIZE + nameBytes.length <= archive.length; header++) { + if (buf.getInt(header) != CENTRAL_FILE_HEADER_SIGNATURE + || (buf.getShort(header + CDH_NAME_LENGTH) & 0xFFFF) != nameBytes.length + || !Arrays.equals(archive, header + CDH_FIXED_SIZE, header + CDH_FIXED_SIZE + nameBytes.length, + nameBytes, 0, nameBytes.length)) { + continue; + } + assertThat(buf.getInt(header + CDH_UNCOMPRESSED_SIZE)).isEqualTo((int) ZIP64_SIZE_SENTINEL); + assertThat(buf.getInt(header + CDH_COMPRESSED_SIZE)).isEqualTo((int) ZIP64_SIZE_SENTINEL); + int field = header + CDH_FIXED_SIZE + nameBytes.length; + int end = field + (buf.getShort(header + CDH_EXTRA_LENGTH) & 0xFFFF); + while (field < end) { + if ((buf.getShort(field) & 0xFFFF) == ZIP64_EXTRA_ID) { + return field + 4; + } + field += 4 + (buf.getShort(field + 2) & 0xFFFF); + } + throw new AssertionError("no zip64 extra field for " + name); + } + throw new AssertionError("no central directory entry named " + name); + } + + static int compressionMethodOf(byte[] archive, String name) { + return ByteBuffer.wrap(archive).order(ByteOrder.LITTLE_ENDIAN) + .getShort(centralDirectoryHeaderOf(archive, name) + CDH_METHOD) & 0xFFFF; + } + + private static long compressedSizeOf(byte[] archive, String name) { + return ByteBuffer.wrap(archive).order(ByteOrder.LITTLE_ENDIAN) + .getInt(centralDirectoryHeaderOf(archive, name) + CDH_COMPRESSED_SIZE) & 0xFFFFFFFFL; + } + + private static long localHeaderOffsetOf(byte[] archive, String name) { + return ByteBuffer.wrap(archive).order(ByteOrder.LITTLE_ENDIAN) + .getInt(centralDirectoryHeaderOf(archive, name) + CDH_LOCAL_HEADER_OFFSET) & 0xFFFFFFFFL; + } + + private static long dataOffsetOf(byte[] archive, String name) { + var buf = ByteBuffer.wrap(archive).order(ByteOrder.LITTLE_ENDIAN); + int localHeader = (int) localHeaderOffsetOf(archive, name); + assertThat(buf.getInt(localHeader)).isEqualTo(LOCAL_FILE_HEADER_SIGNATURE); + return localHeader + LFH_FIXED_SIZE + + (buf.getShort(localHeader + LFH_NAME_LENGTH) & 0xFFFF) + + (buf.getShort(localHeader + LFH_EXTRA_LENGTH) & 0xFFFF); + } + + /** + * Rewrites an entry's compression method in its local header as well as the central + * directory, so the two agree as they would in an archive written that way. + */ + static byte[] withCompressionMethod(byte[] archive, String name, int method) { + byte[] patched = archive.clone(); + var buf = ByteBuffer.wrap(patched).order(ByteOrder.LITTLE_ENDIAN); + buf.putShort(centralDirectoryHeaderOf(patched, name) + CDH_METHOD, (short) method); + buf.putShort((int) localHeaderOffsetOf(patched, name) + LFH_METHOD, (short) method); + return patched; + } + + /** Rewrites the uncompressed size in the central directory, which is the one the reader trusts. */ + private static byte[] withDeclaredUncompressedSize(byte[] archive, String name, long size) { + byte[] patched = archive.clone(); + ByteBuffer.wrap(patched).order(ByteOrder.LITTLE_ENDIAN) + .putInt(centralDirectoryHeaderOf(patched, name) + CDH_UNCOMPRESSED_SIZE, (int) size); + return patched; + } + + /** Rewrites the compressed size in the central directory, which is the one the reader trusts. */ + private static byte[] withDeclaredCompressedSize(byte[] archive, String name, long size) { + byte[] patched = archive.clone(); + ByteBuffer.wrap(patched).order(ByteOrder.LITTLE_ENDIAN) + .putInt(centralDirectoryHeaderOf(patched, name) + CDH_COMPRESSED_SIZE, (int) size); + return patched; + } } From bd2a93e3fd7836ca80691782ec8e4a01aa354d4e Mon Sep 17 00:00:00 2001 From: Paul Flynn Date: Tue, 20 Oct 2026 20:25:22 -0400 Subject: [PATCH 2/2] fix(sdk): bound deflate inflation and release payload inflaters A deflated entry's inflation was bounded only by the uncompressed size it declared, so a small archive could make the reader buffer a manifest a thousand times its size before validating it. A deflated entry may now declare no more than the larger of the archive's length and 10 MiB (DEFLATED_SIZE_BUDGET_FLOOR), and one that declares more is rejected before anything is inflated. This applies to the payload as well as the manifest: a deflated payload is ciphertext, which does not compress, so a legitimate one never declares more than the archive holds. TDFReader opened the payload stream in its constructor, so a deflated payload's inflater was allocated by manifest-only reads and failed loads, and released only by garbage collection. The payload is now opened by the first payload read and released when readPayload ends, whether it succeeds or fails. The constructor still checks the payload entry, without opening it, so an unreadable payload is refused before any key is requested. readPayload read exactly the manifest's segment lengths and returned, so a deflated payload that held more data than it declared, or data past its last segment, was accepted. After the last segment a deflated payload must now be at its end. Stored payloads are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Paul Flynn --- .../java/io/opentdf/platform/sdk/TDF.java | 11 ++ .../io/opentdf/platform/sdk/TDFReader.java | 75 ++++++++- .../io/opentdf/platform/sdk/ZipReader.java | 82 +++++++-- .../opentdf/platform/sdk/TDFReaderTest.java | 95 +++++++++++ .../java/io/opentdf/platform/sdk/TDFTest.java | 158 +++++++++++++++++- .../opentdf/platform/sdk/ZipReaderTest.java | 56 ++++++- 6 files changed, 462 insertions(+), 15 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java index fddead945..2455fb5e9 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDF.java @@ -398,6 +398,17 @@ public void readPayload(OutputStream outputStream) throws SDK.TamperException, I validateSegmentSizes(); + try { + readSegments(outputStream, digest); + // the segment lengths are all the loop reads, so check a deflated payload ends there + tdfReader.finishPayload(); + } finally { + // releases a deflated payload's inflater however the read ended + tdfReader.closePayload(); + } + } + + private void readSegments(OutputStream outputStream, MessageDigest digest) throws IOException { for (Manifest.Segment segment : manifest.encryptionInformation.integrityInformation.segments) { byte[] readBuf = new byte[(int) segment.encryptedSegmentSize]; int bytesRead = tdfReader.readPayloadBytes(readBuf); diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java b/sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java index 40e4f468a..b909c8afc 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java @@ -7,6 +7,7 @@ import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.stream.Collectors; +import java.util.zip.ZipEntry; import static io.opentdf.platform.sdk.TDFWriter.TDF_MANIFEST_FILE_NAME; import static io.opentdf.platform.sdk.TDFWriter.TDF_MANIFEST_FILE_NAME_SPEC; @@ -20,7 +21,13 @@ public class TDFReader { private final ZipReader.Entry manifestEntry; - private final InputStream payload; + private final ZipReader.Entry payloadEntry; + /** + * Opened by the first payload read rather than here, so reading only the manifest never + * allocates an inflater for a deflated payload. Released by {@link #closePayload()}. + */ + private InputStream payload; + private boolean payloadReleased = false; public TDFReader(SeekableByteChannel tdf) throws SDKException, IOException { // A zip may legally list the same name twice, and readers disagree about which copy @@ -46,7 +53,11 @@ public TDFReader(SeekableByteChannel tdf) throws SDKException, IOException { } manifestEntry = manifest; - payload = entries.get(TDF_PAYLOAD_FILE_NAME).getData(); + payloadEntry = entries.get(TDF_PAYLOAD_FILE_NAME); + // check the payload now, so one that can't be read (an unsupported compression method, + // or a deflated size over its budget) is refused before any key is requested -- but + // without opening it, since a caller may only want the manifest + payloadEntry.checkReadable(); } String manifest() { @@ -61,19 +72,79 @@ String manifest() { return out.toString(StandardCharsets.UTF_8); } + /** + * Reads the next payload bytes into {@code buf}, opening the payload on the first call. A + * failed read releases the payload; once it has been released this reads nothing. + */ int readPayloadBytes(byte[] buf) { + if (payloadReleased) { + return 0; + } int totalRead = 0; int nread; + boolean succeeded = false; try { + if (payload == null) { + payload = payloadEntry.getData(); + } while (totalRead < buf.length && (nread = payload.read(buf, totalRead, buf.length - totalRead)) >= 0) { totalRead += nread; } + succeeded = true; } catch (IOException e) { throw new SDKException("error reading from payload in TDF", e); + } finally { + if (!succeeded) { + closePayload(); + } } return totalRead; } + /** + * Called once every segment has been read. A deflated payload must end exactly there: a + * further read has to find the end of the entry, which for a deflated entry also confirms + * the deflate stream ends at its declared size. A stored payload is not checked, as before. + * Releases the payload either way. + * + * @throws InvalidZipException if a deflated payload holds data past its last segment + */ + void finishPayload() { + try { + if (payloadEntry.getCompressionMethod() == ZipEntry.DEFLATED && !payloadReleased) { + if (payload == null) { + payload = payloadEntry.getData(); + } + if (payload.read() != -1) { + throw new InvalidZipException("deflated entry [" + payloadEntry.getName() + + "] holds data past the end of the TDF's last segment"); + } + } + } catch (IOException e) { + throw new SDKException("error reading from payload in TDF", e); + } finally { + closePayload(); + } + } + + /** + * Releases the payload stream, and with it a deflated payload's inflater. Idempotent; after + * it, payload reads return nothing. + */ + void closePayload() { + payloadReleased = true; + if (payload == null) { + return; + } + try { + payload.close(); + } catch (IOException e) { + throw new SDKException("error closing payload in TDF", e); + } finally { + payload = null; + } + } + PolicyObject readPolicyObject() { String manifestJson = manifest(); Manifest manifest = Manifest.readManifest(manifestJson); diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java index 9e038434c..a8571ac2a 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/ZipReader.java @@ -13,6 +13,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.concurrent.atomic.AtomicInteger; import java.util.zip.DataFormatException; import java.util.zip.Inflater; @@ -109,6 +110,13 @@ public CentralDirectoryRecord(long numEntries, long offsetToStart) { private static final int STORED = 0; /** Compression method 8: raw DEFLATE (RFC 1951), which some other TDF writers use. */ private static final int DEFLATED = 8; + /** + * The least uncompressed size a deflated entry may declare, however small the archive: 10 MiB. + * Past it, an entry may declare no more than the archive's length. A deflated TDF payload is + * ciphertext, which does not compress, so it never declares more than the archive holds; + * the floor leaves room for a manifest that compresses well. + */ + private static final long DEFLATED_SIZE_BUDGET_FLOOR = 10L * 1024 * 1024; CentralDirectoryRecord readEndOfCentralDirectory() throws IOException { long eoCDRStart = zipChannel.size() - END_OF_CENTRAL_DIRECTORY_SIZE; // 22 is the minimum size of the EOCDR @@ -259,31 +267,62 @@ private long findStartOfData() throws IOException { * directory declares. Any other compression method is refused here, so an archive whose * other entries use one can still be listed and those entries left unread. *

+ * A deflated entry may not declare an uncompressed size larger than the larger of the + * archive's length and 10 MiB. Inflation never passes the declared size, so this is what + * bounds how far a small archive can expand. + *

* Entries share the archive's channel. Each stream positions the channel itself on every * read, so streams over different entries may be interleaved, but a stream must not be - * used from more than one thread at a time. + * used from more than one thread at a time. Close a deflated entry's stream to release its + * inflater. * * @return the entry's uncompressed bytes * @throws InvalidZipException if the entry uses a compression method other than stored - * or deflate, or its local header is invalid. A deflated stream also throws it from - * {@code read} when its data is corrupt or does not inflate to exactly the declared - * uncompressed size. + * or deflate, declares a deflated size over its budget, or its local header is + * invalid. A deflated stream also throws it from {@code read} when its data is + * corrupt or does not inflate to exactly the declared uncompressed size. */ public InputStream getData() throws IOException { + long startOfData = checkReadable(); + if (compressionMethod == DEFLATED) { + return new InflatingInputStream(channelStream(startOfData, compressedSize), fileSize, fileName); + } + return channelStream(startOfData, fileSize); + } + + /** + * Makes every check {@link #getData()} makes, without opening a stream or allocating an + * inflater, so a reader can refuse an entry it will only read later. + * + * @return the offset of the entry's first byte of data + * @throws InvalidZipException as {@link #getData()} does before it returns a stream + */ + long checkReadable() throws IOException { switch (compressionMethod) { case STORED: - return channelStream(findStartOfData(), fileSize); + break; case DEFLATED: - // zip64 sizes are read as signed longs, so one at or above 2^63 arrives negative - if (compressedSize < 0 || fileSize < 0) { - throw new InvalidZipException("entry [" + fileName + "] declares a size out of range: compressed " - + Long.toUnsignedString(compressedSize) + ", uncompressed " + Long.toUnsignedString(fileSize)); - } - return new InflatingInputStream(channelStream(findStartOfData(), compressedSize), fileSize, fileName); + checkDeflatedSizes(); + break; default: throw new InvalidZipException("unsupported compression method " + compressionMethod + " for entry [" + fileName + "]; only stored (0) and deflate (8) are supported"); } + return findStartOfData(); + } + + private void checkDeflatedSizes() throws IOException { + // zip64 sizes are read as signed longs, so one at or above 2^63 arrives negative + if (compressedSize < 0 || fileSize < 0) { + throw new InvalidZipException("entry [" + fileName + "] declares a size out of range: compressed " + + Long.toUnsignedString(compressedSize) + ", uncompressed " + Long.toUnsignedString(fileSize)); + } + long budget = Math.max(zipChannel.size(), DEFLATED_SIZE_BUDGET_FLOOR); + if (fileSize > budget) { + throw new InvalidZipException("deflated entry [" + fileName + "] declares an uncompressed size of " + + fileSize + " bytes, more than its budget of " + budget + + " bytes (the larger of the archive's length and " + DEFLATED_SIZE_BUDGET_FLOOR + " bytes)"); + } } /** @@ -415,6 +454,24 @@ public List getEntries() { return entries; } + /** Inflaters created by {@link Entry#getData()}, across every reader in this JVM. */ + private static final AtomicInteger INFLATERS_CREATED = new AtomicInteger(); + /** Inflaters created and not yet released by closing their stream. */ + private static final AtomicInteger OPEN_INFLATERS = new AtomicInteger(); + + /** + * How many inflaters deflated entries have created. An inflater holds native memory until it + * is released, so the TDF reader's tests use this to check it creates them only when needed. + */ + static int inflatersCreated() { + return INFLATERS_CREATED.get(); + } + + /** How many inflaters are currently allocated: created, and their stream not yet closed. */ + static int openInflaters() { + return OPEN_INFLATERS.get(); + } + /** * Inflates a raw DEFLATE stream to exactly the uncompressed size the central directory * declares. Output is produced only as it is read, so a large entry is never held in memory, @@ -443,6 +500,8 @@ private static final class InflatingInputStream extends InputStream { this.compressed = compressed; this.uncompressedSize = uncompressedSize; this.entryName = entryName; + INFLATERS_CREATED.incrementAndGet(); + OPEN_INFLATERS.incrementAndGet(); } @Override @@ -549,6 +608,7 @@ public void close() throws IOException { if (!closed) { closed = true; inflater.end(); + OPEN_INFLATERS.decrementAndGet(); compressed.close(); } } diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/TDFReaderTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/TDFReaderTest.java index 0e887bdd2..b5b9bc879 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFReaderTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFReaderTest.java @@ -240,4 +240,99 @@ void rejectsAManifestWithAnUnsupportedCompressionMethod() throws IOException { .hasMessageContaining("unsupported compression method 12"); } } + + /** + * The manifest is read whole into memory before it is validated, so a tiny archive whose + * deflated manifest declares 16 MiB is refused on its declaration, before anything is + * inflated. + */ + @Test + void rejectsADeflatedManifestBombBeforeInflatingIt() throws IOException { + byte[] archive = ZipReaderTest.zipOf(Zip64Mode.Never, + ZipReaderTest.stored(TDFWriter.TDF_PAYLOAD_FILE_NAME, PAYLOAD.getBytes(StandardCharsets.UTF_8)), + ZipReaderTest.deflated("manifest.json", new byte[16 * 1024 * 1024])); + assertThat(archive.length).isLessThan(64 * 1024); + int inflatersBefore = ZipReader.inflatersCreated(); + try (var tdf = new SeekableInMemoryByteChannel(archive)) { + assertThatThrownBy(() -> SDK.readManifest(tdf)) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[manifest.json]") + .hasMessageContaining("budget"); + } + assertThat(ZipReader.inflatersCreated()).isEqualTo(inflatersBefore); + } + + /** + * A deflated payload over the budget is refused when the reader is built, before a key is + * requested. Real payloads are ciphertext, which does not compress, so a legitimate one never + * declares more than the archive holds. + */ + @Test + void rejectsADeflatedPayloadDeclaringMoreThanTheBudget() throws IOException { + byte[] archive = ZipReaderTest.zipOf(Zip64Mode.Never, + ZipReaderTest.deflated(TDFWriter.TDF_PAYLOAD_FILE_NAME, new byte[11 * 1024 * 1024]), + ZipReaderTest.stored("manifest.json", MANIFEST.getBytes(StandardCharsets.UTF_8))); + try (var tdf = new SeekableInMemoryByteChannel(archive)) { + assertThatThrownBy(() -> new TDFReader(tdf)) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[" + TDFWriter.TDF_PAYLOAD_FILE_NAME + "]") + .hasMessageContaining("uncompressed size of " + (11 * 1024 * 1024) + " bytes") + .hasMessageContaining("budget of " + (10 * 1024 * 1024) + " bytes"); + } + } + + /** + * Reading only the manifest never opens the payload, so a deflated payload gets no inflater. + * The manifest here is stored, so any inflater at all would have to be the payload's. + */ + @Test + void manifestOnlyReadsDoNotCreateAPayloadInflater() throws IOException { + byte[] archive = ZipReaderTest.zipOf(Zip64Mode.Never, + ZipReaderTest.deflated(TDFWriter.TDF_PAYLOAD_FILE_NAME, ZipReaderTest.compressibleBytes(50_000)), + ZipReaderTest.stored("manifest.json", MANIFEST.getBytes(StandardCharsets.UTF_8))); + int inflatersBefore = ZipReader.inflatersCreated(); + try (var tdf = new SeekableInMemoryByteChannel(archive)) { + var reader = new TDFReader(tdf); + assertThat(reader.manifest()).isEqualTo(MANIFEST); + assertThat(reader.manifest()).isEqualTo(MANIFEST); + } + assertThat(ZipReader.inflatersCreated()).isEqualTo(inflatersBefore); + } + + /** + * Once the payload has been read to its end it is released, and a failed read releases it + * too. A deflated manifest's inflater is released as soon as the manifest has been read. + */ + @Test + void releasesInflatersWhenThePayloadIsFinishedOrFails() throws IOException { + byte[] payload = ZipReaderTest.compressibleBytes(50_000); + byte[] archive = ZipReaderTest.zipOf(Zip64Mode.Never, + ZipReaderTest.deflated(TDFWriter.TDF_PAYLOAD_FILE_NAME, payload), + ZipReaderTest.deflated("manifest.json", MANIFEST.getBytes(StandardCharsets.UTF_8))); + int openBefore = ZipReader.openInflaters(); + + try (var tdf = new SeekableInMemoryByteChannel(archive)) { + var reader = new TDFReader(tdf); + assertThat(reader.manifest()).isEqualTo(MANIFEST); + assertThat(ZipReader.openInflaters()).isEqualTo(openBefore); + + assertThat(reader.readPayloadBytes(new byte[payload.length])).isEqualTo(payload.length); + assertThat(ZipReader.openInflaters()).isEqualTo(openBefore + 1); + reader.finishPayload(); + assertThat(ZipReader.openInflaters()).isEqualTo(openBefore); + } + + // declares one byte fewer than it holds, so the read that reaches the end fails + byte[] corrupt = ZipReaderTest.withDeclaredUncompressedSize( + ZipReaderTest.zipOf(Zip64Mode.Never, + ZipReaderTest.deflated(TDFWriter.TDF_PAYLOAD_FILE_NAME, payload), + ZipReaderTest.stored("manifest.json", MANIFEST.getBytes(StandardCharsets.UTF_8))), + TDFWriter.TDF_PAYLOAD_FILE_NAME, payload.length - 1L); + try (var tdf = new SeekableInMemoryByteChannel(corrupt)) { + var reader = new TDFReader(tdf); + var buf = new byte[payload.length]; + assertThatThrownBy(() -> reader.readPayloadBytes(buf)).isInstanceOf(InvalidZipException.class); + assertThat(ZipReader.openInflaters()).isEqualTo(openBefore); + } + } } diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java index 773f37952..5b2bc4199 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java @@ -40,6 +40,7 @@ import java.util.concurrent.Future; import java.util.function.Consumer; import java.util.function.Predicate; +import java.util.function.UnaryOperator; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.zip.CRC32; @@ -976,6 +977,13 @@ public void testDecryptingATDFWithDeflatedEntries(String manifestName, boolean d */ private static byte[] repackWithDeflate(byte[] tdfBytes, String manifestName, boolean deflateManifest, boolean deflatePayload) throws IOException { + return repackWithDeflate(tdfBytes, manifestName, deflateManifest, deflatePayload, UnaryOperator.identity()); + } + + /** As above, with the payload bytes passed through {@code editPayload} on the way. */ + private static byte[] repackWithDeflate(byte[] tdfBytes, String manifestName, boolean deflateManifest, + boolean deflatePayload, UnaryOperator editPayload) + throws IOException { final byte[] manifest; final byte[] payload; try (var channel = new SeekableInMemoryByteChannel(tdfBytes)) { @@ -986,12 +994,160 @@ private static byte[] repackWithDeflate(byte[] tdfBytes, String manifestName, bo var out = new ByteArrayOutputStream(); try (var zip = new ZipOutputStream(out)) { - putZipEntry(zip, TDFWriter.TDF_PAYLOAD_FILE_NAME, payload, deflatePayload); + putZipEntry(zip, TDFWriter.TDF_PAYLOAD_FILE_NAME, editPayload.apply(payload), deflatePayload); putZipEntry(zip, manifestName, manifest, deflateManifest); } return out.toByteArray(); } + /** A single-segment TDF of {@code plaintext}, written by this SDK. */ + private static byte[] smallTdf(TDF tdf, String plaintext) throws Exception { + var out = new ByteArrayOutputStream(); + tdf.createTDF(new ByteArrayInputStream(plaintext.getBytes(StandardCharsets.UTF_8)), out, + Config.newTDFConfig( + Config.withAutoconfigure(false), + Config.withKasInformation(getRSAKASInfos()))); + return out.toByteArray(); + } + + private static byte[] withByteAppended(byte[] bytes, byte extra) { + byte[] longer = Arrays.copyOf(bytes, bytes.length + 1); + longer[bytes.length] = extra; + return longer; + } + + private TDF tdfWithTestKas() { + return new TDF(new FakeServicesBuilder().setKas(kas) + .setKeyAccessServerRegistryService(kasRegistryService).build()); + } + + /** + * readPayload reads exactly the segment lengths the manifest lists, so by itself it never + * looks past them. A deflated payload has to end there. Here the entry inflates to the real + * ciphertext plus one extra byte, so every segment hash still verifies. Declaring the + * ciphertext's length leaves the extra byte in the deflate stream past the declared size; + * declaring one more leaves it inside the entry but past the last segment. Both are refused + * on the normal read path. + */ + @ParameterizedTest + @ValueSource(ints = { 0, 1 }) + public void testDeflatedPayloadWithDataPastTheLastSegmentIsRejected(int extraDeclared) throws Exception { + var tdf = tdfWithTestKas(); + byte[] original = smallTdf(tdf, "hello"); + int ciphertextLength; + try (var channel = new SeekableInMemoryByteChannel(original)) { + ciphertextLength = readZipEntry(new ZipReader(channel), TDFWriter.TDF_PAYLOAD_FILE_NAME).size(); + } + byte[] withTail = repackWithDeflate(original, TDFWriter.TDF_MANIFEST_FILE_NAME, false, true, + payload -> withByteAppended(payload, (byte) '!')); + byte[] tampered = ZipReaderTest.withDeclaredUncompressedSize( + withTail, TDFWriter.TDF_PAYLOAD_FILE_NAME, (long) ciphertextLength + extraDeclared); + + var reader = tdf.loadTDF(new SeekableInMemoryByteChannel(tampered), platformUrl); + var unwrapped = new ByteArrayOutputStream(); + assertThatThrownBy(() -> reader.readPayload(unwrapped)) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[" + TDFWriter.TDF_PAYLOAD_FILE_NAME + "]"); + } + + /** A stored payload keeps its existing behavior: nothing past the last segment is examined. */ + @Test + public void testStoredPayloadWithDataPastTheLastSegmentIsStillRead() throws Exception { + var tdf = tdfWithTestKas(); + byte[] withTail = repackWithDeflate(smallTdf(tdf, "hello"), TDFWriter.TDF_MANIFEST_FILE_NAME, false, false, + payload -> withByteAppended(payload, (byte) '!')); + + var unwrapped = new ByteArrayOutputStream(); + tdf.loadTDF(new SeekableInMemoryByteChannel(withTail), platformUrl).readPayload(unwrapped); + + assertThat(unwrapped.toString(StandardCharsets.UTF_8)).isEqualTo("hello"); + } + + /** + * The payload is opened by the first payload read and released once it has been read and + * verified, so reading only the manifest never creates its inflater. The manifest is stored + * here, so every inflater counted is the payload's. + */ + @Test + public void testPayloadInflaterExistsOnlyWhileThePayloadIsRead() throws Exception { + var data = new byte[(int) (Config.MIN_SEGMENT_SIZE * 2.5)]; + new Random(99).nextBytes(data); + var tdf = tdfWithTestKas(); + var original = new ByteArrayOutputStream(); + tdf.createTDF(new ByteArrayInputStream(data), original, + Config.newTDFConfig( + Config.withAutoconfigure(false), + Config.withKasInformation(getRSAKASInfos()), + Config.withSegmentSize(Config.MIN_SEGMENT_SIZE))); + byte[] repacked = repackWithDeflate(original.toByteArray(), "manifest.json", false, true); + int createdBefore = ZipReader.inflatersCreated(); + int openBefore = ZipReader.openInflaters(); + + SDK.readManifest(new SeekableInMemoryByteChannel(repacked)); + var reader = tdf.loadTDF(new SeekableInMemoryByteChannel(repacked), platformUrl); + assertThat(ZipReader.inflatersCreated()) + .withFailMessage("reading the manifest and loading the TDF created a payload inflater") + .isEqualTo(createdBefore); + + List openWhileWriting = new ArrayList<>(); + var unwrapped = new ByteArrayOutputStream() { + @Override + public synchronized void write(byte[] b, int off, int len) { + openWhileWriting.add(ZipReader.openInflaters()); + super.write(b, off, len); + } + }; + reader.readPayload(unwrapped); + + assertThat(unwrapped.toByteArray()).containsExactly(data); + assertThat(openWhileWriting).isNotEmpty().allMatch(open -> open == openBefore + 1); + assertThat(ZipReader.inflatersCreated()).isEqualTo(createdBefore + 1); + assertThat(ZipReader.openInflaters()) + .withFailMessage("the payload inflater was not released after the payload was read") + .isEqualTo(openBefore); + } + + /** A load that fails never opens the payload, and releases the deflated manifest's inflater. */ + @Test + public void testFailedLoadLeavesNoInflaterBehind() throws Exception { + var tdf = tdfWithTestKas(); + byte[] repacked = repackWithDeflate(smallTdf(tdf, "hello"), "manifest.json", true, true); + var readerConfig = Config.newTDFReaderConfig(Config.WithKasAllowlist("https://kas.invalid")); + int createdBefore = ZipReader.inflatersCreated(); + int openBefore = ZipReader.openInflaters(); + + try (var channel = new SeekableInMemoryByteChannel(repacked)) { + assertThatThrownBy(() -> tdf.loadTDF(channel, readerConfig)).isInstanceOf(SDKException.class); + } + + assertThat(ZipReader.inflatersCreated()) + .withFailMessage("only the manifest should have been inflated") + .isEqualTo(createdBefore + 1); + assertThat(ZipReader.openInflaters()).isEqualTo(openBefore); + } + + /** + * A payload read that fails part way releases the payload inflater. Flipping a ciphertext bit + * fails the segment's integrity check or its decryption, depending on the segment hash + * algorithm; either way the read fails. + */ + @Test + public void testPayloadInflaterIsReleasedWhenAReadFails() throws Exception { + var tdf = tdfWithTestKas(); + byte[] corrupted = repackWithDeflate(smallTdf(tdf, "hello"), "manifest.json", false, true, payload -> { + byte[] copy = payload.clone(); + copy[copy.length / 2] ^= 1; + return copy; + }); + int openBefore = ZipReader.openInflaters(); + + var reader = tdf.loadTDF(new SeekableInMemoryByteChannel(corrupted), platformUrl); + var unwrapped = new ByteArrayOutputStream(); + assertThatThrownBy(() -> reader.readPayload(unwrapped)).isInstanceOf(SDKException.class); + + assertThat(ZipReader.openInflaters()).isEqualTo(openBefore); + } + private static void putZipEntry(ZipOutputStream zip, String name, byte[] content, boolean deflate) throws IOException { var entry = new ZipEntry(name); diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java index 0ff1577ce..e197a20f3 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/ZipReaderTest.java @@ -524,6 +524,60 @@ public void testZipBombIsStoppedAtTheDeclaredSize() throws IOException { assertThat(delivered.size()).isEqualTo(1024); } + /** The floor of the deflated size budget: 10 MiB. */ + private static final int BUDGET_FLOOR = 10 * 1024 * 1024; + + /** + * Inflation stops at the declared size, so the declaration itself has a budget: a deflated + * entry may not claim more than the larger of the archive's length and 10 MiB. 16 MiB of zeros + * deflates to about 16 KiB, so without that budget this archive would expand a thousandfold. + * It is refused before a stream exists, so nothing is inflated. + */ + @Test + public void testDeflatedEntryDeclaringMoreThanTheBudgetIsRejectedBeforeInflating() throws IOException { + byte[] bomb = new byte[16 * 1024 * 1024]; + byte[] archive = zipOf(Zip64Mode.Never, deflated("manifest.json", bomb)); + assertThat(archive.length).isLessThan(64 * 1024); + var entry = entryNamed(new ZipReader(new SeekableInMemoryByteChannel(archive)), "manifest.json"); + int inflatersBefore = ZipReader.inflatersCreated(); + + assertThatThrownBy(entry::getData) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[manifest.json]") + .hasMessageContaining("uncompressed size of " + bomb.length + " bytes") + .hasMessageContaining("budget of " + BUDGET_FLOOR + " bytes"); + assertThat(ZipReader.inflatersCreated()).isEqualTo(inflatersBefore); + } + + /** A small archive may still hold a deflated entry right up to the floor. */ + @Test + public void testDeflatedEntryDeclaringExactlyTheBudgetFloorIsRead() throws IOException { + byte[] zeros = new byte[BUDGET_FLOOR]; + byte[] archive = zipOf(Zip64Mode.Never, deflated("entry", zeros)); + assertThat(archive.length).isLessThan(BUDGET_FLOOR); + + assertThat(readEntryBytes(new ZipReader(new SeekableInMemoryByteChannel(archive)), "entry")).isEqualTo(zeros); + } + + /** Past the floor the budget is the archive's length, however the archive came to be that long. */ + @Test + public void testDeflatedSizeBudgetGrowsWithTheArchive() throws IOException { + byte[] padding = randomBytes(BUDGET_FLOOR + 1024 * 1024, 3); + byte[] zeros = new byte[BUDGET_FLOOR + 512 * 1024]; + byte[] archive = zipOf(Zip64Mode.Never, stored("padding", padding), deflated("entry", zeros)); + assertThat(archive.length).isGreaterThan(zeros.length); + + assertThat(readEntryBytes(new ZipReader(new SeekableInMemoryByteChannel(archive)), "entry")).isEqualTo(zeros); + + byte[] overBudget = withDeclaredUncompressedSize(archive, "entry", archive.length + 1L); + var entry = entryNamed(new ZipReader(new SeekableInMemoryByteChannel(overBudget)), "entry"); + assertThatThrownBy(entry::getData) + .isInstanceOf(InvalidZipException.class) + .hasMessageContaining("[entry]") + .hasMessageContaining("uncompressed size of " + (archive.length + 1L) + " bytes") + .hasMessageContaining("budget of " + archive.length + " bytes"); + } + @ParameterizedTest @ValueSource(longs = { 1, 2, 5_000 }) public void testDeflatedEntrySmallerThanDeclaredIsRejected(long extra) throws IOException { @@ -804,7 +858,7 @@ static byte[] withCompressionMethod(byte[] archive, String name, int method) { } /** Rewrites the uncompressed size in the central directory, which is the one the reader trusts. */ - private static byte[] withDeclaredUncompressedSize(byte[] archive, String name, long size) { + static byte[] withDeclaredUncompressedSize(byte[] archive, String name, long size) { byte[] patched = archive.clone(); ByteBuffer.wrap(patched).order(ByteOrder.LITTLE_ENDIAN) .putInt(centralDirectoryHeaderOf(patched, name) + CDH_UNCOMPRESSED_SIZE, (int) size);