Reject overflowing HPACK integers (#9401)

Decode HPACK integers in Long and reject values that would overflow Int.

This prevents malformed header indexes, dynamic table size updates, and string lengths from wrapping into negative or attacker-chosen small positive values.

Author: oreparaz

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. */

okhttp/src/jvmTest/kotlin/okhttp3/internal/http2/HpackTest.kt

@@ -375,7 +375,7 @@ class HpackTest {
     assertFailsWith<IOException> {
       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<IOException> {
       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<IOException> {
+      hpackReader!!.readHeaders()
+    }.also { expected ->
+      assertThat(expected.message).isEqualTo("HPACK integer overflow")
     }
   }
 
@@ -823,6 +833,38 @@ class HpackTest {
       .isEqualTo(0x7fffffff)
   }
 
+  @Test
+  fun overflowingValueRejected() {
+    assertFailsWith<IOException> {
+      newReader(Buffer().write("8080808008".decodeHex())).readInt(0xff, 0x7f)
+    }.also { expected ->
+      assertThat(expected.message).isEqualTo("HPACK integer overflow")
+    }
+  }
+
+  @Test
+  fun tooManyContinuationBytesRejected() {
+    assertFailsWith<IOException> {
+      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<IOException> {
+      newReader(source).readHeaders()
+    }.also { expected ->
+      assertThat(expected.message).isEqualTo("HPACK integer overflow")
+    }
+  }
+
   @Test
   fun prefixMask() {
     hpackWriter!!.writeInt(31, 31, 0)