STRUTS2 - S2-020 VULNERABILITY IMPACT
That, so called vulnerability, which by the way made Struts team develop 3 upgrades in less than week, is a result of the whole framework work-flow concept (dunno how to call it)… and also the lack of proper configuration in web-app. I, for example, knew that in Struts I can set object properties with request parameters but dunno why I’ve never thought how big impact that have on the security. If you think about that… user can change EVERY property action has access to. Of course there are some limitations, action must have setter for that property. But what with properties that are objects, then action just need getter for it and everyone can access them and so on. Now this looks serious because user can make a little mess with our app. Here’s example:
I’ve created new Struts2 app with 2.3.15 version and added to it new class of user. User class is simple, it consists of two properties: (String) username and (boolean) admin. There are getters and setters and constructor with name argument. I’ve added user object to HelloWorld class and getter to get access to it. In page I check if user is admin or not and welcomes him. Here how this looks:
Now…I know that user object is named “userTest”… let’s try adding a parameter: userTest.admin=true.
Voila Bob’s your admin:
User object got also property “username”. Let’s play with it:
Where is Bob… who knows. XSS will go with it also. Now imagine you give user possibility to change username with some form and input validation inside action and no validation inside object because you think that only validated input will come there. But user changes it with request param and you got a problem. But that problem is programmer’s fault because his/her app is not properly secured. It is hard to secure every class in every possible way. Just look on my post about attacking Tomcat 8 using that vulnerability. That exploit just goes through objects tree and changes properties responsible for logging. Who would think that everyone will be able to access these objects? Ok… enough rhetorical questions. We need FIX, we need it NOW! I’ve already provided one solution. Secure everything as damn good as you can… and I wish you luck :)
I think it’s impossible to secure everything so let’s look for another solution. Struts team provided one partially, and here how it looks:
<interceptor-ref name="params">
<param name="excludeParams">^class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param>
</interceptor-ref>
It’s regex of excluded parameters names. They say it works… it almost worked. You could bypass that in several ways. So there was another workaround (S2-021):
<interceptor-ref name="params">
<param name="excludeParams">(.*\.|^|.*|\[('|"))(c|C)lass(\.|('|")]|\[).*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param>
</interceptor-ref>
This one solves ClassLoader problem. I don’t know if this is what is inside patch/new version. But that still leaves us with access to other objects. I looked inside that topic and found that one can implement interface called ParameterNameAware inside class. That will add “acceptableParameterName” method which will be called for every parameter and return true if it’s acceptable or false if it’s not. I set it to always return false and made test. Nothing changed… user properties have been changed. What the… ? Few googling later… looks like that method is only called for excluded parameters (excluded by interceptor). So let’s exclude all of them.
<interceptor-ref name="params">
<param name="excludeParams">.*</param>
</interceptor-ref>
Now it looks better, user can’t change anything.
Now in every action, I have list of accepted parameter names and inside “acceptableParameterName” method I check if my list contain provided parameter. Users can change only things I let them change and action can have full access to objects.
As you can see that vulnerability in Struts2 was quite complex. There are other
ways of securing access to objects and their properties and maybe they are commonly
used but for me solution provided here looks good. Ahhh, one more thing, implementing
this solution will give you one more benefit (or not). You can’t access object
setters directly, so accepting parameter “userTest.username” won’t let user change
“username” field inside object “userTest”. You must write handlers inside actions.
Normally that ParameterInterceptor would find proper class and set property and n
ow it just excludes every param, maybe that’s why parameters weren’t passed to
checking method inside action (they were already set by interceptor).
That’s all!