This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push: new 9e32edbf8a Fix BZ 69762 - overflow during HPACK integer decoding 9e32edbf8a is described below commit 9e32edbf8aeefd85ed6d89e431a9013d6076d615 Author: Mark Thomas <ma...@apache.org> AuthorDate: Wed Jul 30 17:01:40 2025 +0100 Fix BZ 69762 - overflow during HPACK integer decoding The valid maximum value hasn't changed. This just rejects attempts to exceed it. https://bz.apache.org/bugzilla/show_bug.cgi?id=69762 --- java/org/apache/coyote/http2/Hpack.java | 28 +++++++++--- .../apache/coyote/http2/LocalStrings.properties | 1 + test/org/apache/coyote/http2/TestHpack.java | 51 ++++++++++++++++++++++ webapps/docs/changelog.xml | 5 +++ 4 files changed, 78 insertions(+), 7 deletions(-) diff --git a/java/org/apache/coyote/http2/Hpack.java b/java/org/apache/coyote/http2/Hpack.java index b0ec92e97c..08d5d2555e 100644 --- a/java/org/apache/coyote/http2/Hpack.java +++ b/java/org/apache/coyote/http2/Hpack.java @@ -26,8 +26,16 @@ final class Hpack { private static final byte LOWER_DIFF = 'a' - 'A'; static final int DEFAULT_TABLE_SIZE = 4096; - private static final int MAX_INTEGER_OCTETS = 8; // not sure what a good value for this is, but the spec says we - // need to provide an upper bound + /* + * The HPack specification says there SHOULD be an upper bound on this. + * + * Tomcat has opted to limit values to INTEGER.MAX_VALUE. Give that there is the prefix byte and then each octet + * provides up to 7-bits, a total a 5 octets plus the prefix may be required. + * + * Note: The maximum value represented by 5 octets is greater than INTEGER.MAX_VALUE. + * + */ + private static final int MAX_INTEGER_OCTETS = 5; /** * table that contains powers of two, used as both bitmask and to quickly calculate 2^n @@ -151,10 +159,12 @@ final class Hpack { int sp = source.position(); int mask = PREFIX_TABLE[n]; - int i = mask & source.get(); + // Use long internally as the value may exceed Integer.MAX_VALUE + long result = mask & source.get(); int b; - if (i < PREFIX_TABLE[n]) { - return i; + if (result < PREFIX_TABLE[n]) { + // Casting is safe as result must be less than 255 at this point. + return (int) result; } else { int m = 0; do { @@ -169,11 +179,15 @@ final class Hpack { return -1; } b = source.get(); - i = i + (b & 127) * (PREFIX_TABLE[m] + 1); + result = result + (b & 127) * (PREFIX_TABLE[m] + 1); + if (result > Integer.MAX_VALUE) { + throw new HpackException(sm.getString("hpack.integerEncodedTooBig")); + } m += 7; } while ((b & 128) == 128); } - return i; + // Casting is safe as result must be less than Integer.MAX_VALUE at this point + return (int) result; } /** diff --git a/java/org/apache/coyote/http2/LocalStrings.properties b/java/org/apache/coyote/http2/LocalStrings.properties index 61cb8497bf..bfcd3f6d39 100644 --- a/java/org/apache/coyote/http2/LocalStrings.properties +++ b/java/org/apache/coyote/http2/LocalStrings.properties @@ -37,6 +37,7 @@ frameType.checkPayloadSize=Payload size of [{0}] is not valid for frame type [{1 frameType.checkStream=Invalid frame type [{0}] hpack.integerEncodedOverTooManyOctets=HPACK variable length integer encoded over too many octets, max is [{0}] +hpack.integerEncodedTooBig=The maximum permitted value of an HPACK encoded variable length integer is Integer.MAX_VALUE hpack.invalidCharacter=The Unicode character [{0}] at code point [{1}] cannot be encoded as it is outside the permitted range of 0 to 255. hpackEncoder.encodeHeader=Encoding header [{0}] with value [{1}] diff --git a/test/org/apache/coyote/http2/TestHpack.java b/test/org/apache/coyote/http2/TestHpack.java index 0d88ed112c..9d39d4acf2 100644 --- a/test/org/apache/coyote/http2/TestHpack.java +++ b/test/org/apache/coyote/http2/TestHpack.java @@ -147,4 +147,55 @@ public class TestHpack { decoder.decode(output); Assert.assertEquals(headerValue, headers2.getHeader(headerName)); } + + + @Test + public void testDecodeIntegerMaxValue() throws HpackException { + ByteBuffer bb = ByteBuffer.allocate(9); + bb.put((byte) 255); + bb.put((byte) 254); + bb.put((byte) 255); + bb.put((byte) 255); + bb.put((byte) 255); + bb.put((byte) 7); + bb.position(0); + + Assert.assertEquals(Integer.MAX_VALUE,Hpack.decodeInteger(bb, 1)); + } + + + @Test(expected = HpackException.class) + public void testDecodeIntegerMaxValuePlus1() throws HpackException { + ByteBuffer bb = ByteBuffer.allocate(9); + bb.put((byte) 255); + bb.put((byte) 255); + bb.put((byte) 255); + bb.put((byte) 255); + bb.put((byte) 255); + bb.put((byte) 7); + bb.position(0); + + Hpack.decodeInteger(bb, 1); + } + + + @Test(expected = HpackException.class) + public void testDecodeIntegerZeroValues() throws HpackException { + ByteBuffer bb = ByteBuffer.allocate(12); + bb.put((byte) 255); + bb.put((byte) 128); + bb.put((byte) 128); + bb.put((byte) 128); + bb.put((byte) 128); + bb.put((byte) 128); + bb.put((byte) 128); + bb.put((byte) 128); + bb.put((byte) 128); + bb.put((byte) 128); + bb.put((byte) 128); + bb.put((byte) 128); + bb.position(0); + + Hpack.decodeInteger(bb, 1); + } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 38d114e626..fd379a6fd5 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -120,6 +120,11 @@ HTTP/1.1 following an async request, which was present for AJP. (remm/markt) </fix> + <fix> + <bug>69762</bug>: Fix possible overflow during HPACK decoding of + integers. Note that the maximum permitted value of an HPACK decoded + integer is <code>Integer.MAX_VALUE</code>. (markt) + </fix> </changelog> </subsection> <subsection name="Cluster"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org