Security issue: how to avoid payload attacks for the HTTP server

[zhangkaizhao]

To avoid payload attacks for the HTTP server, a configurable variable to limit the size of HTTP client body size is needed for each HTTP request. But I do not find it out from the HTTP module and all its sub-modules.

There are HTTP::MAX_REQUEST_LINE_SIZE for request line and HTTP::MAX_HEADERS_SIZE for headers already but none for request body yet.

The well-known web frameworks Kemal, Amber and Lucky in Crystal all use the std library for HTTP request parsing, and then they all have this security issue.

I think this is a very important issue but it seems that no one has talken about it yet. Do I miss something for this issue?

---

How others fix this issue?

• Nginx: http://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size
• Apache: https://httpd.apache.org/docs/2.4/mod/core.html#limitrequestbody
• Django: https://docs.djangoproject.com/en/3.1/ref/settings/#data-upload-max-memory-size
• Flask: https://flask.palletsprojects.com/en/1.1.x/patterns/fileuploads/#improving-uploads
• ...

[jhass]

The HTTP::Server API does not read the request body into memory, it stops reading from the client socket after the headers.

So this is the job of whatever is reading the request body into memory.

Arguably stdlib parsing things (JSON, YAML, HTTP::Formdata etc) could support an easy way to limit how much data they read from an IO, but this can already also quite easily achieved by wrapping the request body IO into an IO::Sized.

[straight-shoota]

It might still be a good idea to install a limit in HTTP::Server as a sensible default. Users of HTTP::Server API probably don't address this issue when consuming the request body.
Wrapping the request IO in IO::Sized seems like a good idea. Plus a config value which allows for fine tuning (and disabling).

[jhass]

So, what's a reasonable default limit?

[jhass]

Also IO::Sized just pretends the stream is shorter than it actually is. How do we signal consumers that they've run into the limit? Should HTTP::Server signal back to the client that it breached the limit or is that the consumer's job? If HTTP::Server signals, how can the consumer override that behavior?

[straight-shoota]

Yeah, maybe IO::Sized won't work for this and we need a custom solution. Shouldn't be hard to do.

How do we signal consumers that they've run into the limit?

It should probably raise when a consumer tries to read over the limit.

Should HTTP::Server signal back to the client that it breached the limit or is that the consumer's job?

HTTP::Server enforces this limit so it probably should also be responsible to communicate to the client. But if the handler rescues the exception and sends a custom response, the server wouldn't.

If HTTP::Server signals, how can the consumer override that behavior?

What behaviour are you referring to? The signalling or the limiting itself? The limit can be set and disabled via server configuration. The signalling on enforced limit can't be overridden. You have to disable the limit instead.

[jhass]

So, of course my actual point here is that there's a lot of application specific decision in here.

[straight-shoota]

I don't think so. Standalone webservers/load balancers like apache, nginx, traefik implement this. So it surely can work without application specific decisions. The only application specific setting is enabling this and the actual limit value.

[straight-shoota]

Of course, if you need more specific behaviour like dynamic limits based on request properties, you'll have to deactivate the server-wide configuration and implement your custom limit enforcement. But for most users, a standard behaviour with a good default value should really be all they need and it would be great to have this automatically included.

[asterite]

This is very easy to implement or design: just check how Go (or any other web server) does it 😉

[straight-shoota]

It seems Go's net/http server doesn't have this by default. You need to use MaxBytesReader in your handler.

[j8r]

No need to have IO::Sized. As said in #9901, if body is exposed as an IO, it will be very simple to limit the body size with IO#gets(limit) : String?.

[asterite]

@j8r If you have an endpoint where you allow users to upload a file, you will most likely be using IO.copy(...) to stream the request to the file. If the file is large enough (the client could just be streaming infinitely) then it will fill out your disk.

I think the way Go does it is great. You can control this in every endpoint you need it, probably only those that do stream a request somewhere...

[jhass]

To clarify, that's why IO.copy has this handy optional argument to limit the length that's being copied.

[didactic-drunk]

Shouldn't hitting a limit indicate an error rather than writing a truncated (possibly corrupt) file?