wolkykim/libasyncd

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 vulnerable
  • Content-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 :)