8 ms·

Tales of SugarCRM Security Horrors

235 points by peter_tonoli 9y ago

educar 9y ago
The page is unreadable on mobile :(
- CaliforniaKarl 9y ago
  It was fine for me on an iPhone SE, after switching to landscape orientation.
  - throwanem 9y ago
    It's fine in Safari reader mode.
- hdhzy 9y ago
  Looks very good in Reader mode on Firefox for Android but the bare layout is indeed not mobile friendly.
  - deckiedan 9y ago
    Android Firefox readermode is fantastic. I absolutely love it. I wish it was the default view for most pages with a toggle to see the original!
  - llekn 9y ago
    Reader mode is by far my favourite Firefox feature! It is overwhelming how sweet is to read focused on the content instead on closing the next-to-scam popups that decides to open up on a random part of the article. One interesting side effect of reader mode is that as all content has the same format, you get used very fast to compare content not based on external artifacts (font, colors, page layout), but in the actual message.
  - jniles 9y ago
    I had never used reader mode on FF, but just gave it a whirl for this atricle and really like it. Thanks for the tip!
- kalmari 9y ago
  you'll hit hi res native and no mobile version sites from time to time as others have said go landscape and pinch out to zoom to text block.
ibotty 9y ago
Whoa! That's horrifying. Not only don't they update their open source version when fixing security bugs (Great argument against choosing open core solutions btw), they don't even fix most bugs!
- roblabla 9y ago
  This isn't a great argument against chosing open core solutions, it's a great argument against chosing SugarCRM. Many open core solutions (I'm thinking of gitlab, but there are probably many other) fixes bugs in both versions.
  - ibotty 9y ago
    It is. If there are two versions, it's possible (it even "adds value") to fix bugs in one version and not the other. If there is only one, you can't do that. Simple as that.
    - roblabla 9y ago
      It makes the company (look at sugarcrm) look bad when they do that. Open core can be done well, just like proprietary software can be done wrong.
deleted 9y ago
[deleted]
nkuttler 9y ago
Wow, this kept getting better and better, I didn't expect to make it through the entire text. Some parts are shocking.
orf 9y ago
The SugarCRM administration panel has a button labeled "remove XSS". We have a picture of it up in our office. Yes. A button that attempts to remove XSS payloads from the database that admins can click. That's the level of security competence we are talking about here. Edit: Here is the button: http://i.imgur.com/hC9KmWh.png http://i.imgur.com/hC9KmWh.png
- blowski 9y ago
  This one: https://support.sugarcrm.com/Documentation/Sugar_Versions/7.7/Ent/Administration_Guide/System/Repair/index.html#Remove_XSS https://support.sugarcrm.com/Documentation/Sugar_Versions/7....
- JumpCrisscross 9y ago
  I'm a numpty. Can you explain why this is bad?
  - code_duck 9y ago
    Can you think of a case where it would be helpful to store XSS in the database until removed by clicking this button? What happens if we view the admin panel and trigger one of these malicious scripts, because I haven't clicked this button recently enough? Oh, maybe someone should click it very frequently! Or... Hmm, maybe rather than storing data that may be mass deleted by a script, with no review, after possibly compromising our security, we should just reject it in the first place and log a security incident for that user.
    - mgkimsal 9y ago
      my view would be 'escape on output' is always better, regardless. there may be reasons I'd want to keep it in there - for analysis, to prove someone was trying to be malicious, or perhaps to demonstrate there's actually a real problem getting past some input mechanisms. However, the 'defender' in me can think of a situation where this would be good: Input filtering mechanisms have improved to deal with new attacks/problems, but old data is not 'new input', so you'd apply the updated algorithms to existing data.
hdhzy 9y ago
I wonder what's the use case for serializing and unserializing objects using php built-in functions. Is this some kind of "I'm too lazy to json encode a subset of properties" or are there some edge cases where one would use this extremely sharp knife?
- stephenr 9y ago
  Sharp knives are the least dangerous if you're cutting something. A sharp knife will just cut the thing. A dull/less sharp knife will not cut, causing you to add more pressure, causing the knife to slip, which is when you're more likely to cause yourself harm.
  - hdhzy 9y ago
    Good point.
- throwanem 9y ago
  There are cases in which one might use it, but no cases in which one should.
  - somebehemoth 9y ago
    If you have completely sanitized data what is the problem with object serialization? Thank you.
    - hdhzy 9y ago
      I think the problem lies somewhere inside "completely sanitized data"."If you have them completely sanitized it usually is not what many would call object serialization (php unserialize) but rather a data format (JSON).
    - throwanem 9y ago
      The only reliable way to sanitize PHP-serialized data is to unserialize it, scrub it, and reserialize it. This poses a nigh intractable chicken-egg problem, and makes a switch to JSON by far a more economical option.
blowski 9y ago
> there are still chances for both authenticated (CVE-2012-0694) and unauthenticated (KIS-2016-07) attackers to exploit PHP internal memory corruption vulnerabilities which do not require objects declarations, like this ten years old vulnerability which requires just an array definition or this one which relies on references and arrays declarations The two bugs linked were both fixed around 10 years ago. If you're running PHP v4.4, your problems are basically infinite. It would be nice to make a clearer distinction between PHP problems and SugarCRM problems.
- egix 9y ago
  I said "like this 10 years old vulnerability", which means there could be other 0-day vulnerabilities affecting even the latest PHP versions and that could be exploited without relying on objects declarations within the serialized string. Using the unserialize() PHP function itself is not security issue, unless you use it with user-supplied input, and that's the reason why this is a SugarCRM problem and not a PHP problem.
  - blowski 9y ago
    It's a great post, by the way, no criticisms of that. But I interpreted that specific bit as "this core PHP bug was reported 10 years ago and still hasn't been fixed", so makes it sound like a huge and ongoing PHP vulnerability.
doubleplusgood 9y ago
A few years ago, my team and I tried building a small CRM solution based on SugarCRM; we figured, "hey, it's basically a simple CRUD app with some reports and somewhat-dynamic objects, right"? We gave up after a week (ended up building the thing in Django). vTiger/SugarCRM is most likely the worst PHP codebase still in active development/production.
kitcar 9y ago
How timely; I recently evaluated sugarcrm/suitecrm but similar to author was dismayed by their code quality. Does anyone have any recommendations of other open source CRMs?
- thr0w4way 9y ago
  Maybe you should have a look at OroCRM and OroPlatform (https://www.orocrm.com https://www.orocrm.com, https://www.orocrm.com/oro-platform https://www.orocrm.com/oro-platform). It is built on Symfony and seems rather new, maybe not feature complete.
  - kitcar 9y ago
    Based on OroCRM's comparison chart their community edition is effectively a non-feature complete version of their commercial product, i.e. same scenario that happened to Sugar? https://www.orocrm.com/orocrm-enterprise-and-community https://www.orocrm.com/orocrm-enterprise-and-community
- gketuma 9y ago
  I won't consider SuiteCRM to be part of this. Understand that even though SuiteCRM was a fork of SugarCRM CE, the community has made so many changes that I will consider it at this point a completely different product. All the vulnerabilities listed above do not apply to SuiteCRM in its current release and if you look at the blog of SuiteCRM, it is heavily advertised that SugarCRM CE is full of security issues and everyone should migrate out of it[1]. Here is the quote: “SugarCRM Community Edition users need to migrate to an alternative platform as soon as possible. The number of current vulnerabilities in Community Edition is worrying. There will be no more support for Community Edition after April 2017 and the vulnerabilities will increase as the software ages. Simply put, if you're running SugarCRM Community Edition, you're becoming a soft target.” https://suitecrm.com/index.php?option=com_easyblog&view=entry&id=116 https://suitecrm.com/index.php?option=com_easyblog&view=entr...
  - verbify 9y ago
    So suitecrm want people to migrate to their product and claim to have superior security? That's hardly surprising.
tiatia 9y ago
Can someone recommend a CRM? Preferably open source and free? Currently we are considering odoo but any advice appreciated. https://comparisons.financesonline.com/sugar-crm-vs-odoo https://comparisons.financesonline.com/sugar-crm-vs-odoo
ReligiousFlames 9y ago
SugarCRM stopped public development long ago. Most people use Sugar non-CE or SuiteCRM (a maintained fork) which probably has similar/same vulns.
dmilicevic 9y ago
The good thing is that Sugar is slowly but steadily replacing the old codebase but they should be more transparent on addressing these serious issues.
- mgmarum 9y ago
  Yes. For example, SugarCRM is adding prepared statement support which mitigates any potential SQL injection problems. I mention this since it was posted just this last week. These changes are very large and far reaching so it does take time. https://developer.sugarcrm.com/2017/04/17/use-of-prepared-statements-in-sugar-7-9/ https://developer.sugarcrm.com/2017/04/17/use-of-prepared-st...
  - dmilicevic 9y ago
    Great stuff again Matthew :), your blogs are always easy to read and helpful! Personally I think that string concatenation in query building throughout the Sugar code base (campaigns, workflows) is very problematic and could also be exposed in a couple of scenarios. But like I said, this seems to be the work in progress currently at Sugar. Hopefully Sugar will come forward with a response to these allegations because these are serious security risks.
    - mgmarum 9y ago
      Thanks. Yes, I am hopeful that a response will be forthcoming.
  - bpicolo 9y ago
    > mitigates any potential SQL injection problems Not quite, string concat for defining queries is still plenty vulnerable regardless of PDO.
    - dmilicevic 9y ago
      it mitigates quite some SQL injection possibilities, but yes, string concat while building queries still remains an issue.
philsnow 9y ago
The core issue is that sugar crm uses PHP built in `unserialize` on user controlled input, and they don't want to switch to json ostensibly because of performance issues. Why don't they hmac the payloads (with a timestamp and something tied to the user (an ID, the username, whatever)) and verify the hmac before deserializing?Verifying an hmac prevents undetected tampering, is fast, and there are libraries for it in ~every language.
- iancarroll 9y ago
  The performance part doesn't make sense to me either. What are they serializing in a CRM that is so time-critical? I found a benchmark [0] that shows differences of like .2 milliseconds for a 904 byte array. [0] http://techblog.procurios.nl/k/n618/news/view/34972/14863/cache-a-large-array-json-serialize-or-var_export.html http://techblog.procurios.nl/k/n618/news/view/34972/14863/ca...
elchief 9y ago
You need to run a WAF like modsecurity in front of any PHP application these days.
chmars 9y ago
Other CRM providers would probably deserve a closer look too. Marketcircle for example has never been able to offer reliable SSL support for its CalDAV / CardDAV server. And they are switching to a cloud solution too – closed source and proprietary …
dmilicevic 9y ago
response to the blog: https://blog.sugarcrm.com/2017/04/24/important-security-update/ https://blog.sugarcrm.com/2017/04/24/important-security-upda...
- eg1x 9y ago
  Their latest update (4/25/17): Based on impact and reach, none of the vulnerabilities in in the second section scored higher than 'medium'... It’s important to note that updates based on issues scored as ‘medium’ are no longer provided to our last-generation open source Community Edition (CE), so the bloggers post no longer aligns with our current commercial products and solutions. So, I just replied to their blog post with this comment (awaiting moderation, so will probably never be published): Hi Rich, I'd like to know more about your latest update, the one with regards to the second section of my post: you're saying you rated all of the vulnerabilities I reported with a "medium" CVSS score (which version btw? v3.0?). However, I reported two SQL injection vulnerabilities and according to your security advisories (https://www.sugarcrm.com/security/sugarcrm-sa-2016-003 https://www.sugarcrm.com/security/sugarcrm-sa-2016-003 and https://www.sugarcrm.com/security/sugarcrm-sa-2017-001 https://www.sugarcrm.com/security/sugarcrm-sa-2017-001), in the past you rated SQL injection vulnerabilities in SugarCRM with 'High' or 'Important' risk level... May I know why now they're considered of 'Medium' risk level? They same applies to the remaining vulnerabilities, which might allow a malicious user to execute arbitrary PHP code, and so far in your security advisories this kind of issues has been rated with 'Critical' risk level (like this: https://www.sugarcrm.com/security/sugarcrm-sa-2016-001 https://www.sugarcrm.com/security/sugarcrm-sa-2016-001)... The numbers don't add up!

235 points by peter_tonoli 9y ago

educar 9y ago

The page is unreadable on mobile :(

CaliforniaKarl 9y ago
It was fine for me on an iPhone SE, after switching to landscape orientation.
- throwanem 9y ago
  It's fine in Safari reader mode.
hdhzy 9y ago
Looks very good in Reader mode on Firefox for Android but the bare layout is indeed not mobile friendly.
- deckiedan 9y ago
  Android Firefox readermode is fantastic. I absolutely love it. I wish it was the default view for most pages with a toggle to see the original!
- llekn 9y ago
  Reader mode is by far my favourite Firefox feature! It is overwhelming how sweet is to read focused on the content instead on closing the next-to-scam popups that decides to open up on a random part of the article. One interesting side effect of reader mode is that as all content has the same format, you get used very fast to compare content not based on external artifacts (font, colors, page layout), but in the actual message.
- jniles 9y ago
  I had never used reader mode on FF, but just gave it a whirl for this atricle and really like it. Thanks for the tip!
kalmari 9y ago
you'll hit hi res native and no mobile version sites from time to time as others have said go landscape and pinch out to zoom to text block.

ibotty 9y ago

Whoa! That's horrifying. Not only don't they update their open source version when fixing security bugs (Great argument against choosing open core solutions btw), they don't even fix most bugs!

roblabla 9y ago
This isn't a great argument against chosing open core solutions, it's a great argument against chosing SugarCRM. Many open core solutions (I'm thinking of gitlab, but there are probably many other) fixes bugs in both versions.
- ibotty 9y ago
  It is. If there are two versions, it's possible (it even "adds value") to fix bugs in one version and not the other. If there is only one, you can't do that. Simple as that.
  - roblabla 9y ago
    It makes the company (look at sugarcrm) look bad when they do that. Open core can be done well, just like proprietary software can be done wrong.

deleted 9y ago

[deleted]

nkuttler 9y ago

Wow, this kept getting better and better, I didn't expect to make it through the entire text. Some parts are shocking.

orf 9y ago

The SugarCRM administration panel has a button labeled "remove XSS". We have a picture of it up in our office. Yes. A button that attempts to remove XSS payloads from the database that admins can click. That's the level of security competence we are talking about here. Edit: Here is the button: http://i.imgur.com/hC9KmWh.png http://i.imgur.com/hC9KmWh.png

blowski 9y ago
This one: https://support.sugarcrm.com/Documentation/Sugar_Versions/7.7/Ent/Administration_Guide/System/Repair/index.html#Remove_XSS https://support.sugarcrm.com/Documentation/Sugar_Versions/7....
JumpCrisscross 9y ago
I'm a numpty. Can you explain why this is bad?
- code_duck 9y ago
  Can you think of a case where it would be helpful to store XSS in the database until removed by clicking this button? What happens if we view the admin panel and trigger one of these malicious scripts, because I haven't clicked this button recently enough? Oh, maybe someone should click it very frequently! Or... Hmm, maybe rather than storing data that may be mass deleted by a script, with no review, after possibly compromising our security, we should just reject it in the first place and log a security incident for that user.
  - mgkimsal 9y ago
    my view would be 'escape on output' is always better, regardless. there may be reasons I'd want to keep it in there - for analysis, to prove someone was trying to be malicious, or perhaps to demonstrate there's actually a real problem getting past some input mechanisms. However, the 'defender' in me can think of a situation where this would be good: Input filtering mechanisms have improved to deal with new attacks/problems, but old data is not 'new input', so you'd apply the updated algorithms to existing data.

hdhzy 9y ago

I wonder what's the use case for serializing and unserializing objects using php built-in functions. Is this some kind of "I'm too lazy to json encode a subset of properties" or are there some edge cases where one would use this extremely sharp knife?

stephenr 9y ago
Sharp knives are the least dangerous if you're cutting something. A sharp knife will just cut the thing. A dull/less sharp knife will not cut, causing you to add more pressure, causing the knife to slip, which is when you're more likely to cause yourself harm.
- hdhzy 9y ago
  Good point.
throwanem 9y ago
There are cases in which one might use it, but no cases in which one should.
- somebehemoth 9y ago
  If you have completely sanitized data what is the problem with object serialization? Thank you.
  - hdhzy 9y ago
    I think the problem lies somewhere inside "completely sanitized data"."If you have them completely sanitized it usually is not what many would call object serialization (php unserialize) but rather a data format (JSON).
  - throwanem 9y ago
    The only reliable way to sanitize PHP-serialized data is to unserialize it, scrub it, and reserialize it. This poses a nigh intractable chicken-egg problem, and makes a switch to JSON by far a more economical option.

blowski 9y ago

> there are still chances for both authenticated (CVE-2012-0694) and unauthenticated (KIS-2016-07) attackers to exploit PHP internal memory corruption vulnerabilities which do not require objects declarations, like this ten years old vulnerability which requires just an array definition or this one which relies on references and arrays declarations The two bugs linked were both fixed around 10 years ago. If you're running PHP v4.4, your problems are basically infinite. It would be nice to make a clearer distinction between PHP problems and SugarCRM problems.

egix 9y ago
I said "like this 10 years old vulnerability", which means there could be other 0-day vulnerabilities affecting even the latest PHP versions and that could be exploited without relying on objects declarations within the serialized string. Using the unserialize() PHP function itself is not security issue, unless you use it with user-supplied input, and that's the reason why this is a SugarCRM problem and not a PHP problem.
- blowski 9y ago
  It's a great post, by the way, no criticisms of that. But I interpreted that specific bit as "this core PHP bug was reported 10 years ago and still hasn't been fixed", so makes it sound like a huge and ongoing PHP vulnerability.

doubleplusgood 9y ago

A few years ago, my team and I tried building a small CRM solution based on SugarCRM; we figured, "hey, it's basically a simple CRUD app with some reports and somewhat-dynamic objects, right"? We gave up after a week (ended up building the thing in Django). vTiger/SugarCRM is most likely the worst PHP codebase still in active development/production.

kitcar 9y ago

How timely; I recently evaluated sugarcrm/suitecrm but similar to author was dismayed by their code quality. Does anyone have any recommendations of other open source CRMs?

thr0w4way 9y ago
Maybe you should have a look at OroCRM and OroPlatform (https://www.orocrm.com https://www.orocrm.com, https://www.orocrm.com/oro-platform https://www.orocrm.com/oro-platform). It is built on Symfony and seems rather new, maybe not feature complete.
- kitcar 9y ago
  Based on OroCRM's comparison chart their community edition is effectively a non-feature complete version of their commercial product, i.e. same scenario that happened to Sugar? https://www.orocrm.com/orocrm-enterprise-and-community https://www.orocrm.com/orocrm-enterprise-and-community
gketuma 9y ago
I won't consider SuiteCRM to be part of this. Understand that even though SuiteCRM was a fork of SugarCRM CE, the community has made so many changes that I will consider it at this point a completely different product. All the vulnerabilities listed above do not apply to SuiteCRM in its current release and if you look at the blog of SuiteCRM, it is heavily advertised that SugarCRM CE is full of security issues and everyone should migrate out of it[1]. Here is the quote: “SugarCRM Community Edition users need to migrate to an alternative platform as soon as possible. The number of current vulnerabilities in Community Edition is worrying. There will be no more support for Community Edition after April 2017 and the vulnerabilities will increase as the software ages. Simply put, if you're running SugarCRM Community Edition, you're becoming a soft target.” https://suitecrm.com/index.php?option=com_easyblog&view=entry&id=116 https://suitecrm.com/index.php?option=com_easyblog&view=entr...
- verbify 9y ago
  So suitecrm want people to migrate to their product and claim to have superior security? That's hardly surprising.

tiatia 9y ago

Can someone recommend a CRM? Preferably open source and free? Currently we are considering odoo but any advice appreciated. https://comparisons.financesonline.com/sugar-crm-vs-odoo https://comparisons.financesonline.com/sugar-crm-vs-odoo

ReligiousFlames 9y ago

SugarCRM stopped public development long ago. Most people use Sugar non-CE or SuiteCRM (a maintained fork) which probably has similar/same vulns.

dmilicevic 9y ago

The good thing is that Sugar is slowly but steadily replacing the old codebase but they should be more transparent on addressing these serious issues.

mgmarum 9y ago
Yes. For example, SugarCRM is adding prepared statement support which mitigates any potential SQL injection problems. I mention this since it was posted just this last week. These changes are very large and far reaching so it does take time. https://developer.sugarcrm.com/2017/04/17/use-of-prepared-statements-in-sugar-7-9/ https://developer.sugarcrm.com/2017/04/17/use-of-prepared-st...
- dmilicevic 9y ago
  Great stuff again Matthew :), your blogs are always easy to read and helpful! Personally I think that string concatenation in query building throughout the Sugar code base (campaigns, workflows) is very problematic and could also be exposed in a couple of scenarios. But like I said, this seems to be the work in progress currently at Sugar. Hopefully Sugar will come forward with a response to these allegations because these are serious security risks.
  - mgmarum 9y ago
    Thanks. Yes, I am hopeful that a response will be forthcoming.
- bpicolo 9y ago
  > mitigates any potential SQL injection problems Not quite, string concat for defining queries is still plenty vulnerable regardless of PDO.
  - dmilicevic 9y ago
    it mitigates quite some SQL injection possibilities, but yes, string concat while building queries still remains an issue.

philsnow 9y ago

The core issue is that sugar crm uses PHP built in `unserialize` on user controlled input, and they don't want to switch to json ostensibly because of performance issues. Why don't they hmac the payloads (with a timestamp and something tied to the user (an ID, the username, whatever)) and verify the hmac before deserializing?Verifying an hmac prevents undetected tampering, is fast, and there are libraries for it in ~every language.

iancarroll 9y ago
The performance part doesn't make sense to me either. What are they serializing in a CRM that is so time-critical? I found a benchmark [0] that shows differences of like .2 milliseconds for a 904 byte array. [0] http://techblog.procurios.nl/k/n618/news/view/34972/14863/cache-a-large-array-json-serialize-or-var_export.html http://techblog.procurios.nl/k/n618/news/view/34972/14863/ca...

elchief 9y ago

You need to run a WAF like modsecurity in front of any PHP application these days.

chmars 9y ago

Other CRM providers would probably deserve a closer look too. Marketcircle for example has never been able to offer reliable SSL support for its CalDAV / CardDAV server. And they are switching to a cloud solution too – closed source and proprietary …

dmilicevic 9y ago

response to the blog: https://blog.sugarcrm.com/2017/04/24/important-security-update/ https://blog.sugarcrm.com/2017/04/24/important-security-upda...

eg1x 9y ago
Their latest update (4/25/17): Based on impact and reach, none of the vulnerabilities in in the second section scored higher than 'medium'... It’s important to note that updates based on issues scored as ‘medium’ are no longer provided to our last-generation open source Community Edition (CE), so the bloggers post no longer aligns with our current commercial products and solutions. So, I just replied to their blog post with this comment (awaiting moderation, so will probably never be published): Hi Rich, I'd like to know more about your latest update, the one with regards to the second section of my post: you're saying you rated all of the vulnerabilities I reported with a "medium" CVSS score (which version btw? v3.0?). However, I reported two SQL injection vulnerabilities and according to your security advisories (https://www.sugarcrm.com/security/sugarcrm-sa-2016-003 https://www.sugarcrm.com/security/sugarcrm-sa-2016-003 and https://www.sugarcrm.com/security/sugarcrm-sa-2017-001 https://www.sugarcrm.com/security/sugarcrm-sa-2017-001), in the past you rated SQL injection vulnerabilities in SugarCRM with 'High' or 'Important' risk level... May I know why now they're considered of 'Medium' risk level? They same applies to the remaining vulnerabilities, which might allow a malicious user to execute arbitrary PHP code, and so far in your security advisories this kind of issues has been rated with 'Critical' risk level (like this: https://www.sugarcrm.com/security/sugarcrm-sa-2016-001 https://www.sugarcrm.com/security/sugarcrm-sa-2016-001)... The numbers don't add up!