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 67bb36d53..dc5d5539c 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 @@ -44,17 +44,20 @@ class Tlv( fun parse(data: ByteArray): Tlv { val result = mutableMapOf>() var rest = data - while (rest.isNotEmpty()) { + // 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() - val v = rest.sliceArray(IntRange(2, (2 + l) - 1)) - rest = rest.sliceArray(IntRange(2 + l, rest.size - 1)) + // 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 - if (!result.containsKey(t)) { - result[t] = mutableListOf() - } - result[t]?.add(v) + result.getOrPut(t) { mutableListOf() }.add(v) } 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 new file mode 100644 index 000000000..4cc0551a8 --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip19Bech32/tlv/TlvParseTest.kt @@ -0,0 +1,118 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.nip19Bech32.tlv + +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: +// - input had a trailing single byte (read `rest[1]` with size==1) +// - a TLV entry declared length > remaining bytes (slice walked off the end) +// Both are reachable from a hostile naddr/nevent/nprofile/nrelay relay hint. +class TlvParseTest { + private fun bytes(vararg ints: Int) = ByteArray(ints.size) { ints[it].toByte() } + + @Test + fun parsesEmptyInput() { + val tlv = Tlv.parse(bytes()) + assertEquals(null, tlv.firstAsString(0)) + } + + @Test + fun singleTrailingByteDoesNotThrow() { + // Pre-fix: rest[1] threw IndexOutOfBoundsException. + val tlv = Tlv.parse(bytes(0x05)) + assertEquals(null, tlv.firstAsString(5)) + } + + @Test + fun parsesSingleEntry() { + // type=1, length=3, value=[0xa, 0xb, 0xc] + val tlv = Tlv.parse(bytes(1, 3, 0xa, 0xb, 0xc)) + assertContentEquals(byteArrayOf(0xa, 0xb, 0xc), tlv.data[1.toByte()]?.first()) + } + + @Test + fun parsesEntryWithEmptyValue() { + // type=2, length=0 + val tlv = Tlv.parse(bytes(2, 0)) + assertContentEquals(byteArrayOf(), tlv.data[2.toByte()]?.first()) + } + + @Test + fun parsesMultipleEntries() { + // (1,2,[a,b]) (3,1,[c]) + val tlv = Tlv.parse(bytes(1, 2, 0xa, 0xb, 3, 1, 0xc)) + assertContentEquals(byteArrayOf(0xa, 0xb), tlv.data[1.toByte()]?.first()) + assertContentEquals(byteArrayOf(0xc), tlv.data[3.toByte()]?.first()) + } + + @Test + fun groupsRepeatedTypes() { + // (5,1,[a]) (5,1,[b]) — two entries under the same type get appended + val tlv = Tlv.parse(bytes(5, 1, 0xa, 5, 1, 0xb)) + val entries = tlv.data[5.toByte()] + assertEquals(2, entries?.size) + assertContentEquals(byteArrayOf(0xa), entries?.get(0)) + assertContentEquals(byteArrayOf(0xb), entries?.get(1)) + } + + @Test + fun truncatedDeclaredLengthIsSkippedNotThrown() { + // type=1, length=5, but only 2 value bytes follow → truncated, must be skipped. + // Pre-fix: sliceArray(IntRange(2, 6)) on a 4-byte array threw. + val tlv = Tlv.parse(bytes(1, 5, 0xa, 0xb)) + assertEquals(null, tlv.data[1.toByte()]) + } + + @Test + fun keepsValidPrefixWhenLaterTupleIsTruncated() { + // First (1,2,[a,b]) is well-formed, then (2,5,[c,d]) is truncated. + val tlv = Tlv.parse(bytes(1, 2, 0xa, 0xb, 2, 5, 0xc, 0xd)) + assertContentEquals(byteArrayOf(0xa, 0xb), tlv.data[1.toByte()]?.first()) + assertEquals(null, tlv.data[2.toByte()]) + } + + @Test + fun handlesUnsignedLengthByte() { + // length byte 0xFF must be treated as 255 (unsigned), not -1. + val payload = ByteArray(255) { 0x42 } + val tlv = Tlv.parse(byteArrayOf(7, 0xFF.toByte()) + payload) + assertContentEquals(payload, tlv.data[7.toByte()]?.first()) + } + + @Test + 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. + 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) + } + } +}