Stop an injected fragment from destroying an in-flight assembly

Fragment packets are unauthenticated and bypass the deduplicator, and
both halves of the reassembly key are attacker-choosable, so a stream in
flight has to survive a crafted packet aimed at it. Two paths discarded
the whole set on a single bad fragment:

A header whose total or originalType disagrees with the pinned metadata
now rejects that fragment and leaves the stream alone. Nothing from a
conflicting header was ever stored, so keeping the set stays inside the
same memory bound, and a stream that genuinely stalls is still reaped by
the timeout sweep.

A fragment that trips the per-set cumulative cap only clears the set when
it created it. An oversized fragment must not be able to destroy an
assembly it did not start — which is the rule the global-cap branch a few
lines below already applied.

Both are demonstrated against a real fragmented packet: each test fails
on main and passes here.

Ports the storage half of permissionlesstech/bitchat#1515.
This commit is contained in:
Taksh 2026-08-15 11:10:51 +05:30
parent 5156f7de89
commit 6ec10ef774
2 changed files with 123 additions and 2 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
}
}
@ -239,7 +246,13 @@ class FragmentManager {
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,112 @@ 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 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())
}
}