android, desktop: close fail-closed gaps in metadata strippers (PR review)

- MP4 processContainer: throw on trailing bytes inside a container box (would
  shrink moov and stale stco/co64 offsets)
- AVI processHeaderChunks + main loop: throw on negative chunkSize / overrun
  instead of silently breaking (would truncate output and skip metadata)
- GIF: bounds-check GCT, Image Descriptor, LCT, LZW min-code-size reads so
  truncated input fails with IllegalArgumentException, not IndexOutOfBounds
- WebP: throw on VP8X with chunkSize < 10 (would land flag-write on next
  chunk's FourCC and corrupt output)

6 regression tests covering each gap.

apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/platform/MetadataStrip.kt

@@ -170,6 +170,7 @@ fun stripGifMetadata(data: ByteArray): ByteArray {
   val hasGCT = (packed and 0x80) != 0
   val gctSize = if (hasGCT) 3 * (1 shl ((packed and 0x07) + 1)) else 0
   if (hasGCT) {
+    if (13 + gctSize > data.size) throw IllegalArgumentException("Invalid GIF: GCT (size $gctSize) extends past file (size ${data.size})")
     out.write(data, 13, gctSize)
   }
 
@@ -256,6 +257,7 @@ private fun skipExtensionBlock(data: ByteArray, start: Int): Int =
 /** Copy an Image Descriptor block + optional Local Color Table + image data. Returns new position. */
 private fun copyImageDescriptor(data: ByteArray, start: Int, out: java.io.ByteArrayOutputStream): Int {
   // Image Descriptor is 10 bytes (0x2C + 9 bytes)
+  if (start + 10 > data.size) throw IllegalArgumentException("Invalid GIF: truncated Image Descriptor at offset $start")
   out.write(data, start, 10)
   var pos = start + 10
 
@@ -264,11 +266,13 @@ private fun copyImageDescriptor(data: ByteArray, start: Int, out: java.io.ByteAr
   val hasLCT = (packed and 0x80) != 0
   if (hasLCT) {
     val lctSize = 3 * (1 shl ((packed and 0x07) + 1))
+    if (pos + lctSize > data.size) throw IllegalArgumentException("Invalid GIF: LCT (size $lctSize) extends past file at offset $pos")
     out.write(data, pos, lctSize)
     pos += lctSize
   }
 
   // LZW Minimum Code Size
+  if (pos >= data.size) throw IllegalArgumentException("Invalid GIF: truncated before LZW min code size at offset $pos")
   out.write(data[pos].toInt() and 0xFF)
   pos++
 
@@ -315,6 +319,9 @@ fun stripWebPMetadata(data: ByteArray): ByteArray {
     }
 
     if (fourCC == "VP8X") {
+      // Per spec VP8X payload is fixed at 10 bytes. If it's smaller, our flags-byte write
+      // at the post-loop fixup would land on the next chunk's FourCC and corrupt structure.
+      if (chunkSize < 10) throw IllegalArgumentException("Invalid WebP: VP8X chunk too small ($chunkSize bytes, need 10)")
       // +8 to skip the chunk header (fourCC + size), points to the flags byte in the output
       vp8xOffset = out.size() + 8
     }
@@ -606,6 +613,9 @@ fun stripVideoMetadata(inputPath: String, outputPath: String) {
       }
       pos += boxSize
     }
+    // Fail closed if any bytes remain — silently dropping them would shrink the container,
+    // shifting absolute file offsets that stco/co64 in moov reference into mdat.
+    if (pos != end) throw IllegalArgumentException("MP4 container has ${end - pos} trailing byte(s) at end of range $offset..$end")
     return containerOut.toByteArray()
   }
 
@@ -803,10 +813,14 @@ fun stripAviMetadata(inputPath: String, outputPath: String) {
     while (p + 8 <= end) {
       val fourCC = String(d, p, 4, Charsets.US_ASCII)
       val chunkSize = readLE32(d, p + 4)
-      if (chunkSize < 0) break
+      // Fail closed: silent break would shorten the rewritten LIST and could skip past real
+      // metadata chunks that follow a malformed one. Match the MKV-side fail-closed posture.
+      if (chunkSize < 0) throw IllegalArgumentException("AVI header chunk '$fourCC' has negative size at offset $p: $chunkSize")
       val paddedSize = if (chunkSize % 2 == 0) chunkSize.toLong() else chunkSize.toLong() + 1L
       val totalSizeLong = 8L + paddedSize
-      if (totalSizeLong > Int.MAX_VALUE || p + totalSizeLong > end) break
+      if (totalSizeLong > Int.MAX_VALUE || p + totalSizeLong > end) {
+        throw IllegalArgumentException("AVI header chunk '$fourCC' at offset $p overruns container: totalSize=$totalSizeLong, remaining=${end - p}")
+      }
       val totalSize = totalSizeLong.toInt()
 
       if (fourCC == "LIST" && p + 12 <= end) {
@@ -853,7 +867,9 @@ fun stripAviMetadata(inputPath: String, outputPath: String) {
         raf.readFully(chunkHeader)
         val fourCC = String(chunkHeader, 0, 4, Charsets.US_ASCII)
         val chunkSize = readLE32(chunkHeader, 4)
-        if (chunkSize < 0) break
+        // Fail closed: a silent break would truncate the output (RIFF size is rewritten at the end
+        // to match), letting a crafted file pass through with arbitrary tail data silently dropped.
+        if (chunkSize < 0) throw IllegalArgumentException("AVI top-level chunk '$fourCC' has negative size: $chunkSize")
         // Use Long to avoid overflow for odd chunkSize near Int.MAX_VALUE
         val paddedSize = if (chunkSize % 2 == 0) chunkSize.toLong() else chunkSize.toLong() + 1L
 

apps/multiplatform/common/src/commonTest/kotlin/chat/simplex/app/MetadataStripTest.kt

@@ -2259,3 +2259,125 @@ class AnimImageFileDispatchTest {
     }
   }
 }
+
+/** Regression tests for fail-closed gaps flagged in PR #6999 review. */
+class ReviewFailClosedTest {
+
+  // --- GIF bounds checks (review #4, #5) ---
+
+  @Test fun testGifTruncatedGctThrowsIllegalArgument() {
+    // GIF89a header + LSD where the packed byte advertises a GCT (0x80) of 3*2^(0+1) = 6 bytes,
+    // but the file ends right after the LSD. Without the bounds check this throws IOB from
+    // ByteArrayOutputStream.write; with it the stripper throws IllegalArgumentException.
+    val truncated = byteArrayOf(
+      0x47, 0x49, 0x46, 0x38, 0x39, 0x61,           // "GIF89a"
+      0x01, 0x00, 0x01, 0x00,                       // 1x1
+      0x80.toByte(),                                // packed: GCT bit set, size code 0 → 6 bytes
+      0x00, 0x00                                    // bg, aspect
+    )
+    assertFailsWith<IllegalArgumentException> { stripGifMetadata(truncated) }
+  }
+
+  @Test fun testGifTruncatedImageDescriptorThrowsIllegalArgument() {
+    // Header + LSD (no GCT) + image-descriptor sentinel only — descriptor needs 10 bytes total.
+    val truncated = byteArrayOf(
+      0x47, 0x49, 0x46, 0x38, 0x39, 0x61,           // "GIF89a"
+      0x01, 0x00, 0x01, 0x00,                       // 1x1
+      0x00, 0x00, 0x00,                             // no GCT, bg, aspect
+      0x2C, 0x00                                    // image-descriptor sentinel + 1 stray byte
+    )
+    assertFailsWith<IllegalArgumentException> { stripGifMetadata(truncated) }
+  }
+
+  // --- WebP VP8X size check (review #6) ---
+
+  @Test fun testWebpVp8xTooSmallThrowsIllegalArgument() {
+    // VP8X with chunkSize = 0 (spec mandates 10). Without the check, vp8xOffset would land on
+    // the next chunk's FourCC and the post-loop flag-clear would corrupt that byte.
+    val malformed = byteArrayOf(
+      0x52, 0x49, 0x46, 0x46,                       // "RIFF"
+      0x14, 0x00, 0x00, 0x00,                       // file size - 8 = 20
+      0x57, 0x45, 0x42, 0x50,                       // "WEBP"
+      0x56, 0x50, 0x38, 0x58,                       // "VP8X"
+      0x00, 0x00, 0x00, 0x00,                       // chunkSize = 0 (truncated, malformed)
+      0x45, 0x58, 0x49, 0x46,                       // "EXIF" — would be flag-corrupted by bug
+      0x00, 0x00, 0x00, 0x00                        // EXIF chunkSize = 0
+    )
+    assertFailsWith<IllegalArgumentException> { stripWebPMetadata(malformed) }
+  }
+
+  // --- MP4 processContainer trailing bytes (review #1) ---
+
+  @Test fun testMp4ContainerWithTrailingBytesThrowsIllegalArgument() {
+    // moov containing a single trak that DOES NOT TILE — i.e., trak's declared size leaves
+    // a few bytes inside moov unaccounted for. Without the trailing-byte throw, processContainer
+    // silently drops those bytes, shrinking moov and shifting mdat → broken stco/co64.
+    val mvhd = box("mvhd", ByteArray(100))
+    val trak = box("trak", ByteArray(100))
+    val moov = box("moov", mvhd + trak + ByteArray(3))  // 3 trailing bytes inside moov
+    val ftyp = box("ftyp", "isom".toByteArray() + ByteArray(8))
+    val mdat = box("mdat", ByteArray(8))
+
+    val inputFile = File.createTempFile("test_mp4_trailing_", ".mp4")
+    inputFile.writeBytes(ftyp + moov + mdat)
+    val outputFile = File.createTempFile("test_mp4_trailing_out_", ".mp4")
+    try {
+      assertFailsWith<IllegalArgumentException> {
+        stripVideoMetadata(inputFile.absolutePath, outputFile.absolutePath)
+      }
+    } finally {
+      inputFile.delete(); outputFile.delete()
+    }
+  }
+
+  // --- AVI silent break → throw (review #2, #3) ---
+
+  @Test fun testAviTopLevelNegativeChunkSizeThrows() {
+    // Top-level chunk with the high bit set in chunkSize. Previously silent-break, now throws.
+    val data = byteArrayOf(
+      0x52, 0x49, 0x46, 0x46,                                 // "RIFF"
+      0x14, 0x00, 0x00, 0x00,                                 // file size - 8 = 20
+      0x41, 0x56, 0x49, 0x20,                                 // "AVI "
+      0x4A, 0x55, 0x4E, 0x4B,                                 // "JUNK" chunk
+      0xFF.toByte(), 0xFF.toByte(), 0xFF.toByte(), 0xFF.toByte()  // chunkSize = -1 (top bit set)
+    )
+    val inputFile = File.createTempFile("test_avi_neg_", ".avi")
+    inputFile.writeBytes(data)
+    val outputFile = File.createTempFile("test_avi_neg_out_", ".avi")
+    try {
+      assertFailsWith<IllegalArgumentException> {
+        stripAviMetadata(inputFile.absolutePath, outputFile.absolutePath)
+      }
+    } finally {
+      inputFile.delete(); outputFile.delete()
+    }
+  }
+
+  @Test fun testAviHdrlNegativeChunkSizeThrows() {
+    // hdrl LIST containing an inner chunk with negative size. Previously silent-break,
+    // now throws so subsequent _PMX/strd/IDIT metadata can't be silently dropped.
+    val innerChunk = byteArrayOf(
+      0x61, 0x76, 0x69, 0x68,                                 // "avih"
+      0xFF.toByte(), 0xFF.toByte(), 0xFF.toByte(), 0xFF.toByte()  // chunkSize = -1
+    )
+    val hdrlListBody = "hdrl".toByteArray() + innerChunk
+    val hdrlList = byteArrayOf(0x4C, 0x49, 0x53, 0x54) +      // "LIST"
+      byteArrayOf(hdrlListBody.size.toByte(), 0, 0, 0) +      // size
+      hdrlListBody
+    val data = byteArrayOf(
+      0x52, 0x49, 0x46, 0x46,                                 // "RIFF"
+      (12 + hdrlList.size).toByte(), 0, 0, 0,                 // file size - 8
+      0x41, 0x56, 0x49, 0x20                                  // "AVI "
+    ) + hdrlList
+    val inputFile = File.createTempFile("test_avi_hdrl_neg_", ".avi")
+    inputFile.writeBytes(data)
+    val outputFile = File.createTempFile("test_avi_hdrl_neg_out_", ".avi")
+    try {
+      assertFailsWith<IllegalArgumentException> {
+        stripAviMetadata(inputFile.absolutePath, outputFile.absolutePath)
+      }
+    } finally {
+      inputFile.delete(); outputFile.delete()
+    }
+  }
+}