From 47d5b0dba53935d5332cd41a80a353b3fc90e7b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Quenaudon?= Date: Tue, 12 May 2026 17:43:08 +0100 Subject: [PATCH] Add negative-length check in skipGroup --- .../kotlin/com/squareup/wire/ByteArrayProtoReader32.kt | 1 + .../commonMain/kotlin/com/squareup/wire/ProtoReader.kt | 1 + .../kotlin/com/squareup/wire/ProtoReader32Test.kt | 10 ++++++++++ .../kotlin/com/squareup/wire/ProtoReaderTest.kt | 10 ++++++++++ 4 files changed, 22 insertions(+) diff --git a/wire-runtime/src/commonMain/kotlin/com/squareup/wire/ByteArrayProtoReader32.kt b/wire-runtime/src/commonMain/kotlin/com/squareup/wire/ByteArrayProtoReader32.kt index 2dfa9aafe8..dfc3b6f028 100644 --- a/wire-runtime/src/commonMain/kotlin/com/squareup/wire/ByteArrayProtoReader32.kt +++ b/wire-runtime/src/commonMain/kotlin/com/squareup/wire/ByteArrayProtoReader32.kt @@ -228,6 +228,7 @@ internal class ByteArrayProtoReader32( } STATE_LENGTH_DELIMITED -> { val length = internalReadVarint32() + if (length < 0) throw ProtocolException("Negative length: $length. Reader position: $pos. Last read tag: $tag.") skip(length) } STATE_VARINT -> { diff --git a/wire-runtime/src/commonMain/kotlin/com/squareup/wire/ProtoReader.kt b/wire-runtime/src/commonMain/kotlin/com/squareup/wire/ProtoReader.kt index 2c18a307f9..99adb4524f 100644 --- a/wire-runtime/src/commonMain/kotlin/com/squareup/wire/ProtoReader.kt +++ b/wire-runtime/src/commonMain/kotlin/com/squareup/wire/ProtoReader.kt @@ -267,6 +267,7 @@ open class ProtoReader(private val source: BufferedSource) { } STATE_LENGTH_DELIMITED -> { val length = internalReadVarint32() + if (length < 0) throw ProtocolException("Negative length: $length. Reader position: $pos. Last read tag: $tag.") pos += length.toLong() source.skip(length.toLong()) } diff --git a/wire-runtime/src/commonTest/kotlin/com/squareup/wire/ProtoReader32Test.kt b/wire-runtime/src/commonTest/kotlin/com/squareup/wire/ProtoReader32Test.kt index bee79a5fef..5d0b7e5d8c 100644 --- a/wire-runtime/src/commonTest/kotlin/com/squareup/wire/ProtoReader32Test.kt +++ b/wire-runtime/src/commonTest/kotlin/com/squareup/wire/ProtoReader32Test.kt @@ -78,5 +78,15 @@ class ProtoReader32Test { }.isInstanceOf().hasMessage("Wire recursion limit exceeded") } + /** We had a bug where negative lengths in skipped groups crashed with runtime exceptions. */ + @Test fun testSkipGroupRejectsNegativeLength() { + val data = "9b060a80ffffff0f9c06".decodeHex() + + assertFailure { + Person.ADAPTER.decode(data) + }.isInstanceOf() + .hasMessage("Negative length: -128. Reader position: 8. Last read tag: 1.") + } + // Consider pasting new tests into ProtoReaderTest.kt also. } diff --git a/wire-runtime/src/commonTest/kotlin/com/squareup/wire/ProtoReaderTest.kt b/wire-runtime/src/commonTest/kotlin/com/squareup/wire/ProtoReaderTest.kt index 79289dd187..e75b572d88 100644 --- a/wire-runtime/src/commonTest/kotlin/com/squareup/wire/ProtoReaderTest.kt +++ b/wire-runtime/src/commonTest/kotlin/com/squareup/wire/ProtoReaderTest.kt @@ -79,5 +79,15 @@ class ProtoReaderTest { }.isInstanceOf().hasMessage("Wire recursion limit exceeded") } + /** We had a bug where negative lengths in skipped groups crashed with runtime exceptions. */ + @Test fun testSkipGroupRejectsNegativeLength() { + val data = "9b060a80ffffff0f9c06".decodeHex() + + assertFailure { + Person.ADAPTER.decode(Buffer().write(data)) + }.isInstanceOf() + .hasMessage("Negative length: -128. Reader position: 8. Last read tag: 1.") + } + // Consider pasting new tests into ProtoReader32Test.kt also. }