Vulnerability: Define a maximum length for requests
Opened this issue · 0 comments
I was starting to implement a simple HTTP application using libsyncd
when I noticed currently there seems to be no mechanism to limit the size of requests. This can be exploited by an attacker by sending an arbitrary amount of data to a server. While the user may provide code to impose a limit for simple TCP it seems to me that is not the case for HTTP requests using ad_http_handler()
.
Explanation
To process a HTTP request ad_http_handler()
can be used. This function calls http_parser()
, which returns AD_TAKEOVER
except when the request is complete (both header and body have been received). Therefore, the user's callbacks are not called while the request is not complete, which means
- the user cannot verify how much data has been received;
- all the received data will stay in the RAM while the request is not complete (since the user's callbacks aren't called to process the data).
This leads to a simple attack vector enabling to exhaust the memory of a server.
Mitigation
The best way I could think of to mitigate this vulnerability (of course you might have a better one) is to define a maximum request length per server. A default value should be provided (for example 10 MB) which the user could override with ad_server_set_option()
. This would "protect" both raw TCP and HTTP(S) applications.
At first I thought of rejecting requests based on the Content-Length
header, but
- raw TCP would be left unprotected (unless the user implemented a limit, which he might not do because he did not thing about this type of attack)
Content-Length
refers only to the body of the request, meaning the headers would still be vulnerableContent-Length
is not absolutely required by HTTP/1.1
However, using the content length header might still be useful for early rejections: for example if Content-Length
states the body will contain 1 GB but our server only accepts 10 MB then the request could be rejected right away.
As a last note: when refusing a HTTP request for being too long, a response with a HTTP 413 status code could be sent before closing the connection (so that the client is not left in the dark as to why it was closed).
I hope this can help improving the security of libasyncd
:)