diff --git a/data/src/main/java/com/google/maps/android/data/parser/kml/KmzParser.kt b/data/src/main/java/com/google/maps/android/data/parser/kml/KmzParser.kt index 9d3bc90d6..336a2d86f 100644 --- a/data/src/main/java/com/google/maps/android/data/parser/kml/KmzParser.kt +++ b/data/src/main/java/com/google/maps/android/data/parser/kml/KmzParser.kt @@ -91,7 +91,7 @@ class KmzParser( @Throws(IOException::class) private fun checkLimit() { if (mTotalBytes > mMaxBytes) { - throw java.io.IOException("Zip bomb detected! Uncompressed size exceeds limit of $mMaxBytes bytes.") + throw IOException("Zip bomb detected! Uncompressed size exceeds limit of $mMaxBytes bytes.") } } } @@ -114,24 +114,31 @@ class KmzParser( while (entry != null) { entryCount++ if (entryCount > maxKmzEntryCount) { - throw java.io.IOException("Zip bomb detected! Max number of entries exceeded: $maxKmzEntryCount") + throw IOException("Zip bomb detected! Max number of entries exceeded: $maxKmzEntryCount") } val name = entry.name - if (!entry.isDirectory) { - if (name.endsWith(".kml", ignoreCase = true) && kml == null) { - // Found the KML file (first one found is usually the main one in KMZ) - // We need to read it into a byte array because we can't close the ZipInputStream yet - val bytes = countingStream.readBytes() - kml = KmlParser().parse(ByteArrayInputStream(bytes)) - } else { - // Try to decode as image - val bytes = countingStream.readBytes() - val bitmap = imageDecoder.decode(bytes) - if (bitmap != null) { - images[name] = bitmap - } + if (entry.isDirectory) { + // Directory entries in a ZIP archive must never carry a data payload. + // Reading even a single byte detects rogue payloads without decompressing large streams. + if (countingStream.read() != -1) { + throw IOException("Zip bomb or malformed KMZ detected: directory entry '$name' contains unexpected data payload.") + } + } else if (name.endsWith(".kml", ignoreCase = true) && kml == null) { + // Found the KML file (first one found is usually the main one in KMZ) + // We need to read it into a byte array because we can't close the ZipInputStream yet + val bytes = countingStream.readBytes() + kml = KmlParser().parse(ByteArrayInputStream(bytes)) + } else { + // Try to decode as image + val bytes = countingStream.readBytes() + val bitmap = imageDecoder.decode(bytes) + if (bitmap != null) { + images[name] = bitmap } } + // Drain any unread bytes through countingStream before closing entry to ensure + // ZipInputStream.closeEntry() never bypasses cumulative byte accounting. + drainEntry(countingStream) zipInputStream.closeEntry() entry = zipInputStream.nextEntry } @@ -146,7 +153,20 @@ class KmzParser( return kml.copy(images = images) } + /** + * Drains any remaining bytes from the current zip entry through the counting stream, + * ensuring all inflated bytes count towards the uncompressed size limit. + */ + @Throws(IOException::class) + private fun drainEntry(stream: InputStream) { + val buffer = ByteArray(DRAIN_BUFFER_SIZE) + while (stream.read(buffer) != -1) { + // CountingInputStream counts bytes read and enforces maxKmzUncompressedTotalSize limit. + } + } + companion object { + private const val DRAIN_BUFFER_SIZE = 8192 val SUPPORTED_EXTENSIONS = setOf("kmz") fun canParse(header: String): Boolean { diff --git a/data/src/test/java/com/google/maps/android/data/parser/kml/KmzParserTest.kt b/data/src/test/java/com/google/maps/android/data/parser/kml/KmzParserTest.kt index a993da197..13e939ac6 100644 --- a/data/src/test/java/com/google/maps/android/data/parser/kml/KmzParserTest.kt +++ b/data/src/test/java/com/google/maps/android/data/parser/kml/KmzParserTest.kt @@ -23,8 +23,10 @@ import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test +import org.junit.Assert.assertThrows import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream +import java.io.IOException import java.util.zip.ZipEntry import java.util.zip.ZipOutputStream @@ -70,6 +72,78 @@ class KmzParserTest { assertEquals(mockBitmap, kml.images["image.png"]) } + /** + * Demonstrates issue #1790: KMZ directory entries bypass the cumulative decompression limit. + * + * A malicious KMZ can package a large compressed payload inside an entry named with a trailing slash + * (e.g., "padding/"). Without proper accounting or validation, ZipInputStream.closeEntry() silently + * inflates and drains the entry without incrementing the uncompressed byte counter, bypassing + * the configured decompression budget. + */ + @Test + fun `parse rejects directory entry with payload exceeding decompression limit`() { + val kmlContent = + """ + + + + Test Placemark + + + + """.trimIndent() + + // 2,000 bytes payload in a directory entry, configured with a 1,000-byte limit + val directoryPayload = ByteArray(2000) { 0x41 } + val kmzStream = + createKmzStream( + "padding/" to directoryPayload, + "doc.kml" to kmlContent.toByteArray(), + ) + + val parser = KmzParser(maxKmzUncompressedTotalSize = 1000L) + + assertThrows(IOException::class.java) { + parser.parse(kmzStream) + } + } + + /** + * Verifies that standard well-formed KMZ files containing empty directory entries + * (e.g. "images/") parse successfully without false-positive zip bomb rejections. + */ + @Test + fun `parse allows legitimate empty directory entries`() { + val kmlContent = + """ + + + + Doc in Dir + + + + """.trimIndent() + + val kmzStream = + createKmzStream( + "images/" to ByteArray(0), + "doc.kml" to kmlContent.toByteArray(), + ) + + val parser = KmzParser() + val kml = parser.parse(kmzStream) + + assertNotNull(kml.document) + assertEquals( + "Doc in Dir", + kml.document + ?.placemarks + ?.first() + ?.name, + ) + } + private fun createKmzStream(vararg entries: kotlin.Pair): ByteArrayInputStream { val baos = ByteArrayOutputStream() val zos = ZipOutputStream(baos)