From dc1ef530271a4ace53e16c686485976c843e8333 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sat, 2 May 2026 14:41:51 +0200 Subject: [PATCH] Code review: - switch Tlv.parse to integer cursor - tighten Tlv.parse loop + drop tautological fuzz assertion --- .../quartz/nip19Bech32/tlv/Tlv.kt | 26 ++++++++----------- .../quartz/nip19Bech32/tlv/TlvParseTest.kt | 5 ++-- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip19Bech32/tlv/Tlv.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip19Bech32/tlv/Tlv.kt index dc5d5539c..f88be25cd 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip19Bech32/tlv/Tlv.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip19Bech32/tlv/Tlv.kt @@ -43,21 +43,17 @@ class Tlv( companion object { fun parse(data: ByteArray): Tlv { val result = mutableMapOf>() - var rest = data - // Need at least the 2-byte (type, length) header to read another tuple. A - // single trailing byte previously crashed at `rest[1]` (security review - // 2026-04-24 §2.5). - while (rest.size >= 2) { - val t = rest[0] - val l = rest[1].toUByte().toInt() - // Clamp so a declared length exceeding the remaining bytes is treated as - // a truncated entry to skip rather than an array-bounds throw. - val end = (2 + l).coerceAtMost(rest.size) - val v = rest.copyOfRange(2, end) - rest = rest.copyOfRange(end, rest.size) - if (v.size < l) continue - - result.getOrPut(t) { mutableListOf() }.add(v) + var pos = 0 + // Each tuple needs a 2-byte (type, length) header plus `length` value bytes. + // Stop on a single trailing byte or a declared length that exceeds the + // remaining bytes — both cases previously threw IndexOutOfBoundsException + // (security review 2026-04-24 §2.5). + while (pos + 2 <= data.size) { + val t = data[pos] + val l = data[pos + 1].toUByte().toInt() + if (pos + 2 + l > data.size) break + result.getOrPut(t) { mutableListOf() }.add(data.copyOfRange(pos + 2, pos + 2 + l)) + pos += 2 + l } return Tlv(result) } diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip19Bech32/tlv/TlvParseTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip19Bech32/tlv/TlvParseTest.kt index 4cc0551a8..fc2c44b5b 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip19Bech32/tlv/TlvParseTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip19Bech32/tlv/TlvParseTest.kt @@ -24,7 +24,6 @@ import kotlin.random.Random import kotlin.test.Test import kotlin.test.assertContentEquals import kotlin.test.assertEquals -import kotlin.test.assertTrue // Regression for security review 2026-04-24 §2.5 / Finding #10. // Pre-fix `Tlv.parse` threw `IndexOutOfBoundsException` whenever: @@ -107,12 +106,12 @@ class TlvParseTest { fun arbitraryFuzzInputDoesNotThrow() { // 200 random byte sequences of varying lengths up to 64 bytes. Pre-fix this // would have hit IndexOutOfBoundsException on a meaningful fraction of inputs. + // Implicit assertion: parse must not throw on any input. val rng = Random(0xC0FFEE) repeat(200) { val len = rng.nextInt(0, 65) val data = ByteArray(len) { rng.nextInt().toByte() } - val tlv = Tlv.parse(data) - assertTrue(tlv.data.size >= 0) + Tlv.parse(data) } } }