[Bug 65340] Hpack decode NegativeArraySizeException: -1

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=65340

--- Comment #10 from Thomas <qingdaoh...@163.com> ---
I have reproduced the issue, please see the detail as below.
Reason description:
When the length of one header value is greater than 127 and its first prefix
byte is the last one in org.apache.coyote.http2.Http2Parser#headerReadBuffer,
then the return value of org.apache.coyote.http2.Hpack#decodeInteger will be
-1. And then the exception "java.lang.NegativeArraySizeException: -1" is thrown
in org.apache.coyote.http2.HpackDecoder#readHpackString, because the length is
-1.

Reproduce steps:
example project: https://github.com/qingdaoheze/tomcat-hpack-error-reproduce
1.Start up the tomcat server through
tomcat-server-gzip/src/main/java/org/sample/http2/tomcat/TomcatHttp2Main.java
2.Run the code in
jetty9-client-sample/src/main/java/org/sample/jetty/client/http2/HttpClientWithHttp2Transport.java
3.The exception java.lang.NegativeArraySizeException will be thrown.

the hex string of the byte array in
org.apache.coyote.http2.Http2Parser#headerReadBuffer is:
40fffb068e46638e566469c91d68653c9e2495a1b3196312c3090be0b2a249505f1297c224951297c4a89252f897c1632b4c8da7e575f68fda4eb28e4ae5e883492a28eb24bd11d68925e88925107a78f03eb47493ad08e92d1d51d3ad7e0a7322497a76acd24bd04fb3274349d251d6535bd2c0acf099b0b28fec6892529fc19124b251d472c8897a483e9594de25133d1e7e57ad7de174968474944fb32f413f5cbd04cbef4bd052f447495f132f7495fe8ff443fd257c4cf4947d24e80133d2467a4bd0e6376f1d257fa044fb2f3dd257de643d1f97a3e1e92859ec6f12747c2f0a4e881c87ede0dd2590de0fd25ec63f494e6ede0c7e92f63f829ba4a7f06ec7e92f63f837494fd8de2539bb7831e5ec63f494d234e48eb4d3258c4309232b4c8da7e575f68fda4eb28e4ae5e883492a28eb24bd11d68925e88925e83eb47493ad08e92d1d51d3ad7e0a7322497a76acd24bd04fb3274349d251d6535bd2c0acf099b0b28fec6892529fc19124b251d472c8897a483e9594de25133d1e7e57ad7de174968474944fb32f413f5cbd04cbef4bd052f447495f132f7495fe8ff443fd257c4cf4947d24e80133d2467a4bd0e6376f1d257fa044fb2f3dd257de643d1f97a3e1e92859ec6f12747c2f0a4e881c87ede0dd2590de0fd25ec63f494e6ede0c7e92f63f829ba4a7f06ec7e92f63f837494fd8de2539bb7831e5ec63f494d234e48eb4d3258c4309232b4c8da7e575f68fda4eb28e4ae5e883492a28eb24bd11d68925e88925e83eb47493ad08e92d1d51d3ad7e0a7322497a76acd24bd04fb3274349d251d6535bd2c0acf099b0b28fec6892529fc19124b251d472c8897a483e9594de25133d1e7e57ad7de174968474944fb32f413f5cbd04cbef4bd052f447495f132f7495fe8ff443fd257c4cf4947d24e80133d2467a4bd0e6376f1d257fa044fb2f3dd257de643d1f97a3e1e92859ec6f12747c2f0a4e881c87ede0dd2590de0fd25ec63f494e6ede0c7e92f63f829ba4a7f06ec7e92f63f837494fd8de2539bb7831e5ec63f494c7ede3a044fb3d25e881f4394dd3c4a7376e9298fd1979f0cbf63e9257d2514af00afbcc87a3f2f47c3d250b3d8de24e8f85e149d10390fdbc1ba4b21bc1fa4bd8c7e929cddbc18fd25ec7f0537494fe0dd8fd25ec7f06e929fb1bc4a7376f063cbd8c7e9298fdbc74089f67a4bd103e8729ba7894e6edd2531fa32f3e197ec7d24afa4a295e015f7990f47e5e8f87a4a167b1bc49d1f0bc293a20721fb783749643783f497b18fd2539bb7831fa4bd8fe0a6e929fc1bb1fa4bd8fe0dd253f637894e6ede0c797b18fd2531fb78e8113ecf497a207d0e5374f129cddba4a63f465e7c32fd8fa495f49452bc02bef321e8fcbd1f0f4942cf637893a3e178527440e43f6f06e92c86f07e92f631fa4a7fffd

Suggested solution(modify
org.apache.coyote.http2.HpackDecoder#readHpackString)：
    private String readHpackString(ByteBuffer buffer) throws HpackException {
        if (!buffer.hasRemaining()) {
            return null;
        }
        byte data = buffer.get(buffer.position());

        int length = Hpack.decodeInteger(buffer, 7);
        if (buffer.remaining() < length || length == -1) {
            return null;
        }
        boolean huffman = (data & 0b10000000) != 0;
        if (huffman) {
            return readHuffmanString(length, buffer);
        }
        StringBuilder stringBuilder = new StringBuilder(length);
        for (int i = 0; i < length; ++i) {
            stringBuilder.append((char) buffer.get());
        }
        return stringBuilder.toString();
    }
the changed content is adding " || length == -1" in the following line.
if (buffer.remaining() < length || length == -1)


Summary, I suggest that:
1.Fix the bug using the alike solution mentioned above.
2.Expand the default size of
org.apache.coyote.http2.Http2Parser#headerReadBuffer, for example,
maxHeaderSize of the server.

@Mark Thomas

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org