Merge pull request #826 from vincenzopalazzo/fix/file-tlv-forward-compat

Skip unknown file TLVs instead of dropping the transfer

app/src/main/java/com/bitchat/android/model/BitchatFilePacket.kt

@@ -14,6 +14,13 @@ import java.nio.ByteOrder
  * Length field for TLV is 2 bytes (UInt16, big-endian) for all TLVs.
  * For large files, CONTENT is chunked into multiple TLVs of up to 65535 bytes each.
  *
+ * Unknown TLV types are SKIPPED, not rejected: the tag list above is a floor,
+ * not a ceiling, and a decoder that bails on the first tag it does not know
+ * makes the format unextendable — the whole file is lost over a field that,
+ * by construction, the sender considered optional. The iOS client has always
+ * done this (`case nil: continue` in its own decoder), so rejecting here also
+ * meant the two implementations disagreed about what a valid packet is.
+ *
  * Note: The outer BitchatPacket uses version 2 (4-byte payload length), so this
  * TLV payload can exceed 64 KiB even though each TLV value is limited to 65535 bytes.
  * Transport-level fragmentation then splits the final packet for BLE MTU.
@@ -26,7 +33,15 @@ data class BitchatFilePacket(
 ) {
     private enum class TLVType(val v: UByte) {
         FILE_NAME(0x01u), FILE_SIZE(0x02u), MIME_TYPE(0x03u), CONTENT(0x04u);
-        companion object { fun from(value: UByte) = values().find { it.v == value } }
+        companion object {
+            fun from(value: UByte): TLVType? = when (value) {
+                FILE_NAME.v -> FILE_NAME
+                FILE_SIZE.v -> FILE_SIZE
+                MIME_TYPE.v -> MIME_TYPE
+                CONTENT.v -> CONTENT
+                else -> null
+            }
+        }
     }
 
     fun encode(): ByteArray? {
@@ -90,8 +105,15 @@ data class BitchatFilePacket(
                 var size: Long? = null
                 var mime: String? = null
                 var contentBytes: ByteArray? = null
-                while (off + 3 <= data.size) { // minimum TLV header size (type + 2 bytes length)
-                    val t = TLVType.from(data[off].toUByte()) ?: return null
+                var skippedUnknownTLVs = 0
+                while (off < data.size) {
+                    // Every TLV needs at least a type and a 2-byte length.
+                    // Reject a truncated trailing header instead of silently
+                    // accepting it, matching the iOS decoder.
+                    if (data.size - off < 3) return null
+                    // A null `t` is an unknown tag: read its length like any
+                    // other 2-byte TLV and skip its value, matching iOS.
+                    val t = TLVType.from(data[off].toUByte())
                     off += 1
                     // CONTENT uses 4-byte length; others use 2-byte length
                     val len: Int
@@ -105,6 +127,18 @@ data class BitchatFilePacket(
                         off += 2
                     }
                     if (len < 0 || off + len > data.size) return null
+                    if (t == null) {
+                        // Unknown tag: advance past the value without copying it
+                        // and without logging. A peer can pad a packet with
+                        // zero-length unknown TLVs — 3 bytes each — so anything
+                        // per-TLV here is attacker-scaled: at the payload
+                        // ceiling that is millions of copies and formatted log
+                        // lines monopolising the mesh handler. Counted and
+                        // reported once after the loop instead.
+                        off += len
+                        skippedUnknownTLVs += 1
+                        continue
+                    }
                     val value = data.copyOfRange(off, off + len)
                     off += len
                     when (t) {
@@ -124,6 +158,9 @@ data class BitchatFilePacket(
                         }
                     }
                 }
+                if (skippedUnknownTLVs > 0) {
+                    android.util.Log.d("BitchatFilePacket", "⏭️ Skipped $skippedUnknownTLVs unknown TLV(s)")
+                }
                 val n = name ?: return null
                 val c = contentBytes ?: return null
                 val s = size ?: c.size.toLong()
@@ -138,4 +175,3 @@ data class BitchatFilePacket(
         }
     }
 }
-

app/src/test/kotlin/com/bitchat/FileTransferTest.kt

@@ -5,6 +5,7 @@ import com.bitchat.android.model.BitchatMessage
 import com.bitchat.android.model.BitchatMessageType
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertNull
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
@@ -139,6 +140,116 @@ class FileTransferTest {
         assertEquals(32L, decoded.fileSize)
     }
 
+    @Test
+    fun `decode should skip unknown TLV types instead of dropping the whole file`() {
+        // Given: a packet from a peer that added one TLV this build does not
+        // know (a message id, tag 0x05), placed before CONTENT the way an
+        // encoder that appends content last would emit it.
+        val content = ByteArray(64) { (it % 256).toByte() }
+        val fileName = "photo.jpg".toByteArray(Charsets.UTF_8)
+        val mimeType = "image/jpeg".toByteArray(Charsets.UTF_8)
+        val unknownValue = "some-message-id".toByteArray(Charsets.UTF_8)
+
+        val buf = ByteBuffer.allocate(
+            (1 + 2 + fileName.size) + (1 + 2 + 4) + (1 + 2 + mimeType.size) +
+                (1 + 2 + unknownValue.size) + (1 + 4 + content.size)
+        ).order(ByteOrder.BIG_ENDIAN)
+        buf.put(0x01.toByte()); buf.putShort(fileName.size.toShort()); buf.put(fileName)
+        buf.put(0x02.toByte()); buf.putShort(4); buf.putInt(content.size)
+        buf.put(0x03.toByte()); buf.putShort(mimeType.size.toShort()); buf.put(mimeType)
+        buf.put(0x05.toByte()); buf.putShort(unknownValue.size.toShort()); buf.put(unknownValue)
+        buf.put(0x04.toByte()); buf.putInt(content.size); buf.put(content)
+
+        // When: Decoding
+        val decoded = BitchatFilePacket.decode(buf.array())
+
+        // Then: the unknown TLV costs nothing — the media still arrives.
+        // Rejecting it made every such transfer vanish with no error on either
+        // side, while iOS decoded the very same bytes fine.
+        assertNotNull(decoded)
+        assertEquals("photo.jpg", decoded!!.fileName)
+        assertEquals("image/jpeg", decoded.mimeType)
+        assertEquals(content.size.toLong(), decoded.fileSize)
+        assertEquals(content.size, decoded.content.size)
+        for (i in content.indices) {
+            assertEquals(content[i], decoded.content[i])
+        }
+    }
+
+    @Test
+    fun `decode should skip an unknown TLV that trails the content`() {
+        // Given: the same kind of extension appended AFTER content, which a
+        // decoder that stops at the first unknown tag also loses.
+        val content = ByteArray(16) { 0x7F }
+        val fileName = "note.m4a".toByteArray(Charsets.UTF_8)
+        val trailing = ByteArray(4) { 0x11 }
+
+        val buf = ByteBuffer.allocate(
+            (1 + 2 + fileName.size) + (1 + 4 + content.size) + (1 + 2 + trailing.size)
+        ).order(ByteOrder.BIG_ENDIAN)
+        buf.put(0x01.toByte()); buf.putShort(fileName.size.toShort()); buf.put(fileName)
+        buf.put(0x04.toByte()); buf.putInt(content.size); buf.put(content)
+        buf.put(0x7F.toByte()); buf.putShort(trailing.size.toShort()); buf.put(trailing)
+
+        // When: Decoding
+        val decoded = BitchatFilePacket.decode(buf.array())
+
+        // Then: Defaults still apply and the content is intact
+        assertNotNull(decoded)
+        assertEquals("note.m4a", decoded!!.fileName)
+        assertEquals("application/octet-stream", decoded.mimeType)
+        assertEquals(content.size.toLong(), decoded.fileSize)
+        assertEquals(content.size, decoded.content.size)
+    }
+
+    @Test
+    fun `decode should reject an incomplete TLV header after an unknown extension`() {
+        // Given: a valid file and unknown extension followed by either only a
+        // tag or a tag plus one length byte.
+        val content = ByteArray(8) { 0x2A }
+        val fileName = "truncated.bin".toByteArray(Charsets.UTF_8)
+        val buf = ByteBuffer.allocate(
+            (1 + 2 + fileName.size) + (1 + 4 + content.size) + (1 + 2)
+        ).order(ByteOrder.BIG_ENDIAN)
+        buf.put(0x01.toByte()); buf.putShort(fileName.size.toShort()); buf.put(fileName)
+        buf.put(0x04.toByte()); buf.putInt(content.size); buf.put(content)
+        buf.put(0x05.toByte()); buf.putShort(0)
+        val packetWithUnknownExtension = buf.array()
+
+        // When/Then: Android rejects the same incomplete tails that iOS does.
+        assertNull(BitchatFilePacket.decode(packetWithUnknownExtension + byteArrayOf(0x06)))
+        assertNull(BitchatFilePacket.decode(packetWithUnknownExtension + byteArrayOf(0x06, 0x00)))
+    }
+
+    @Test
+    fun `decode should handle a packet padded with many zero-length unknown TLVs`() {
+        // Given: the cheapest padding a peer can send — a zero-length unknown
+        // TLV is 3 bytes, so one packet can carry hundreds of thousands of
+        // them. Anything the skip path does per TLV (copying the empty value,
+        // formatting a log line) is scaled by the sender, not by us.
+        val padCount = 200_000
+        val content = ByteArray(8) { 0x5A }
+        val fileName = "padded.bin".toByteArray(Charsets.UTF_8)
+
+        val buf = ByteBuffer.allocate(
+            (1 + 2 + fileName.size) + (padCount * 3) + (1 + 4 + content.size)
+        ).order(ByteOrder.BIG_ENDIAN)
+        buf.put(0x01.toByte()); buf.putShort(fileName.size.toShort()); buf.put(fileName)
+        repeat(padCount) {
+            buf.put(0x05.toByte()); buf.putShort(0)
+        }
+        buf.put(0x04.toByte()); buf.putInt(content.size); buf.put(content)
+
+        // When: Decoding
+        val decoded = BitchatFilePacket.decode(buf.array())
+
+        // Then: the real fields still come through and the padding is ignored
+        assertNotNull(decoded)
+        assertEquals("padded.bin", decoded!!.fileName)
+        assertEquals(content.size.toLong(), decoded.fileSize)
+        assertEquals(content.size, decoded.content.size)
+    }
+
     @Test
     fun `replaceFilePathInContent should correctly format content markers for different file types`() {
         // Given: Different file types