Code review:
- switch Tlv.parse to integer cursor - tighten Tlv.parse loop + drop tautological fuzz assertion
This commit is contained in:
@@ -43,21 +43,17 @@ class Tlv(
|
|||||||
companion object {
|
companion object {
|
||||||
fun parse(data: ByteArray): Tlv {
|
fun parse(data: ByteArray): Tlv {
|
||||||
val result = mutableMapOf<Byte, MutableList<ByteArray>>()
|
val result = mutableMapOf<Byte, MutableList<ByteArray>>()
|
||||||
var rest = data
|
var pos = 0
|
||||||
// Need at least the 2-byte (type, length) header to read another tuple. A
|
// Each tuple needs a 2-byte (type, length) header plus `length` value bytes.
|
||||||
// single trailing byte previously crashed at `rest[1]` (security review
|
// Stop on a single trailing byte or a declared length that exceeds the
|
||||||
// 2026-04-24 §2.5).
|
// remaining bytes — both cases previously threw IndexOutOfBoundsException
|
||||||
while (rest.size >= 2) {
|
// (security review 2026-04-24 §2.5).
|
||||||
val t = rest[0]
|
while (pos + 2 <= data.size) {
|
||||||
val l = rest[1].toUByte().toInt()
|
val t = data[pos]
|
||||||
// Clamp so a declared length exceeding the remaining bytes is treated as
|
val l = data[pos + 1].toUByte().toInt()
|
||||||
// a truncated entry to skip rather than an array-bounds throw.
|
if (pos + 2 + l > data.size) break
|
||||||
val end = (2 + l).coerceAtMost(rest.size)
|
result.getOrPut(t) { mutableListOf() }.add(data.copyOfRange(pos + 2, pos + 2 + l))
|
||||||
val v = rest.copyOfRange(2, end)
|
pos += 2 + l
|
||||||
rest = rest.copyOfRange(end, rest.size)
|
|
||||||
if (v.size < l) continue
|
|
||||||
|
|
||||||
result.getOrPut(t) { mutableListOf() }.add(v)
|
|
||||||
}
|
}
|
||||||
return Tlv(result)
|
return Tlv(result)
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-3
@@ -24,7 +24,6 @@ import kotlin.random.Random
|
|||||||
import kotlin.test.Test
|
import kotlin.test.Test
|
||||||
import kotlin.test.assertContentEquals
|
import kotlin.test.assertContentEquals
|
||||||
import kotlin.test.assertEquals
|
import kotlin.test.assertEquals
|
||||||
import kotlin.test.assertTrue
|
|
||||||
|
|
||||||
// Regression for security review 2026-04-24 §2.5 / Finding #10.
|
// Regression for security review 2026-04-24 §2.5 / Finding #10.
|
||||||
// Pre-fix `Tlv.parse` threw `IndexOutOfBoundsException` whenever:
|
// Pre-fix `Tlv.parse` threw `IndexOutOfBoundsException` whenever:
|
||||||
@@ -107,12 +106,12 @@ class TlvParseTest {
|
|||||||
fun arbitraryFuzzInputDoesNotThrow() {
|
fun arbitraryFuzzInputDoesNotThrow() {
|
||||||
// 200 random byte sequences of varying lengths up to 64 bytes. Pre-fix this
|
// 200 random byte sequences of varying lengths up to 64 bytes. Pre-fix this
|
||||||
// would have hit IndexOutOfBoundsException on a meaningful fraction of inputs.
|
// would have hit IndexOutOfBoundsException on a meaningful fraction of inputs.
|
||||||
|
// Implicit assertion: parse must not throw on any input.
|
||||||
val rng = Random(0xC0FFEE)
|
val rng = Random(0xC0FFEE)
|
||||||
repeat(200) {
|
repeat(200) {
|
||||||
val len = rng.nextInt(0, 65)
|
val len = rng.nextInt(0, 65)
|
||||||
val data = ByteArray(len) { rng.nextInt().toByte() }
|
val data = ByteArray(len) { rng.nextInt().toByte() }
|
||||||
val tlv = Tlv.parse(data)
|
Tlv.parse(data)
|
||||||
assertTrue(tlv.data.size >= 0)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user