bound the request body size read by RPCServletUtils

[alphacharlie-dev]

RPCServletUtils.readContent reads the request body into a ByteArrayOutputStream in a loop that
only terminates at end-of-stream, then materialises the result as a String:

while (true) {
  int byteCount = in.read(buffer);
  if (byteCount == -1) {
    break;
  }
  out.write(buffer, 0, byteCount);
}
return new String(out.toByteArray(), getCharset(expectedCharSet));

Nothing consults Content-Length and no running total is kept, so the peak footprint is roughly
twice the body size with no ceiling. The method is reached without authentication from both
protocols — AbstractRemoteServiceServlet:182 → readContentAsGwtRpc, and
RequestFactoryServlet:129 — so one large POST can drive the JVM to OutOfMemoryError.

Servlet container max-post-size settings generally don't help, because they apply to
form-encoded bodies rather than text/x-gwt-rpc or application/json. The existing
exceedsUncompressedContentLengthLimit helper gates response compression and has no bearing here.

The change

Reject up front when Content-Length already exceeds the limit, and keep a running total inside
the loop so chunked transfers and an understated header are bounded too. Default 1 MiB, raisable
via the gwt.rpc.maxRequestBodyBytes system property; an unparseable value falls back to the
default rather than disabling the check.

Compatibility

Applications sending payloads above 1 MiB will need that property set. 1 MiB is a proposal, not a
measured threshold — you know real-world GWT-RPC payload sizes far better than I do, and I'd
rather take a maintainer's number than defend mine.

Testing

RPCServletUtilsTest passes unchanged, including all six testContentLength* buffer-boundary
cases that exercise this exact method. I also checked the boundary directly: a body of exactly
1 MiB is accepted, one byte over is rejected.

Full JRE suite (ant -f user/build.xml test.nongwt): 25 suites / 2701 tests / 0 failures,
identical to the main baseline. I did not run the GWT-mode/browser suites.

Provenance

Found during a security review of GWT 2.13.1, confirmed still present on main. I have a standalone
reproduction (a generator ServletInputStream feeding 8 MB, asserting it is buffered in full) and
a test asserting the secure behaviour, both runnable against a plain checkout — happy to attach or
open a matching issue if that's useful. I have not attempted to demonstrate actual heap exhaustion;
the scaling argument is analytical.

Reported to the Google OSS VRP, which declined it on repository-tier grounds rather than on the
merits and suggested bringing it here.

[niloc132]

Can you discuss more exactly how you confirmed this? Unless I'm very much mistaken, the servlet container should be managing the content-length header (or chunked encoding, which means no header, or h2, which means the size of the h2 DATA frame is part of the binary format and not a proper header) and should be transparent to the application (or in our case, the toolkit code that decodes the stream into objects)?

Heap exhaustion shouldn't be necessary to confirm, but the ServletRequest.getInputStream() should be correctly bounded to the actual request size. If you discover that it isn't, that seems like it must be a bug in the servlet container, not GWT, as keep-alive, pipelining, etc cannot work correctly with incorrect content-length.

I suspect that if there is a content vs content-length mismatch, you have found a bug in the servlet container you are testing with rather than gwt-rpc's implementation.

[alphacharlie-dev]

Hi Colin — let me take your points one at a time.

1. "Can you discuss more exactly how you confirmed this?"

You're right that heap exhaustion isn't required to confirm it — so I didn't actually OOM anything. The test is a deliberately bounded 8 MB probe. It calls RPCServletUtils.readContent directly with a ServletInputStream that generates 8 MB on the fly (the attacker doesn't even have to hold the bytes in memory), behind a mock HttpServletRequest whose getContentLength() returns -1 — i.e. no Content-Length header, which is exactly the chunked case you mentioned.

On unpatched main, readContent buffers the whole 8 MB into a returned String of length 8,388,608 and throws nothing:

FAIL FIX-003: oversized body fully buffered (len=8388608); no cap.

That proves the read loop has no break conditioned on accumulated size:

while (true) {
    int byteCount = in.read(buffer);
    if (byteCount == -1) break;     // only EOF stops it — never size
    out.write(buffer, 0, byteCount);
}
return new String(out.toByteArray(), charset);

Whatever the stream yields, GWT buffers — so a multi-GB body scales straight to OOM (recorded analytically; no need to actually crash a box). After the patch, the same probe is rejected:

ServletException: Request body exceeds the maximum of 1048576 bytes

And the regression suite is clean — ant test.nongwt: 25 suites / 2701 tests / 0 failures, matching main, including all 16 RPCServletUtilsTest cases.

Worth noting: the test isolates GWT's own code path — it isn't claiming a container misbehaves, it just shows that GWT itself has no bound. Whether the container supplies one is points 3–5 below.

2. "The container manages content-length / chunked / h2, and that should be transparent to the app."

You're right that the container handles the transport framing faithfully — I'm not disputing that. The disagreement is about what it bounds. The container guarantees it delivers exactly the bytes the client framed; it does not guarantee those bytes are small. The size is chosen by whoever sends the request. "Bounded to the actual request size" is true, but the actual request size is attacker-controlled, and GWT buffers all of it. HTTP/2 is the same story — the container reassembles all the DATA frames and hands the total over; the total is still attacker-controlled.

3. "getInputStream() should be correctly bounded to the actual request size."

It is — but that size is whatever the client declared or streamed, and two attacker paths need no container bug:

• Chunked encoding. With Transfer-Encoding: chunked there is no Content-Length header at all — which is exactly what the probe in point 1 simulates (getContentLength() == -1). The body is as large as the attacker cares to stream, the container passes it through, and GWT buffers it. The existing comment in readContent already acknowledges we can't rely on a Content-Length: "Need to support 'Transfer-Encoding: chunked', so do not rely on presence of a 'Content-Length' request header."
• A large, honest Content-Length. The client sends Content-Length: 2000000000 and then ~2 GB of body. The container delivers all of it correctly (keep-alive and pipelining still work, because the length is honest), and GWT buffers all of it. No mismatch, no bug — just a big request.

4. "If it isn't bounded, that's a container bug — keep-alive and pipelining can't work with an incorrect content-length."

I agree — and that's exactly the point: those mechanisms require a correct content-length, so a malicious client has every reason to send a correct one, just very large. The attack doesn't use an incorrect length, so the keep-alive/pipelining argument doesn't apply to it. The client follows the transport rules and still fills the heap, because nothing on the GWT side says "a request this big isn't legitimate."

5. "A content vs content-length mismatch would be a bug in the servlet container, not gwt-rpc."

Agreed — but I'm not relying on a mismatch. The probe uses the no-Content-Length (chunked) case, and the realistic attack uses either that or an honest large Content-Length. In both, the bytes delivered match what was framed; the container is doing its job. The only problem is that GWT buffers an unbounded amount before it ever looks at the payload. (And for completeness, the only existing size check in this file — exceedsUncompressedContentLengthLimit — gates response compression, not request size, and the same readContent path is shared by RequestFactory, not just GWT-RPC.)

So why fix it in GWT rather than rely on the container?

Container body limits exist (Tomcat maxPostSize, Jetty's limits, etc.), but they're opt-in, they differ between containers and versions, and their defaults typically apply to application/x-www-form-urlencoded form parsing — not to text/x-gwt-rpc or application/json raw body reads. GWT is a library that runs in whatever container the application picked, so it can't assume every deployment remembered to configure a limit for this content type. Capping readContent is defense in depth at a boundary we already treat as security-sensitive, and it protects every deployment regardless of container config. The cap is configurable (gwt.rpc.maxRequestBodyBytes, default 1 MiB) so apps with legitimately large payloads can raise it — happy to adjust the default or the mechanism if you'd prefer a different shape.

[niloc132]

So this is just "If an attacker sends 1gb, the server receives 1gb, and that's bad"? You have described (but not provided?) a test case where a ServletInputStream instance is created that provides too much data, but no real examples (though you offered, and I asked).

Whatever the stream yields, GWT buffers — so a multi-GB body scales straight to OOM (recorded analytically; no need to actually crash a box).

Integer.MAX_VALUE is only 2gb, that's going to crash a server? BAOS will definitely throw OOM if you ask for that much (real limit is about Integer.MAX_VALUE - 2 or so), and asking for a little less might take some time (getting into DoS territory). Compression enters the picture and lets the client send less to use more bytes - but I'm still not sure how we can comprehensively solve both usability and security sides here:

1mb is a "medium sized" message to send over gwt-rpc, I definitely wouldn't feel comfortably imposing limits here until we had some kind of super-linear growth bug. I think there are several appropriate places to define a limit like this without baking assumptions like this into GWT itself:

• Proxy? Most applications live behind a proxy in my experience, often for checkbox-checking exercises, but also to unify things like certificate handling. These uniformly have ways to limit request sizes
• Servlet container: provide configuration across all requests to limit uploads. Each container is different, example Jetty documentation can be found at https://jetty.org/docs/jetty/12.1/operations-guide/modules/standard.html#size-limit
• Application level filter: a simple filter can be written that checks (as this patch does) the incoming content length. This still leaves chunked encoding, which can be also checked through a HttpServletRequestWrapper that provides a length-limiting ServletInputStream (exercise left to reader or their llm of choice). This could allow limitations to be based on ACLs or URLs.
• Servlet-level limitations: A given RemoteServiceServlet might be the best way to limit sizes. The protected method String readContent(HttpServletRequest) is what calls into RPCServletUtils and provides another chance to check/wrap the incoming stream, or provide an alternative implementation.

I think a reasonable case could be made both for and against just about any arbitrary size you could specify here (100k? 10mb? 100mb?), and arguably it would need to be defined per-servlet, possibly subject to some access controls (rather than a single system property to rule them all). I don't think we can reasonably take this change with some specific, concrete issues.

[alphacharlie-dev]

Colin — fair, and I think you're right on the substance. Let me concede the points rather than keep defending the change as-is:

• It's linear, not super-linear: a request body is roughly 1 byte in ≈ 1 byte buffered, with no amplification (and browsers don't gzip gwt-rpc requests, so there's no compression angle on this path either). The per-request ceiling is also effectively ~Integer.MAX_VALUE because of the byte[]/String backing, so the real exposure is concurrency — N clients each pushing ~2GB — which is exactly what edge body-size limits and rate limiting are for. That's an operational/infra concern, not a library bug.
• 1 MiB is too blunt a default and, as you say, below what legitimate calls can send. I take the "medium-sized message" point.
• The layering you lay out (proxy / container / filter / servlet-level override) is the right place, and RemoteServiceServlet.readContent is the obvious in-framework hook.

The one thing I'd still flag — not to relitigate, but because it informs what would be useful — is that the "the container already bounds it" assumption has a real gap for this content type: Tomcat's maxPostSize (and the analogous defaults in other containers) apply to application/x-www-form-urlencoded form parsing, not to raw text/x-gwt-rpc / application/json body reads. So out-of-the-box this path often isn't bounded. But that's an argument for making a limit easy to opt into, not for baking one in — which I think is your point.

To that end: I'll skip the end-to-end PoC. I'd been about to build one, but you've correctly placed linear DoS outside GWT's scope, so the PoC would only demonstrate the thing you've already said isn't GWT's job — not worth either of our time.

What I'd like to do instead is gut the PR down to something purely opt-in, with no default and no behavior change unless an app asks for it:

• a readContent(HttpServletRequest, int maxContentLength) overload (or a small length-limiting ServletInputStream wrapper) that a servlet/filter can opt into, and
• a short note on RemoteServiceServlet.readContent pointing deployers at the container/filter options and the opt-in helper.

No system property, no default cap, nothing breaks. If even that's more than you want in-framework, I'm happy to close this and drop the pattern as a wiki/gist writeup instead — let me know which you'd prefer and I'll rework (or close) accordingly.

[zbynek]

something purely opt-in, with no default and no behavior change

In case you're using GWT yourself, what problem does it solve for you, that you can't solve with the container settings mentioned above?
If not, do you see any indication from the community that someone needs that?

Increasing complexity of the system for something that can be easily achieved in other ways is IMHO not worth it.

It might make sense to make a documentation PR mentioning this concern and the Jetty / Tomcat properties that allow to mitigate it, possibly as a paragraph in https://www.gwtproject.org/doc/latest/DevGuideServerCommunication.html