Merge 84429c8c0583ba6cbd33e866b53c1b62376449e2 into 945c9e025be1c3ea08db76398893c640452bcd20

This commit is contained in:
Taksh Kothari 2026-09-08 13:25:01 +03:00 committed by GitHub
commit 86394fba6f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 167 additions and 3 deletions

View File

@ -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
}

View File

@ -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<BitchatPacket, List<BitchatPacket>> {
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<BitchatPacket>) {
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())
}
}