Re: [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical)

[Tim <tim-security () sentinelchicken org>]

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



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

tim

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