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

Upload delay resulting in DoS #33

Open
mmmds opened this issue Feb 17, 2020 · 3 comments
Open

Upload delay resulting in DoS #33

mmmds opened this issue Feb 17, 2020 · 3 comments

Comments

@mmmds
Copy link

mmmds commented Feb 17, 2020

There is a vulnerability which allows to perform DoS attack against the application
server. The problem lies in handling delay parameter when upload is
initiated (gwtupload.server.UploadServlet#parsePostRequest). Value from
this parameter is used as an argument for Thread.sleep invocation.
Malicious user can specify even max integer value 2147483647, which
would cause a thread to sleep for almost 25 days
(gwtupload.server.AbstractUploadListener#update). Additionally, the
value from delay parameter is assigned the field which in case of
servlets behaves as a global variable. It means every further request
will use this value and also will be put to sleep. Putting a thread to
sleep excludes it from a limited set of available threads, so after a
suitable number of upload requests (Tomcat by default has limit of 200
threads) the whole application will become unresponsive and will not
accept any new requests.

https://github.com/manolo/gwtupload/blob/master/core/src/main/java/gwtupload/server/UploadServlet.java

protected String parsePostRequest(HttpServletRequest request, HttpServletResponse response) {

    try {
      String delay = request.getParameter(PARAM_DELAY);
      String maxFilesize = request.getParameter(PARAM_MAX_FILE_SIZE);
      maxSize = maxFilesize != null && maxFilesize.matches("[0-9]*") ? Long.parseLong(maxFilesize) : maxSize;
      uploadDelay = Integer.parseInt(delay);
    } catch (Exception e) { }
[...]
  protected AbstractUploadListener createNewListener(HttpServletRequest request) {
    int delay = request.getParameter("nodelay") != null ? 0 : uploadDelay;
    if (isAppEngine()) {
      return new MemoryUploadListener(delay, getContentLength(request));
    } else {
      return new UploadListener(delay, getContentLength(request));
    }
  }

https://github.com/manolo/gwtupload/blob/master/core/src/main/java/gwtupload/server/AbstractUploadListener.java

    // Just a way to slow down the upload process and see the progress bar in fast networks.
    if (slowUploads > 0 && done < total) {
      try {
        Thread.sleep(slowUploads);
      } catch (Exception e) {
        exception = new RuntimeException(e);
      }

The same way the servlet accepts maxFileSize parameter, but its abuse will only prevent from uploading files; the server won't suffer.

@mmmds
Copy link
Author

mmmds commented Jun 7, 2020

jduo added a commit to jduo/gwtupload that referenced this issue Jun 19, 2020
- When a user POSTS with the delay parameter, we no longer modify
the servlet state and just use a local variable to hold this. So this
no longer affects all subsequent requests.
- Add a server setting maxDelay which defaults to 1 minute.
- The user-specified delay is capped fro [0, maxDelay] now to avoid
the DOS attack.
@huntr-helper
Copy link

‎‍🛠️ A fix has been provided for this issue. Please reference: 418sec#1

🔥 This fix has been provided through the https://huntr.dev/ bug bounty platform.

@csware
Copy link
Contributor

csware commented Feb 14, 2021

Fix merged, can be closed

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

3 participants