Opened 18 months ago

Closed 18 months ago

Last modified 18 months ago

#1488782 closed Bugs (fixed)

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

Reported by: bk2204 Owned by:
Priority: 2 Milestone: 0.8.3
Component: Core functionality Version: 0.8.2
Severity: major Keywords:
Cc:

Description

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.

Change History (5)

comment:1 Changed 18 months ago by alec

  • Milestone changed from later to 0.9-beta
  • Priority changed from 5 to 2

Confirmed. This is check-recent action.

comment:2 follow-up: Changed 18 months ago by alec

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.

comment:3 in reply to: ↑ 2 Changed 18 months ago by thomasb

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.

comment:4 Changed 18 months ago by alec

  • Resolution set to fixed
  • Status changed from new to closed

comment:5 Changed 18 months ago by thomasb

  • Milestone changed from 0.9-beta to 0.8.3

Adjust milestone.

Note: See TracTickets for help on using tickets.