#1484254 closed Bugs (fixed)
XSS (cross-site scripting) in _mbox value
| Reported by: | cweb | Owned by: | |
|---|---|---|---|
| Priority: | 1 - Highest | Milestone: | |
| Component: | Client Scripts | Version: | 0.1-beta2 |
| Severity: | critical | Keywords: | security xss |
| Cc: |
Description
Query string value '_mbox' is consumed at line 32 of program/steps/mail/func.inc. The actual code that isn't properly sanitizing the user-input/output looks like it's at line 21 of program/include/main.inc.
if (!empty($_SESSION['mbox']))
$IMAP->set_mailbox($_SESSION['mbox']);
The result is that scriptable code can be injected into the rendered page, where the rcmail object is being built. So for example, a URL constructed as such will inject attacker-supplied code into the page:
https://site/mail/?_task=mail&_mbox=XSSbug')//--></script><body><script>alert(document.cookie)</script>
The rendered page looks like:
var rcmail = new rcube_webmail();
rcmail.set_env('comm_path', './?_task=mail');
rcmail.set_env('read_when_deleted', '1');
rcmail.set_env('flag_for_deletion', '1');
rcmail.add_label('loading', 'Loading...');
rcmail.add_label('checkingmail', 'Checking for new messages...');
rcmail.set_env('task', 'mail');
rcmail.set_env('mailbox', 'XSSbug')//--></script><body><script>alert(document.cookie)</script>');
rcmail.set_env('trash_mailbox', 'Trash');
The code is persistent for the session, as it sets a value which is reused again and again.
Change History (4)
comment:1 Changed 6 years ago by robin
comment:2 Changed 6 years ago by till
Just a suggestion, but instead of
if (strlen($_GET['_mbox']))
please do:
if (isset($_GET['_mbox']) && !empty($_GET['_mbox']))
comment:3 Changed 6 years ago by robin
The function get_input_value in program/include/main.inc already does isset(). An empty value of $mbox only results in error.
This patch breaks the vulnerability, but I want to be sure it doens't break anything else.
comment:4 Changed 6 years ago by robin
- Resolution set to fixed
- Status changed from new to closed
Fixed in SVN revision 482.
How's this for a fix:
Index: program/steps/mail/func.inc =================================================================== --- program/steps/mail/func.inc (revision 479) +++ program/steps/mail/func.inc (working copy) @@ -30,10 +30,10 @@ } // set imap properties and session vars -if (strlen($_GET['_mbox'])) +if (strlen($mbox = get_input_value('_mbox', RCUBE_INPUT_GET))) { - $IMAP->set_mailbox($_GET['_mbox']); - $_SESSION['mbox'] = $_GET['_mbox']; + $IMAP->set_mailbox($mbox); + $_SESSION['mbox'] = $mbox; } if (strlen($_GET['_page']))