diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http2/Hpack.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http2/Hpack.kt index 07fe714aa727..7c269d9c8da8 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http2/Hpack.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http2/Hpack.kt @@ -365,19 +365,25 @@ object Hpack { } // This is a multibyte value. Read 7 bits at a time. - var result = prefixMask + var result = prefixMask.toLong() var shift = 0 + var byteCount = 0 while (true) { + // An Int.MAX_VALUE payload needs at most 5 continuation bytes after the prefix. + if (byteCount == 5) { + throw IOException("HPACK integer overflow") + } val b = readByte() - if (b and 0x80 != 0) { // Equivalent to (b >= 128) since b is in [0..255]. - result += b and 0x7f shl shift - shift += 7 - } else { - result += b shl shift // Last byte. - break + byteCount++ + val increment = (b and 0x7f).toLong() shl shift + if (increment > Int.MAX_VALUE.toLong() - result) { + throw IOException("HPACK integer overflow") } + result += increment + if (b and 0x80 == 0) break + shift += 7 } - return result + return result.toInt() } /** Reads a potentially Huffman encoded byte string. */ diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/internal/http2/HpackTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/internal/http2/HpackTest.kt index 60be037c1bf3..cf6fc58a1ea5 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/internal/http2/HpackTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/internal/http2/HpackTest.kt @@ -375,7 +375,7 @@ class HpackTest { assertFailsWith { hpackReader!!.readHeaders() }.also { expected -> - assertThat(expected.message).isEqualTo("Header index too large -2147483521") + assertThat(expected.message).isEqualTo("HPACK integer overflow") } } @@ -413,8 +413,18 @@ class HpackTest { assertFailsWith { hpackReader!!.readHeaders() }.also { expected -> - assertThat(expected.message) - .isEqualTo("Invalid dynamic table size update -2147483648") + assertThat(expected.message).isEqualTo("HPACK integer overflow") + } + } + + @Test + fun dynamicTableSizeUpdateRejectsWrappedSmallPositive() { + bytesIn.writeByte(0x3f) + bytesIn.write("ffffffff8f00".decodeHex()) + assertFailsWith { + hpackReader!!.readHeaders() + }.also { expected -> + assertThat(expected.message).isEqualTo("HPACK integer overflow") } } @@ -823,6 +833,38 @@ class HpackTest { .isEqualTo(0x7fffffff) } + @Test + fun overflowingValueRejected() { + assertFailsWith { + newReader(Buffer().write("8080808008".decodeHex())).readInt(0xff, 0x7f) + }.also { expected -> + assertThat(expected.message).isEqualTo("HPACK integer overflow") + } + } + + @Test + fun tooManyContinuationBytesRejected() { + assertFailsWith { + newReader(Buffer().write("808080808001".decodeHex())).readInt(0x1f, 0x1f) + }.also { expected -> + assertThat(expected.message).isEqualTo("HPACK integer overflow") + } + } + + @Test + fun stringLengthOverflowRejected() { + val source = Buffer() + source.writeByte(0x00) + source.writeByte(0x7f) + source.write("8080808008".decodeHex()) + + assertFailsWith { + newReader(source).readHeaders() + }.also { expected -> + assertThat(expected.message).isEqualTo("HPACK integer overflow") + } + } + @Test fun prefixMask() { hpackWriter!!.writeInt(31, 31, 0)