From 46de1a36232192d046de85a1c3857b2a19483a91 Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Mon, 27 Jul 2026 22:03:01 +0200 Subject: [PATCH 1/2] fix: skip unknown file TLVs instead of dropping the transfer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `BitchatFilePacket.decode` resolved every tag through the four-value `TLVType` enum and bailed on the first miss: val t = TLVType.from(data[off].toUByte()) ?: return null So a file packet carrying one tag this build does not know is not partially understood — it is discarded whole, media included. The receiver shows nothing and logs nothing; the sender sees a successful transfer. Both apps look healthy. iOS has always skipped unknown tags (`case nil: continue` in its own `BitchatFilePacket.decode`), so the two implementations disagreed about what a valid packet is, and the tag list stopped being extensible in practice: any optional field added by a newer or third-party client costs every Android peer the whole file rather than just that field. Unknown tags now advance past the value, exactly as iOS does. Known-tag handling, the 4-byte CONTENT length, multi-CONTENT concatenation and every existing rejection are unchanged. The skip path is deliberately allocation- and log-free per TLV, because its iteration count is chosen by the sender: a zero-length unknown TLV costs 3 bytes, so a padded packet would otherwise mean millions of empty array copies and formatted log lines monopolising the mesh handler. The count is reported once after the loop instead. Tests: `decode should skip unknown TLV types instead of dropping the whole file` (extension before CONTENT), `decode should skip an unknown TLV that trails the content` (after it), and `decode should handle a packet padded with many zero-length unknown TLVs` (200k of them). The first two fail on the old decoder. --- .../android/model/BitchatFilePacket.kt | 27 +++++- .../kotlin/com/bitchat/FileTransferTest.kt | 91 +++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) 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..0487eb86 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. @@ -90,8 +97,11 @@ data class BitchatFilePacket( var size: Long? = null var mime: String? = null var contentBytes: ByteArray? = null + var skippedUnknownTLVs = 0 while (off + 3 <= data.size) { // minimum TLV header size (type + 2 bytes length) - val t = TLVType.from(data[off].toUByte()) ?: 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 +115,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 +146,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() diff --git a/app/src/test/kotlin/com/bitchat/FileTransferTest.kt b/app/src/test/kotlin/com/bitchat/FileTransferTest.kt index 8a136e69..70882e86 100644 --- a/app/src/test/kotlin/com/bitchat/FileTransferTest.kt +++ b/app/src/test/kotlin/com/bitchat/FileTransferTest.kt @@ -139,6 +139,97 @@ 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 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 From 1a792ecfec893dc5f387b95f716bd3cafbcc9a5b Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:33:02 +0200 Subject: [PATCH 2/2] fix: harden unknown file TLV parsing --- .../android/model/BitchatFilePacket.kt | 17 +++++++++++++--- .../kotlin/com/bitchat/FileTransferTest.kt | 20 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) 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 0487eb86..710d72b1 100644 --- a/app/src/main/java/com/bitchat/android/model/BitchatFilePacket.kt +++ b/app/src/main/java/com/bitchat/android/model/BitchatFilePacket.kt @@ -33,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? { @@ -98,7 +106,11 @@ data class BitchatFilePacket( var mime: String? = null var contentBytes: ByteArray? = null var skippedUnknownTLVs = 0 - while (off + 3 <= data.size) { // minimum TLV header size (type + 2 bytes length) + 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()) @@ -163,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 70882e86..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 @@ -201,6 +202,25 @@ class FileTransferTest { 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