From 56c7e6d0be4162ade3f337f1c76862641c6210bb Mon Sep 17 00:00:00 2001 From: Taksh Date: Mon, 17 Aug 2026 08:42:49 +0530 Subject: [PATCH 1/2] fix(sync): reject out-of-range GCS parameters instead of decoding garbage decodeToSortedSet takes p and m straight off the wire - REQUEST_SYNC carries P as a uint8, so 0..255 all arrive - and decodes with them regardless. Kotlin's shift operators use the low 6 bits of the count, so p >= 64 wraps and the decoder emits values that were never in the filter; p = 0 turns every remainder into a zero-width read and manufactures a value per bit. On today's 20-id filter, p = 0 decodes 105 values. Those values are read as "the peer already has this packet", so the responder withholds packets the peer actually lacks - the failure direction that loses messages. Guard the parameters as iOS does in GCSFilter.decodeToSortedSet: p in 1..32 and m > 1, otherwise decode to nothing, which callers read as "peer has nothing" and answer by sending the data. --- .../main/java/com/bitchat/android/sync/GCSFilter.kt | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/app/src/main/java/com/bitchat/android/sync/GCSFilter.kt b/app/src/main/java/com/bitchat/android/sync/GCSFilter.kt index 212def6a9..6411360f3 100644 --- a/app/src/main/java/com/bitchat/android/sync/GCSFilter.kt +++ b/app/src/main/java/com/bitchat/android/sync/GCSFilter.kt @@ -18,6 +18,14 @@ import kotlin.math.ln * - Bitstream is packed MSB-first in each byte. */ object GCSFilter { + /** + * Highest Golomb-Rice parameter accepted from the wire. P maps to an FPR of + * ~1/2^P; beyond 32 the remainder width exceeds any practical filter, and + * the shifts in decode silently wrap (Kotlin shifts use the low 6 bits of + * the count) into garbage values. + */ + const val MAX_P = 32 + data class Params( val p: Int, // Golomb-Rice parameter (>= 1) val m: Long, // Range M = N * 2^P @@ -70,6 +78,11 @@ object GCSFilter { } fun decodeToSortedSet(p: Int, m: Long, data: ByteArray): LongArray { + // p and m arrive off the wire. Reject out-of-range parameters rather + // than decoding garbage: callers read the result as "peer has nothing" + // and fall back to sending the data, which is the safe direction. + // Matches the iOS guard in GCSFilter.decodeToSortedSet. + if (p < 1 || p > MAX_P || m <= 1L) return LongArray(0) val values = ArrayList() val reader = BitReader(data) var acc = 0L From 1ecb5e9092e6d665a42cd2e5e7c18df41bfae54b Mon Sep 17 00:00:00 2001 From: Taksh Date: Mon, 17 Aug 2026 08:42:49 +0530 Subject: [PATCH 2/2] test(sync): cover the GCS parameter guard Round trip with the built parameters, out-of-range p (0, 33, 64, 200, 255) decoding to nothing, and degenerate m (0, 1) decoding to nothing. The p case fails without the guard: p = 0 yields 105 fabricated values. --- .../android/sync/GCSFilterParameterTest.kt | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 app/src/test/kotlin/com/bitchat/android/sync/GCSFilterParameterTest.kt diff --git a/app/src/test/kotlin/com/bitchat/android/sync/GCSFilterParameterTest.kt b/app/src/test/kotlin/com/bitchat/android/sync/GCSFilterParameterTest.kt new file mode 100644 index 000000000..a453b1687 --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/sync/GCSFilterParameterTest.kt @@ -0,0 +1,53 @@ +package com.bitchat.android.sync + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import java.util.Random + +/** + * `p` and `m` arrive off the wire — a REQUEST_SYNC carries P as a uint8 — and + * the decoded set decides which packets a peer is told it already has. Decoding + * garbage from an out-of-range parameter therefore withholds real packets, so + * out-of-range parameters have to decode to "peer has nothing" instead. + */ +class GCSFilterParameterTest { + + private fun ids(n: Int): List { + val random = Random(42) + return List(n) { + val bytes = ByteArray(16) + random.nextBytes(bytes) + bytes + } + } + + @Test + fun `a filter round-trips with the parameters it was built with`() { + val params = GCSFilter.buildFilter(ids(20), maxBytes = 400, targetFpr = 0.01) + val decoded = GCSFilter.decodeToSortedSet(params.p, params.m, params.data) + + assertTrue("expected a non-empty decode", decoded.isNotEmpty()) + assertTrue("values must stay in range", decoded.all { it in 1 until params.m }) + assertEquals(decoded.toList(), decoded.sorted()) + } + + @Test + fun `an out-of-range p decodes to nothing rather than to garbage`() { + val params = GCSFilter.buildFilter(ids(20), maxBytes = 400, targetFpr = 0.01) + + // 64 and above wrap Kotlin's shift operators; 255 is what the byte allows. + for (p in listOf(0, 33, 64, 200, 255)) { + val decoded = GCSFilter.decodeToSortedSet(p, params.m, params.data) + assertTrue("p=$p should decode to nothing, got ${decoded.size} values", decoded.isEmpty()) + } + } + + @Test + fun `a degenerate m decodes to nothing`() { + val params = GCSFilter.buildFilter(ids(20), maxBytes = 400, targetFpr = 0.01) + + assertTrue(GCSFilter.decodeToSortedSet(params.p, 0L, params.data).isEmpty()) + assertTrue(GCSFilter.decodeToSortedSet(params.p, 1L, params.data).isEmpty()) + } +}