Home page logo

fulldisclosure logo Full Disclosure mailing list archives

Re: [ANN] Struts 2 up to Zero-Day Exploit Mitigation (security | critical)
From: Tim <tim-security () sentinelchicken org>
Date: Fri, 25 Apr 2014 13:06:09 -0700

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

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. 

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 () struts 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.

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

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.

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.

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.


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

  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]