Skip to content

Commit 421f64c

Browse files
authored
CBOR: Relax value range check when decoding numbers (#3167)
A commit e334d1c introduced a strict value range check when decoding integer values. The main intention was to prevent misinterpreting a parsed CBOR document due to integer overflow or truncation. However, the overflow check is too strict and prevent decoding unsigned Kotlin numbers from some unsigned CBOR integers. For now, we should relax the check to verify only that there will be no truncation. Closes #3143
1 parent 85a4f12 commit 421f64c

2 files changed

Lines changed: 54 additions & 9 deletions

File tree

formats/cbor/commonMain/src/kotlinx/serialization/cbor/internal/Decoder.kt

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,17 +130,22 @@ internal open class CborReader(override val cbor: Cbor, protected val parser: Cb
130130
override fun decodeBoolean() = parser.nextBoolean(tags)
131131

132132
override fun decodeByte() = parser.nextNumberWithinRange(
133-
tags, Byte.MIN_VALUE.toLong(), Byte.MAX_VALUE.toLong(), "Byte"
133+
tags, Byte.MIN_VALUE.toLong(), Byte.MAX_VALUE.toLong(), UByte.MAX_VALUE.toLong(), "Byte"
134134
).toByte()
135+
135136
override fun decodeShort() = parser.nextNumberWithinRange(
136-
tags, Short.MIN_VALUE.toLong(), Short.MAX_VALUE.toLong(), "Short"
137+
tags, Short.MIN_VALUE.toLong(), Short.MAX_VALUE.toLong(), UShort.MAX_VALUE.toLong(), "Short"
137138
).toShort()
139+
138140
override fun decodeChar() = parser.nextNumberWithinRange(
139-
tags, Char.MIN_VALUE.code.toLong(), Char.MAX_VALUE.code.toLong(), "Char"
141+
tags, Char.MIN_VALUE.code.toLong(), Char.MAX_VALUE.code.toLong(),
142+
/* no unsigned type for Char */ -1, "Char"
140143
).toInt().toChar()
144+
141145
override fun decodeInt() = parser.nextNumberWithinRange(
142-
tags, Int.MIN_VALUE.toLong(), Int.MAX_VALUE.toLong(), "Int"
146+
tags, Int.MIN_VALUE.toLong(), Int.MAX_VALUE.toLong(), UInt.MAX_VALUE.toLong(), "Int"
143147
).toInt()
148+
144149
override fun decodeLong() = parser.nextNumber(tags)
145150

146151
override fun decodeNull() = parser.nextNull(tags)
@@ -353,10 +358,21 @@ internal class CborParser(private val input: ByteArrayInput, private val verifyO
353358
}
354359
}
355360

356-
internal fun nextNumberWithinRange(tags: ULongArray?, from: Long, to: Long, type: String): Long {
361+
internal fun nextNumberWithinRange(
362+
tags: ULongArray?,
363+
from: Long,
364+
to: Long,
365+
unsignedUpperBound: Long,
366+
type: String,
367+
): Long {
357368
val number = nextNumber(tags)
358-
if (number !in from..to) {
359-
throw CborDecodingException("Decoded number $number is not within the range for type $type ([$from..$to])")
369+
if (number !in from..to && number !in 0..unsignedUpperBound) {
370+
throw CborDecodingException(buildString {
371+
append("Decoded number $number is not within the range for type $type ([$from..$to])")
372+
if (unsignedUpperBound >= 0) {
373+
append(", nor it is within the range for U$type ([0..$unsignedUpperBound])")
374+
}
375+
})
360376
}
361377
return number
362378
}

formats/cbor/commonTest/src/kotlinx/serialization/cbor/CborDecoderTest.kt

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -612,26 +612,42 @@ class CborDecoderTest {
612612
val singleByteValue = "BF6176182AFF"
613613
// 39: two-byte int \ / -16162
614614
val twoByteValue = "BF6176393F21FF"
615-
// 19: two-byte int \ / C0DE
615+
// 19: two-byte int \ / C0DE
616616
val twoByteValueWithShortOverflow = "BF617619C0DEFF"
617+
// 19: two-byte int \ / 01DE
618+
val twoByteValueSlightlyOutOfByte = "BF61761901DEFF"
619+
// 38: negative single-byte int \ / 80
620+
val negativeTwoByteValueSlightlyOutOfByte = "BF61763880FF"
617621
// 1A: four-byte int \ / 0BADC0DE
618622
val fourByteValue = "BF61761A0BADC0DEFF"
623+
// 1A: four-byte int \ / 0001CODE
624+
val fourByteValueSlightlyOutOfShort = "BF61761A0001C0DEFF"
625+
// 39: two-byte negative int \ / 8000
626+
val negativeFourByteValueSlightlyOutOfShort = "BF6176398000FF"
619627
// 1B: eight-byte int \ / 0BADC0DE15BAD000
620628
val eightByteValue = "BF61761B0BADC0DE15BAD000FF"
621629

622630
assertEquals(0x2A, Cbor.decodeFromHexString<IntHolder>(singleByteValue).v)
623631
assertEquals(0xC0DE, Cbor.decodeFromHexString<IntHolder>(twoByteValueWithShortOverflow).v)
624632
assertEquals(0xBADC0DE, Cbor.decodeFromHexString<IntHolder>(fourByteValue).v)
633+
assertEquals(0x1C0DE,Cbor.decodeFromHexString<IntHolder>(fourByteValueSlightlyOutOfShort).v)
634+
assertEquals(-32769,Cbor.decodeFromHexString<IntHolder>(negativeFourByteValueSlightlyOutOfShort).v)
625635
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<IntHolder>(eightByteValue) }
626636

627637
assertEquals(0x2A, Cbor.decodeFromHexString<ShortHolder>(singleByteValue).v)
628638
assertEquals(0xC0DE.toShort(), Cbor.decodeFromHexString<ShortHolder>(twoByteValue).v)
629-
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<ShortHolder>(twoByteValueWithShortOverflow) }
639+
assertEquals(0xC0DE.toShort(), Cbor.decodeFromHexString<ShortHolder>(twoByteValueWithShortOverflow).v)
630640
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<ShortHolder>(fourByteValue) }
641+
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<ShortHolder>(fourByteValueSlightlyOutOfShort) }
642+
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<ShortHolder>(negativeFourByteValueSlightlyOutOfShort) }
631643
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<ShortHolder>(eightByteValue) }
632644

633645
assertEquals(0x2A, Cbor.decodeFromHexString<ByteHolder>(singleByteValue).v)
634646
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<ByteHolder>(twoByteValue) }
647+
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<ByteHolder>(twoByteValueSlightlyOutOfByte) }
648+
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<ByteHolder>(fourByteValueSlightlyOutOfShort) }
649+
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<ByteHolder>(negativeTwoByteValueSlightlyOutOfByte) }
650+
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<ByteHolder>(negativeFourByteValueSlightlyOutOfShort) }
635651
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<ByteHolder>(fourByteValue) }
636652
assertFailsWith<CborDecodingException> { Cbor.decodeFromHexString<ByteHolder>(eightByteValue) }
637653
}
@@ -709,4 +725,17 @@ class CborDecoderTest {
709725
Cbor.decodeFromHexString<BytesHolder>(paddedInput)
710726
}
711727
}
728+
729+
@Test
730+
fun testEncodeUnsignedValuesFromPositiveInteger() {
731+
assertEquals(200U.toUByte(), Cbor.decodeFromHexString<UByte>("18C8"))
732+
checkDecodingException<UByte>("197D00", "Decoded number 32000 is not within the range for type Byte ([-128..127]), nor it is within the range for UByte ([0..255])")
733+
assertEquals(32000U.toUShort(), Cbor.decodeFromHexString<UShort>("197D00"))
734+
assertEquals(32000.toChar(), Cbor.decodeFromHexString<Char>("197D00"))
735+
checkDecodingException<Char>("1A80000000", "Decoded number 2147483648 is not within the range for type Char ([0..65535])")
736+
checkDecodingException<UShort>("1A80000000", "Decoded number 2147483648 is not within the range for type Short ([-32768..32767]), nor it is within the range for UShort ([0..65535])")
737+
assertEquals(2147483648U, Cbor.decodeFromHexString<UInt>("1A80000000"))
738+
checkDecodingException<UInt>("1B8000000000000000", "Decoded number -9223372036854775808 is not within the range for type Int ([-2147483648..2147483647]), nor it is within the range for UInt ([0..4294967295])")
739+
assertEquals(9223372036854775808UL, Cbor.decodeFromHexString<ULong>("1B8000000000000000"))
740+
}
712741
}

0 commit comments

Comments
 (0)