Date: Sat, 26 Apr 2014 12:59:07 +0200
From: Rene Gielen <rgielen@...che.org>
To: Tim <tim-security@...tinelchicken.org>, fulldisclosure@...lists.org
Subject: Re: [FD] [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation
 (security | critical)

Hi Tim,

Am 25.04.14 22:06, schrieb Tim:
> 
> Hi Rene,
> 
> Thanks for your responses.  Keep in mind my criticisms are not
> directed soley at you.  They are directed at the entire Struts team,
> it's practices and culture.

I don't know what insights you have to our practices in general and our
culture. I invite you to name concrete points and make suggestions for
improvements on the dev@...uts.apache.org mailing list.

> 
> I've been on the front lines with applications who were pwned by
> Struts bugs and thousands of users' personal information exposed.
> Millions of $$ burnt on the cleanup efforts.  So there may be a few
> emotions attached to my emails.  Just the way it is.
> 
> 
>>> A) I questioned the last bug fix in the thread here [1], where we
>>>    were all reassured that it was just "ClassLoader manipulation", not 
>>>    RCE.  Clearly that's not true.
>>>
>>
>> At this point in time, it was true. The RCE is not exactly a Struts
>> issue alone, the Struts issue just opens the door to an unprotected
>> field in a certain servlet container environment.
> 
> You expect every servlet container environment to protect against
> Struts?  I find this response to be very pass-the-buckish.
> 

You seem to have misread my paragraph. I'm not sure how this
interpretation regarding such expectation can be derived from what I wrote.

Anyway, let's try to rephrase carefully.

The core part of each EL I'm aware of in JVM space is property based
object graph navigation and property mutation based on the Java Beans
Specification. The said specification defines what a property is and how
to grant public read and/or write access to it. Developers are
recommended to decide carefully when public read or write access should
be granted.

Here is a nice question for a Java job interview: Is it possible to
write a Java class having no properties according to the Java Beans
Specification? The answer is yes, of course. Now, drop the public
modifier requirement for a moment. How about properties now? Well, since
each class extends java.lang.Object, it exposes at least the readable
property "class", since the inherited method getClass() follows the
getter specification (minus public modifier). Here is where Struts
developers "failed" - we missed out on that fact. Lessons learned: each
framework offering EL based graph navigation should think of special
treatment for the class property, especially given the fact that most
EL's mode of work does not care too much about visibility limitations.

So here is my point: the specific container environment we have a
confirmed RCE attack for has a writable property leading to heavy
effects when being re-written. With today's knowledge, I personally
doubt that it was the best possible decision to make this an unguarded
writable property. And no, I don't think this should be revisited for
the purpose of protecting Struts users - it should be revisited in
general, for this particular product.

> My point here is that your team failed to adequately characterize the
> problem's risks.  This leads to more developers ignoring the patch and
> subsequently getting pwned.  Even if you only thought an RCE *might*
> be possible you should state that.  It is the responsible thing to do. 
> 

An SQL injection vulnerability might introduce some probability for
leading to RCE. If I understand it right, I should name it RCE rather
than SQL injection then - as RCE *might* be possible? This sounds like
publishing gut feeling based security information. Our initial impact
statement was "classloader manipulation" which precisely names the
problem. By the time we released security bulletin S2-020, we had no
knowledge about how this could lead to RCE.

Strange enough, there are some smart and creative people out there. Even
stranger, some of them find how to exploit the said vulnerability to
allow for RCE in a certain environment, a few weeks after the
announcement. Luckily, the white- to grey-hatted of these folks tend to
reach a security response team then. This is what happened last week.
>From then on we knew we had to deal with an RCE.

Unfortunately, the RCE impact coincidented with reports about an
insufficient fix. Even worse, the insuffient fix report was disclosed by
accident.

So what would have happened if we had gotten a report about an RCE
exploit which would have been covered already by a *sufficient* fix
introduced with 2.3.16.1? We would have made an announcement to increase
maximum security rating of CVE-2014-0094 due to RCE impact along with a
strong recommendation to upgrade immediately.

We name it an RCE when there *is* an RCE, and believe me, we are neither
shy nor ashamed to do so. To call something an RCE when there is just
some probability for it is IMO *not* a responsible thing to do.

> 
> 
>>> B) The fix for the last CVE was that crappy "^class\." filter, which
>>>    I pointed out was insufficient.  The Struts team quickly fixed
>>>    that, but never bothered to update the "workaround" section in the 
>>>    last advisory to the less-terrible ".*\.class\..*" regex (or whatever
>>>    it was).  So if developers just implemented the work around from
>>>    the advisory, they were obviously not protected.  (In hindsight,
>>>    they never were protected even with the better regex, but was just
>>>    irresponsible not to make the second regex more public.)
>>>
>>
>> Better suggestions are always welcome. We have a mail address to reach
>> us for any concerns regarding security: security@...uts.apache.org
> 
> Well, what I'm saying is that I did give suggestions.  I believe that
> email address was CCed.  Your team took them to heart, which is great.
> But then your team failed to publish them adequately.
> 

I understand your point better now. Indeed, it would have been advisable
to update the mitigation pattern in the security bulletin along with an
announcement. We seem to have missed that. It would have been great if
someone had nagged us to update the page once this finding was made.
Chances are we'll do better on this in future.

Anyway, I'd love to avoid mitigation advices entirely. They more than
often lead to a feeling of false security and might keep users from
upgrading. For practical reasons we have to give them sometimes, though.

> 
> 
>>> C) The Struts team is playing whack-a-mole.  Instead of fixing the
>>>    root issue, they are just adding one blacklist regex after another,
>>>    hoping no one figures out yet another way around it.
>>
>> We try to protect users who are not using a properly configured security
>> manager, as it is always recommended when working with application
>> servers. 
> 
> Wow.  Now you sound like an Oracle representative.  I'm not
> exaggerating either, because they've used that line a few times with
> me.  We all know how well Oracle handles security.
> 
> Using a security manager for web applications is certainly a good
> idea.  No arguing against that.  Do developers do it?  NO.  It simply
> isn't common and usually when I suggest it to a client, they give me a
> blank stare, clueless as to what I'm talking about.
> 

Please re-read my paragraph. We are trying to protect our users where we
can! Even if they don't use a security manager, since we *know* that
most users aren't aware of it.

This leads to a funny situation: since we *do* provide protection code
not relying on JVM security manager, we get slapped in our face once
this mechanisms fail. If we'd throw out these mechanisms for the sake of
telling our users "use security manager, stupid!", we would be in the
comfortable situation of never getting slapped again, nor investing
unpaid time and even own money to fix other folks' security problems any
more.

I still believe that we are doing it right when investing time and code
in protecting our users even without using JVM security. Nevertheless, I
do think we do have a craftsmanship problem both in dev and ops by
ignoring JVM security. "Senior" Java devs, ITIL certified ops and
Web(Sphere|Logic)-buying clients giving me a blank stare? Yes, this is
part of the problem.

If you like, take some time to investigate possible impacts of creating
UEL 3.0 (JSR-341) incorporating web applications (using JSF, or simply
JSP). You might want to re-think running them without JVM security
manager enabled...

> 
> 
>> Sometimes we seem to fail, indeed. As others, we don't seem to
>> be perfect. BTW, we are not only blacklisting - the blacklist is applied
>> for special cases that make it through the whitelist.
> 
> Sure, and if this were only one iteration of failing on the same
> problem, I wouldn't have said a word.  It's just that the failure is
> happening over and over again.  I felt public criticism, if not
> outright shaming, is warranted.
> 

Do you have better technical solutions and project governance patterns
on the open source projects you are contributing to that you can share
with us? Can you concretely contribute better technical solutions to
Struts related issues you have identified - I mean other than just
"throw out OGNL"? Are you or your clients willing to sponsor some full
time devs or dev days for the product they are using for free? If the
answer is yes to one of these questions, we are happily accepting your
help! If the answer is no, I suggest being more careful with terms like
"shame" and such

> 
>>> I urge you to take OGNL and *throw it out*.  Replace it with something
>>> that allows only a white list of properties to be set, based on what
>>> the application defines as relevant.  Until then, I'm recommending to
>>> my clients that they avoid Struts like the plague.
>>>
>>
>> To what alternative? UEL? The attack vector is just using a simple
>> getter semantic which basically works with any EL. Throwing out EL
>> capabilities is no option for most users. Anyway, if somebody likes to
>> help with more than just fingerpointing, he/she is heartly welcome!
> 
> Something that doesn't allow you to directly call methods, and only
> allows you to access properties on objects explicitly defined by the
> app developer.  Keep the syntax similar if you like, but chuck the
> reflection.  Data is data.  Code is code.  Keep them separate.
> 

This is a nice wish list. While we at Struts are in the process of
constantly limiting what OGNL and our interface to it allows (static
method access and what not), other EL providers and consumers seem to go
the opposite way. Users nowadays do like EL expressiveness as much as
they are hating security managers, it seems. However, I do have some
more takeaways on how to improve that I'd rather not like share here.
The Struts development list would be a proper place to discuss this.

But again, keep in mind that the discussed attack vector *solely* relies
on property access. Not much more, not much less. No weird OGNL features
involved, no static method access, no reflection tricks, no double
evaluation. Just getters and setters.

Regards,
René

> tim
> 

-- 
René Gielen
http://twitter.com/rgielen

_______________________________________________
Sent through the Full Disclosure mailing list
http://nmap.org/mailman/listinfo/fulldisclosure
Web Archives & RSS: http://seclists.org/fulldisclosure/

Powered by blists - more mailing lists