diff --git a/app/src/main/java/com/bitchat/android/mesh/FragmentManager.kt b/app/src/main/java/com/bitchat/android/mesh/FragmentManager.kt index 3a10f998..edd7169f 100644 --- a/app/src/main/java/com/bitchat/android/mesh/FragmentManager.kt +++ b/app/src/main/java/com/bitchat/android/mesh/FragmentManager.kt @@ -197,8 +197,15 @@ class FragmentManager { synchronized(fragmentStateLock) { fragmentMetadata[fragmentIDString]?.let { (expectedType, expectedTotal, _) -> if (expectedTotal != fragmentPayload.total || expectedType != fragmentPayload.originalType) { + // Reject the fragment, keep the stream. Fragment packets + // are unauthenticated and both halves of the key are + // attacker-choosable, so discarding the set here let one + // crafted packet destroy a legitimate in-flight + // reassembly. Nothing from a conflicting header is ever + // stored, so keeping the set stays inside the same + // memory bound; a stream that genuinely stalls is reaped + // by the timeout sweep. Log.w(TAG, "Rejecting fragment for $fragmentIDString: inconsistent metadata") - removeFragmentSetLocked(fragmentIDString) return null } } @@ -234,12 +241,30 @@ class FragmentManager { return null } - val oldEntrySize = fragmentMap[fragmentPayload.index]?.size ?: 0 + val heldEntry = fragmentMap[fragmentPayload.index] + if (heldEntry != null && !heldEntry.contentEquals(fragmentPayload.data)) { + // Duplicate delivery is normal in a mesh, but one index of a + // stream must always carry the same bytes. Overwriting + // last-wins let a crafted duplicate replace bytes already + // received from the real sender. This protects indices + // already held; an index still empty when the injected + // fragment arrives is filled by whichever copy wins the + // race, and nothing here can tell them apart. + Log.w(TAG, "Rejecting fragment for $fragmentIDString: index ${fragmentPayload.index} already holds different bytes") + return null + } + val oldEntrySize = heldEntry?.size ?: 0 val newSize = currentSize - oldEntrySize + fragmentPayload.data.size val maxTotalBytes = com.bitchat.android.util.AppConstants.Fragmentation.MAX_FRAGMENT_TOTAL_BYTES if (newSize > maxTotalBytes) { Log.w(TAG, "Rejecting fragment for $fragmentIDString: cumulative size $newSize exceeds cap $maxTotalBytes") - removeFragmentSetLocked(fragmentIDString) + // Only a set this fragment itself created has nothing worth + // preserving. An oversized fragment must not be able to + // destroy an assembly it did not start — same reasoning the + // global-cap branch below already applies. + if (isNewSet) { + removeFragmentSetLocked(fragmentIDString) + } return null } diff --git a/app/src/test/java/com/bitchat/android/mesh/FragmentManagerTest.kt b/app/src/test/java/com/bitchat/android/mesh/FragmentManagerTest.kt index bf3e3d01..17a264ff 100644 --- a/app/src/test/java/com/bitchat/android/mesh/FragmentManagerTest.kt +++ b/app/src/test/java/com/bitchat/android/mesh/FragmentManagerTest.kt @@ -252,4 +252,143 @@ class FragmentManagerTest { } return result } + + // Injected-fragment hardening. Fragment packets are unauthenticated and + // both halves of the reassembly key are attacker-choosable, so a stream in + // flight has to survive a crafted packet aimed at it. Ports the storage + // rules from permissionlesstech/bitchat#1515. + + /** A real packet, fragmented the way the sender would fragment it. */ + private fun honestStream(): Pair> { + val payload = ByteArray(1200) { (it % 251).toByte() } + val packet = BitchatPacket( + version = 1u, + type = MessageType.FILE_TRANSFER.value, + senderID = hexStringToByteArray(senderID), + recipientID = hexStringToByteArray(recipientID), + timestamp = 1u, + payload = payload, + ttl = 7u + ) + val fragments = fragmentManager.createFragments(packet) + assertTrue("Need a multi-fragment stream", fragments.size >= 3) + return packet to fragments + } + + /** Same stream key as [honest], but attacker-chosen contents. */ + private fun injected( + honest: BitchatPacket, + index: Int, + total: Int, + data: ByteArray, + originalType: UByte + ): BitchatPacket { + val decoded = FragmentPayload.decode(honest.payload)!! + return honest.copy( + payload = FragmentPayload( + fragmentID = decoded.fragmentID, + index = index, + total = total, + originalType = originalType, + data = data + ).encode() + ) + } + + private fun feedAllButLast(fragments: List) { + fragments.dropLast(1).forEach { + assertNull("No fragment before the last should complete", fragmentManager.handleFragment(it)) + } + } + + private fun assertCompletes(original: BitchatPacket, last: BitchatPacket) { + val reassembled = fragmentManager.handleFragment(last) + assertNotNull("The honest stream must still complete", reassembled) + assertTrue( + "Reassembled payload must be the honest bytes", + original.payload.contentEquals(reassembled!!.payload) + ) + } + + @Test + fun `a conflicting total is rejected without destroying the stream`() { + val (original, fragments) = honestStream() + feedAllButLast(fragments) + + val decoded = FragmentPayload.decode(fragments[0].payload)!! + assertNull( + fragmentManager.handleFragment( + injected(fragments[0], index = 0, total = decoded.total - 1, + data = ByteArray(4), originalType = decoded.originalType) + ) + ) + + assertCompletes(original, fragments.last()) + } + + @Test + fun `a conflicting original type is rejected without destroying the stream`() { + val (original, fragments) = honestStream() + feedAllButLast(fragments) + + val decoded = FragmentPayload.decode(fragments[0].payload)!! + assertNull( + fragmentManager.handleFragment( + injected(fragments[0], index = 0, total = decoded.total, + data = ByteArray(4), originalType = MessageType.ANNOUNCE.value) + ) + ) + + assertCompletes(original, fragments.last()) + } + + @Test + fun `an index already held keeps its first bytes`() { + val (original, fragments) = honestStream() + feedAllButLast(fragments) + + // Same stream, same index, different bytes: last-wins would swap the + // real sender's bytes out from under the reassembly. + val decoded = FragmentPayload.decode(fragments[0].payload)!! + assertNull( + fragmentManager.handleFragment( + injected(fragments[0], index = 0, total = decoded.total, + data = ByteArray(decoded.data.size) { 0x66 }, + originalType = decoded.originalType) + ) + ) + + assertCompletes(original, fragments.last()) + } + + @Test + fun `an identical duplicate is still accepted`() { + // Duplicate delivery is normal in a mesh; only a *differing* duplicate + // is an attack, so redelivery must not be turned into a rejection. + val (original, fragments) = honestStream() + feedAllButLast(fragments) + + assertNull(fragmentManager.handleFragment(fragments[0])) + + assertCompletes(original, fragments.last()) + } + + @Test + fun `an oversized fragment cannot wipe an assembly it did not start`() { + val (original, fragments) = honestStream() + feedAllButLast(fragments) + + val decoded = FragmentPayload.decode(fragments[0].payload)!! + val oversized = ByteArray( + com.bitchat.android.util.AppConstants.Fragmentation.MAX_FRAGMENT_TOTAL_BYTES + 1 + ) + assertNull( + fragmentManager.handleFragment( + injected(fragments[0], index = decoded.total - 1, total = decoded.total, + data = oversized, originalType = decoded.originalType) + ) + ) + + assertCompletes(original, fragments.last()) + } }