Full Disclosure mailing list archives
Re: [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical)
From: Rene Gielen <rgielen () apache org>
Date: Sat, 26 Apr 2014 12:59:07 +0200
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 () struts 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 () struts apache orgWell, 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/
Current thread:
- [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical) Rene Gielen (Apr 24)
- Re: [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical) Tim (Apr 25)
- Re: [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical) Rene Gielen (Apr 25)
- Re: [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical) Tim (Apr 25)
- Re: [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical) Rene Gielen (Apr 26)
- Re: [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical) Rene Gielen (Apr 28)
- Re: [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical) Alexander Georgiev (Apr 26)
- Re: [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical) Rene Gielen (Apr 27)
- Re: [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical) Rene Gielen (Apr 25)
- Re: [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical) Tim (Apr 25)
- Re: [ANN] Struts 2 up to 2.3.16.1: Zero-Day Exploit Mitigation (security | critical) Rene Gielen (Apr 25)