Prohibited using of zero and negative filed number in ProtoNumber and zero field numbers in input bytes

Fixes #1566

Author: shanshin

formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/internal/Helpers.kt

@@ -87,6 +87,7 @@ internal fun SerialDescriptor.extractParameters(index: Int): ProtoDesc {
         val annotation = annotations[i]
         if (annotation is ProtoNumber) {
             protoId = annotation.number
+            checkFieldNumber(protoId, this)
         } else if (annotation is ProtoType) {
             format = annotation.type
         } else if (annotation is ProtoPacked) {
@@ -118,11 +119,21 @@ internal fun extractProtoId(descriptor: SerialDescriptor, index: Int, zeroBasedD
             return ID_HOLDER_ONE_OF
         } else if (annotation is ProtoNumber) {
             result = annotation.number
+            // 0 or negative numbers are acceptable for enums
+            if (!zeroBasedDefault) {
+                checkFieldNumber(result, descriptor)
+            }
         }
     }
     return result
 }
 
+private fun checkFieldNumber(fieldNumber: Int, descriptor: SerialDescriptor) {
+    if (fieldNumber <= 0) {
+        throw SerializationException("$fieldNumber is not allowed in ProtoNumber for ${descriptor.serialName}, because protobuf support field values in range 1..2147483647")
+    }
+}
+
 internal class ProtobufDecodingException(message: String, e: Throwable? = null) : SerializationException(message, e)
 
 internal expect fun Int.reverseBytes(): Int

formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/internal/ProtobufDecoding.kt

@@ -311,6 +311,9 @@ internal open class ProtobufDecoder(
                 if (protoId == -1) { // EOF
                     return elementMarker.nextUnmarkedIndex()
                 }
+                if (protoId == 0) {
+                    throw SerializationException("0 is not allowed as the protobuf field number in ${descriptor.serialName}, the input bytes may have been corrupted")
+                }
                 val index = getIndexByNum(protoId)
                 if (index == -1) { // not found
                     reader.skipElement()

formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/internal/ProtobufReader.kt

@@ -79,7 +79,7 @@ internal class ProtobufReader(private val input: ByteArrayInput) {
         assertWireType(SIZE_DELIMITED)
         val length = decode32()
         checkLength(length)
-        input.scipExactNBytes(length)
+        input.skipExactNBytes(length)
     }
 
     fun readByteArrayNoTag(): ByteArray {

formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/internal/Streams.kt

@@ -34,7 +34,7 @@ internal class ByteArrayInput(private var array: ByteArray, private val endIndex
     }
 
 
-    fun scipExactNBytes(bytesCount: Int) {
+    fun skipExactNBytes(bytesCount: Int) {
         ensureEnoughBytes(bytesCount)
         position += bytesCount
     }

formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/InvalidFieldNumberTest.kt

@@ -0,0 +1,70 @@
+/*
+ * Copyright 2017-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ */
+
+package kotlinx.serialization.protobuf
+
+
+import kotlinx.serialization.*
+import kotlin.test.*
+
+class InvalidFieldNumberTest {
+
+    @Serializable
+    data class Holder(val value: Int)
+
+    @Serializable
+    data class ListHolder(val value: List<Int>)
+
+    @Serializable
+    data class ZeroProtoNumber(@ProtoNumber(0) val value: Int)
+
+    @Serializable
+    data class NegativeProtoNumber(@ProtoNumber(-5) val value: Int)
+
+    @Test
+    fun testDeserializeZeroInput() {
+        assertFailsWithMessage<SerializationException>("0 is not allowed as the protobuf field number in kotlinx.serialization.protobuf.InvalidFieldNumberTest.Holder, the input bytes may have been corrupted") {
+            // first value with field number = 0
+            val hexString = "000f"
+            ProtoBuf.decodeFromHexString<Holder>(hexString)
+        }
+    }
+
+    @Test
+    fun testDeserializeZeroInputForElement() {
+        assertFailsWithMessage<SerializationException>("0 is not allowed as the protobuf field number in kotlinx.serialization.protobuf.InvalidFieldNumberTest.ListHolder, the input bytes may have been corrupted") {
+            // first element with field number = 0
+            val hexString = "000f"
+            ProtoBuf.decodeFromHexString<ListHolder>(hexString)
+        }
+    }
+
+    @Test
+    fun testSerializeZeroProtoNumber() {
+        assertFailsWithMessage<SerializationException>("0 is not allowed in ProtoNumber for kotlinx.serialization.protobuf.InvalidFieldNumberTest.ZeroProtoNumber, because protobuf support field values in range 1..2147483647") {
+            ProtoBuf.encodeToHexString(ZeroProtoNumber(42))
+        }
+    }
+
+    @Test
+    fun testDeserializeZeroProtoNumber() {
+        assertFailsWithMessage<SerializationException>("0 is not allowed in ProtoNumber for kotlinx.serialization.protobuf.InvalidFieldNumberTest.ZeroProtoNumber, because protobuf support field values in range 1..2147483647") {
+            ProtoBuf.decodeFromHexString<ZeroProtoNumber>("000f")
+        }
+    }
+
+    @Test
+    fun testSerializeNegativeProtoNumber() {
+        assertFailsWithMessage<SerializationException>("-5 is not allowed in ProtoNumber for kotlinx.serialization.protobuf.InvalidFieldNumberTest.NegativeProtoNumber, because protobuf support field values in range 1..2147483647") {
+            ProtoBuf.encodeToHexString(NegativeProtoNumber(42))
+        }
+    }
+
+    @Test
+    fun testDeserializeNegativeProtoNumber() {
+        assertFailsWithMessage<SerializationException>("-5 is not allowed in ProtoNumber for kotlinx.serialization.protobuf.InvalidFieldNumberTest.NegativeProtoNumber, because protobuf support field values in range 1..2147483647") {
+            ProtoBuf.decodeFromHexString<NegativeProtoNumber>("000f")
+        }
+    }
+}

formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/PackedArraySerializerTest.kt

@@ -47,7 +47,6 @@ class PackedArraySerializerTest {
 
     @Serializable
     data class PackedStringCarrier(
-        @ProtoNumber(0)
         @ProtoPacked
         val s: List<String>
     )
@@ -110,12 +109,12 @@ class PackedArraySerializerTest {
     @Test
     fun testEncodeAnnotatedStringList() {
         val obj = PackedStringCarrier(listOf("aaa", "bbb", "ccc"))
-        val expectedHex = "020361616102036262620203636363"
+        val expectedHex = "0a036161610a036262620a03636363"
         val encodedHex = ProtoBuf.encodeToHexString(obj)
         assertEquals(expectedHex, encodedHex)
         assertEquals(obj, ProtoBuf.decodeFromHexString<PackedStringCarrier>(expectedHex))
 
-        val invalidPackedHex = "020C036161610362626203636363"
+        val invalidPackedHex = "0a0C036161610362626203636363"
         val decoded = ProtoBuf.decodeFromHexString<PackedStringCarrier>(invalidPackedHex)
         val invalidString = "\u0003aaa\u0003bbb\u0003ccc"
         assertEquals(PackedStringCarrier(listOf(invalidString)), decoded)

formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/SkipFieldsTest.kt

@@ -14,23 +14,15 @@ class SkipFieldsTest {
     data class Holder(val value: Int)
 
     @Test
-    fun testSkipZeroId() {
-        // first value with id = 0
-        val hexString = "000f082a"
-        val holder = ProtoBuf.decodeFromHexString<Holder>(hexString)
-        assertEquals(42, holder.value)
-    }
-
-    @Test
-    fun testSkipBigId() {
+    fun testSkipBigFieldNumber() {
         // first value with id = 2047 and takes 2 bytes
         val hexString = "f87f20082a"
         val holder = ProtoBuf.decodeFromHexString<Holder>(hexString)
         assertEquals(42, holder.value)
     }
 
     @Test
-    fun testSkipString() {
+    fun testSkipUnknownFiledNumberForString() {
         // first value is size delimited (string) with id = 42
         val hexString = "d2020c48656c6c6f20576f726c6421082a"
         val holder = ProtoBuf.decodeFromHexString<Holder>(hexString)