Jetty.project: Response header overflow leads to buffer corruptions

[nathankooij]

Jetty version
9.4.29 (but traced it back to 9.4.27)

Java version

openjdk 11.0.7 2020-04-14
OpenJDK Runtime Environment 18.9 (build 11.0.7+10)
OpenJDK 64-Bit Server VM 18.9 (build 11.0.7+10, mixed mode)

OS type/version
Debian GNU/Linux 10 (buster)

Description
We run several Spring Boot 2 applications, originally deployed as WAR files on a standalone Jetty server (9.4.15). Recently we changed these to use an embedded Jetty server, which included an upgrade to Jetty 9.4.28. Subsequently, we intermittently observed errors in one of our applications, serving an admin page using webjars. The logs indicated errors such as:

• org.eclipse.jetty.server.HttpChannel () : /path-to-a-webjar org.springframework.security.web.firewall.RequestRejectedException: The request was rejected because the HTTP method "HTGET" was not included within the whitelist [HEAD, DELETE, POST, GET, OPTIONS, PATCH, PUT]
  This was a valid request from the client side, but somehow the HTTP method got corrupted.

• o.e.j.u.thread.strategy.EatWhatYouKill (): java.lang.IllegalArgumentException: newPosition > limit: (212 > 0)
  Truncated this one, see below for an example of a stacktrace.

• org.eclipse.jetty.http.BadMessageException: 500: Response header too large

• o.e.j.u.thread.strategy.EatWhatYouKill () : java.nio.BufferOverflowException: null

The admin page itself would also become highly unresponsive, and some files would fail to be served. A restart of the application would fix the errors, but it would also resolve itself "eventually". Moreover, the errors' frequency grew with load, but we couldn't reproduce it consistently. This application in particular was affected the most, but we also observed the errors in other applications.

We managed to reproduce the issue by creating an endpoint in which we would explicitly send a response header larger than the configured maximum size (8KB), after which an application would immediately become unresponsive and all aforementioned errors would start appearing. We confirmed this behavior wasn't present in our previous Jetty version 9.4.15, and found it was first introduced in 9.4.27. (Perhaps related to https://github.com/eclipse/jetty.project/issues/4541?)

This might also be the same issue as observed in https://github.com/eclipse/jetty.project/issues/4828, but 9.4.29 which included the potential fix didn't resolve the issue for us.

We have for now resolved this internally by allowing bigger response headers, as we do have a use case for these internally.

I have provided below a test that can be used to trigger the behavior. It consists of sending a (too) large response header, after which we concurrently trigger many requests in order to reproduce the behavior. From my testing concurrency is necessary, which could explain why the error was more frequently observed in our webpage (it serves many webjars separately rather than bundling).

Lastly, from my brief foray into exploring this issue, in one of the cases I noticed that when the header overflow was triggered, which released the HttpConnection#_header buffer (but before any error handling was done), this same buffer was used by HttpParser#quickStart, but its limit was set to 0. These are two different requests, and could explain the HTTP method corruption for instance. So it seems that somehow the buffer cleanup in case of a header overflow is perhaps not correctly handled.

If you need any more information, please do let me know.

Example test
Please look at the logs to observe the triggered errors. Further below I have also provided some sample stacktraces that can be reproduced by this test.

    @Test
    public void testBufferCorruption() throws Exception
    {
        Server server = new Server();

        HttpConfiguration config = new HttpConfiguration();
        HttpConnectionFactory http = new HttpConnectionFactory(config);

        LocalConnector connector = new LocalConnector(server, http, null);
        connector.setIdleTimeout(5000);
        server.addConnector(connector);
        ErrorHandler eh = new ErrorHandler();
        eh.setServer(server);
        server.addBean(eh);

        byte[] bytes = new byte[8 * 1024];
        Arrays.fill(bytes, (byte)'X');
        final String longstr = "thisisastringthatshouldreachover8kbytes-" + new String(bytes, StandardCharsets.ISO_8859_1) + "_Z_";
        server.setHandler(new AbstractHandler()
        {
            @SuppressWarnings("unused")
            @Override
            public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
            {
                baseRequest.setHandled(true);
                response.setHeader(HttpHeader.CONTENT_TYPE.toString(), MimeTypes.Type.TEXT_HTML.toString());
                response.setHeader("LongStr", longstr);
                PrintWriter writer = response.getWriter();
                writer.write("<html><h1>FOO</h1></html>");
                writer.flush();
                response.flushBuffer();
            }
        });
        server.start();

        ExecutorService executorService = Executors.newFixedThreadPool(8);

        for (int i = 0; i < 500; ++i) {
            executorService.submit(() ->
                    connector.getResponse("GET / HTTP/1.1\r\n" +
                            "Host: localhost\r\n" +
                            "\r\n"));
        }

        executorService.awaitTermination(5, TimeUnit.SECONDS);
    }

Example stacktraces

2020-06-03 15:48:13.693:WARN:oejut.QueuedThreadPool:qtp954702563-33: 
java.nio.BufferOverflowException
    at java.base/java.nio.HeapByteBuffer.put(HeapByteBuffer.java:216)
    at org.eclipse.jetty.util.BufferUtil.put(BufferUtil.java:399)
    at org.eclipse.jetty.util.BufferUtil.append(BufferUtil.java:484)
    at org.eclipse.jetty.io.ByteArrayEndPoint.fill(ByteArrayEndPoint.java:398)
    at org.eclipse.jetty.server.HttpConnection.fillRequestBuffer(HttpConnection.java:336)
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:254)
    at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
    at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
    at org.eclipse.jetty.io.ByteArrayEndPoint$1.run(ByteArrayEndPoint.java:76)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:806)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:938)
    at java.base/java.lang.Thread.run(Thread.java:834)

    java.lang.ArrayIndexOutOfBoundsException: arraycopy: last destination index 8194 out of bounds for byte[8192]
    at java.base/java.lang.System.arraycopy(Native Method)
    at java.base/java.nio.HeapByteBuffer.put(HeapByteBuffer.java:234)
    at org.eclipse.jetty.util.BufferUtil.put(BufferUtil.java:392)
    at org.eclipse.jetty.util.BufferUtil.append(BufferUtil.java:484)
    at org.eclipse.jetty.io.ByteArrayEndPoint.fill(ByteArrayEndPoint.java:398)
    at org.eclipse.jetty.server.HttpConnection.fillRequestBuffer(HttpConnection.java:336)
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:254)
    at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
    at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
    at org.eclipse.jetty.io.ByteArrayEndPoint$1.run(ByteArrayEndPoint.java:76)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:806)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:938)
    at java.base/java.lang.Thread.run(Thread.java:834)

    java.lang.IllegalArgumentException: newPosition > limit: (5 > 0)
    at java.base/java.nio.Buffer.createPositionException(Buffer.java:318)
    at java.base/java.nio.Buffer.position(Buffer.java:293)
    at java.base/java.nio.ByteBuffer.position(ByteBuffer.java:1086)
    at java.base/java.nio.ByteBuffer.position(ByteBuffer.java:262)
    at org.eclipse.jetty.util.BufferUtil.flipToFlush(BufferUtil.java:219)
    at org.eclipse.jetty.http.HttpGenerator.generateResponse(HttpGenerator.java:459)
    at org.eclipse.jetty.server.HttpConnection$SendCallback.process(HttpConnection.java:743)
    at org.eclipse.jetty.util.IteratingCallback.processing(IteratingCallback.java:241)
    at org.eclipse.jetty.util.IteratingCallback.iterate(IteratingCallback.java:223)
    at org.eclipse.jetty.server.HttpConnection.send(HttpConnection.java:549)
    at org.eclipse.jetty.server.HttpChannel.sendResponse(HttpChannel.java:833)
    at org.eclipse.jetty.server.HttpChannel.sendResponse(HttpChannel.java:851)
    at org.eclipse.jetty.server.HttpChannel.onBadMessage(HttpChannel.java:784)
    at org.eclipse.jetty.server.HttpChannelOverHttp.badMessage(HttpChannelOverHttp.java:283)
    at org.eclipse.jetty.http.HttpParser.badMessage(HttpParser.java:1628)
    at org.eclipse.jetty.http.HttpParser.parseNext(HttpParser.java:1610)
    at org.eclipse.jetty.server.HttpConnection.parseRequestBuffer(HttpConnection.java:364)
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:261)
    at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
    at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
    at org.eclipse.jetty.io.ByteArrayEndPoint$1.run(ByteArrayEndPoint.java:76)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:806)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:938)
    at java.base/java.lang.Thread.run(Thread.java:834)

[joakime]

We've been able to replicate with a variant of your test case. (It proved very useful! Thanks!)

We have a general fix working currently, but are reviewing other buffer pool usages for similar issues.

[gregw]

Thanks for the detailed bug report, test case and initial analysis.
You were right and it was the change from #4541. The issue was in the case of a buffer overflow, we released the header buffer, but did not null the field. We then threw an exception and the onCompleteFailure handling called release on the class, which again released the buffer (since it was not null). Thus the buffer was put into the pool twice and could be taken out by 2 threads and worked on at the same time. The server would have been unstable from that point on!

Your good report has enabled a quick fix and we'll get a release out asap!

[nathankooij]

Thanks for the quick turnaround and clear explanation! Looking forward to the release.

[joakime]

Merged into jetty-9.4.x branch (and merged up into jetty-10.0.x and jetty-11.0.x as well).

[msatam]

We have identified that the bug mentioned here actually has security implications as well. It turned out that corrupt HTTP response buffer unfortunately is sent back in a lot of cases to different clients when the server is under heavy load and ends up sending incorrect responses to different clients. We had a few cases, where due to this particular flaw, clients were able to jump sessions thereby having cross account access. This happens since sometimes the buffer is not completely corrupted and ends up there by sending authentication cookies from one user's response to another user thereby allowing user A to jump in user B's session. We were running version 9.4.28 when this started occuring. As a result we have rolled back to an earlier version where we did not see the bug. I think opening up a CVE for this bug indicating the potential for security implications might be a good idea..

[gregw]

Good idea.

[WalkerWatch]

A CVE was filed for this issue, CVE-2019-17638, and the issue was resolved in 9.4.30.v20200611.

[hossman]

Why does the linked CVE indicate that "in case of too large response headers, Jetty throws an exception to produce an HTTP 431 error." ?

• as far as i can tell from manual testing and a casual reviewing of the code/test modified in this issue, "Response header too large" always results in a 500 error
  
  
  
  • throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Response header too large");
  
  
  • assertThat(response.getStatus(), is(500));
  
  
• if the application generates _response_ headers that are too big, 431 wouldn't be appropriate - it's only for usewhen _request_ headers are too large

[gregw]

@hossman, you are correct. We return a 500 and did so before the CVE and fix as well.
@WalkerWatch can you ask for the text of the CVE to be updated?