diff --git a/app/src/main/java/com/bitchat/android/model/BitchatFilePacket.kt b/app/src/main/java/com/bitchat/android/model/BitchatFilePacket.kt index 5e47742f..710d72b1 100644 --- a/app/src/main/java/com/bitchat/android/model/BitchatFilePacket.kt +++ b/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( } } } - diff --git a/app/src/test/kotlin/com/bitchat/FileTransferTest.kt b/app/src/test/kotlin/com/bitchat/FileTransferTest.kt index 8a136e69..2d1ec24a 100644 --- a/app/src/test/kotlin/com/bitchat/FileTransferTest.kt +++ b/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