Main INBOX page causes DoS against httpd when JS gets 40x error #3983

Closed
rcubetrac opened this Issue Oct 30, 2012 · 8 comments

1 participant

@rcubetrac

Reported by bk2204 on 30 Oct 2012 19:22 UTC as Trac ticket #1488782

Steps to reproduce:

  1. Log into Roundcube.
  2. Click on the INBOX link.
  3. Cause the web server to return a 40x error for Roundcube requests (see below for how).
  4. Click on the INBOX link.
  5. Notice a "Server Error! Internal server error" message.
  6. Look at the logs of httpd.
  7. Notice that Roundcube tries (unsuccessfully) to fetch data every thirty seconds and that every minute it increments the number of requests (first minute, 1; second minute, 2; third minute, 3; etc.)
  8. Watch the number of HTTP requests increase to the point of a DoS.

At cPanel, we can trigger step 3 by removing the session files stored in /var/cpanel/sessions (which causes a 401). This is harder to reproduce on a stock installation, but you can simulate this by adding a "Require user nonexistent" to the Roundcube .htaccess file (which causes a 404). I can see this on the cPanel-patched Roundcube 0.8.2 using PHP 5.2.17 and on an unpatched current git using PHP 5.4.8.

There are no logs in the logs/errors file. This is using Exim 4.80 and Courier IMAP 4.10.0, although they don't seem to be related.

If you have any questions or need more information, please don't hesitate to ask.

Migrated-From: http://trac.roundcube.net/ticket/1488782

@rcubetrac

Comment by @alecpl on 31 Oct 2012 07:31 UTC

Confirmed. This is check-recent action.

@rcubetrac

Milestone changed by @alecpl on 31 Oct 2012 07:31 UTC

later => 0.9-beta

@rcubetrac

Comment by @alecpl on 31 Oct 2012 07:48 UTC

This patch fixes the issue

--- a/program/js/app.js
+++ b/program/js/app.js
@@ -6273,9 +6273,9 @@ function rcube_webmail()

     // re-send keep-alive requests after 30 seconds
     if (action == 'keep-alive')
-      setTimeout(function(){ ref.keep_alive(); }, 30000);
+      setTimeout(function(){ ref.keep_alive(); ref.start_keepalive(); }, 30000);
     else if (action == 'check-recent')
-      setTimeout(function(){ ref.check_for_recent(false); }, 30000);
+      setTimeout(function(){ ref.check_for_recent(false); ref.start_keepalive(); }, 30000);
   };

   // post the given form to a hidden iframe

BTW, when server is in "error state" keep-alive requests are sent every 30 seconds. Maybe it would be better to limit this to two first requests and then use standard interval. This will prevent from high number of http requests (from all user sessions) in case of a temporal server breakdown. This way if server is e.g. overloaded we'll not produce too much more load.

@rcubetrac

Comment by @thomascube on 31 Oct 2012 08:32 UTC

Replying to alec:

This patch fixes the issue
[...]

Looks good. Go ahead and commit this.

BTW, when server is in "error state" keep-alive requests are sent every 30 seconds.

The "error state" might not be on the server but some (temporary) connection problems. That's why we introduced these retries.

Maybe it would be better to limit this to two first requests and then use standard interval.

Another approach would be to remember the timestamp of the last successful response from the server and abort the session (e.g. by returning to the login page) after session lifetime is reached.

@rcubetrac

Comment by @alecpl on 31 Oct 2012 08:37 UTC

Fixed in 92cb7f5

@rcubetrac

Status changed by @alecpl on 31 Oct 2012 08:37 UTC

new => closed

@rcubetrac rcubetrac closed this Oct 31, 2012
@rcubetrac

Comment by @thomascube on 10 Nov 2012 14:33 UTC

Adjust milestone.

@rcubetrac

Milestone changed by @thomascube on 10 Nov 2012 14:33 UTC

0.9-beta => 0.8.3

@rcubetrac rcubetrac added this to the 0.8.3 milestone Mar 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment