
Tales of SugarCRM Security Horrors - peter_tonoli
http://karmainsecurity.com/tales-of-sugarcrm-security-horrors
======
orf
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)

~~~
JumpCrisscross
I'm a numpty. Can you explain why this is bad?

~~~
lucideer
(a) It's clearly such a persistent problem that they explicitly added such a
button.

(b) If* the button really performs as advertised, why on earth should it be a
manual step? Do it automatically.

(c) Best practice here is to escape-on-output, negating the need to remove in-
DB XSS, so the fact that this button exists strongly implies they aren't doing
that

* I would guess that the button is very unlikely to perform comprehensively as advertised

~~~
wyldfire
I think we could address all of your issues if only we could design it such
that something clicked the button as often as possible.

cron, maybe?

~~~
redler
Perhaps SugarCRM should create a new "Secure Enterprise Edition" of the
product. The fees would be substantially higher, of course, with the proceeds
used to hire overseas IT shops whose 24/7 staff would remained logged in,
clicking the button repeatedly. To mitigate the privacy concerns, SugarCRM
could create a special version of that administrative page with nothing except
the "Remove XSS" button.

------
blowski
> 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
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
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.

------
kitcar
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?

~~~
gketuma
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=entr...](https://suitecrm.com/index.php?option=com_easyblog&view=entry&id=116)

~~~
kitcar
FWIW SuiteCRM has the exact same "remove XSS" button in the admin panel that
Sugar has - doesn't exactly scream "we're confident in our security"

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

------
ibotty
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
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
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
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.

------
doubleplusgood
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.

------
philsnow
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
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/ca...](http://techblog.procurios.nl/k/n618/news/view/34972/14863/cache-
a-large-array-json-serialize-or-var_export.html)

------
hdhzy
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
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
Good point.

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

------
dmilicevic
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
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-
st...](https://developer.sugarcrm.com/2017/04/17/use-of-prepared-statements-
in-sugar-7-9/)

~~~
dmilicevic
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
Thanks. Yes, I am hopeful that a response will be forthcoming.

------
chmars
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 …

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

------
dmilicevic
response to the blog: [https://blog.sugarcrm.com/2017/04/24/important-
security-upda...](https://blog.sugarcrm.com/2017/04/24/important-security-
update/)

~~~
eg1x
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!

------
tiatia
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)

------
educar
The page is unreadable on mobile :(

~~~
hdhzy
Looks very good in Reader mode on Firefox for Android but the bare layout is
indeed not mobile friendly.

~~~
deckiedan
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!

