Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulnerability: Define a maximum length for requests #34

Open
goncalor opened this issue Apr 28, 2017 · 0 comments
Open

Vulnerability: Define a maximum length for requests #34

goncalor opened this issue Apr 28, 2017 · 0 comments

Comments

@goncalor
Copy link

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant