diff --git a/okhttp-tls/src/main/kotlin/okhttp3/tls/internal/der/DerReader.kt b/okhttp-tls/src/main/kotlin/okhttp3/tls/internal/der/DerReader.kt index 0844a962d78a..75f1e03aba18 100644 --- a/okhttp-tls/src/main/kotlin/okhttp3/tls/internal/der/DerReader.kt +++ b/okhttp-tls/src/main/kotlin/okhttp3/tls/internal/der/DerReader.kt @@ -305,12 +305,16 @@ internal class DerReader( /** Used for tags and subidentifiers. */ private fun readVariableLengthLong(): Long { - // TODO(jwilson): detect overflow. var result = 0L while (true) { val byteN = source.readByte().toLong() and 0xff if ((byteN and 0b1000_0000L) == 0b1000_0000L) { - result = (result + (byteN and 0b0111_1111)) shl 7 + result += byteN and 0b0111_1111 + // Once more than 56 bits are used the following shift left by 7 would drop set bits and + // could flip the sign, so the encoded value doesn't fit in a Long. Fail instead of + // silently returning a truncated (possibly negative) number. + if (result ushr 56 != 0L) throw ProtocolException("variable length long > Long.MAX_VALUE") + result = result shl 7 } else { return result + byteN } diff --git a/okhttp-tls/src/test/java/okhttp3/tls/internal/der/DerTest.kt b/okhttp-tls/src/test/java/okhttp3/tls/internal/der/DerTest.kt index 418491ab02e5..2d1740256f78 100644 --- a/okhttp-tls/src/test/java/okhttp3/tls/internal/der/DerTest.kt +++ b/okhttp-tls/src/test/java/okhttp3/tls/internal/der/DerTest.kt @@ -405,6 +405,20 @@ internal class DerTest { assertThat(derReader.hasNext()).isFalse() } + @Test fun `decode object identifier with overflowing subidentifier`() { + val buffer = + Buffer() + .write("060a81808080808080808000".decodeHex()) + + val derReader = DerReader(buffer) + + assertFailsWith { + derReader.read("test") { derReader.readObjectIdentifier() } + }.also { expected -> + assertThat(expected.message).isEqualTo("variable length long > Long.MAX_VALUE") + } + } + @Test fun `encode object identifier without adapter`() { val buffer = Buffer() val derWriter = DerWriter(buffer)