You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@struts.apache.org by Dale Newfield <Da...@Newfield.org> on 2007/11/12 18:33:44 UTC
Re: [struts] s2 and DispatchAction
Don Brown wrote:
> Little known fact, but you can specify the method via:
>
> "?method:MY_METHOD_NAME"
>
> This code exists to support the method attribute on the submit tag,
> allowing you to submit the form to different methods based on what
> button is clicked.
I wondered how the submit tag argument worked.
I would argue this is as big a security vulnerability as the
"action!method" capability. (As, for example, it can allow less
privileged users to access more privileged methods that the author
thought were protected via the url pattern by something like
org.acegisecurity.intercept.web.FilterSecurityInterceptor.) Is there
any way to restrict which methods are valid there, or to turn this
capability off?
-Dale
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: [struts] s2 and DispatchAction
Posted by Jeromy Evans <je...@blueskyminds.com.au>.
Dale Newfield wrote:
> Jeromy Evans wrote:
>> I always use the following configuration to minimise the vulnerability::
>>
>> <action name="/home_*" method="do{1}">
>> </action>
>>
>> With that setting, only methods with the prefix "do" in their name
>> can be executed.
>> ie. ?method:update calls doUpdate()
>
> Even if that does exactly what you expect it does (which I'm not
> convinced about), all it does is change the names of the methods. It
> does nothing to mitigate the vulnerability of allowing any user with
> sufficient credentials to call a single method in your action
> ("doView", for example) the ability to call any method in that action.
>
> If you're set up to allow anybody to execute "doView" but only admins
> to execute "doUpdate", then "/viewFoo.do?method:update" could be a
> security hole whether the actual method being called is "update()" or
> "doUpdate()".
>
> It's precisely analogous to the "/viewFoo!update.do" vulnerability,
> which is closable by disabling "allowDynamicMethodCalls". I'm simply
> proposing that the same flag be used to address both vulnerabilities.
> Another solution would be to add an explicit allow list (per action
> class) for what methodNames would be allowed in "?method:methodName",
> but I don't see a good place to put that whitelist, and recognize it's
> just more configuration at a time when the struts developers are
> trying to replace configuration with convention.
>
> -Dale
Right you are. I just built a test app and the "?method:MY_METHOD_NAME"
notation will execute any public method irrespective of the
wildcard/dynamic method call settings.
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: [struts] s2 and DispatchAction
Posted by Dale Newfield <Da...@Newfield.org>.
Jeromy Evans wrote:
> I always use the following configuration to minimise the vulnerability::
>
> <action name="/home_*" method="do{1}">
> </action>
>
> With that setting, only methods with the prefix "do" in their name can
> be executed.
> ie. ?method:update calls doUpdate()
Even if that does exactly what you expect it does (which I'm not
convinced about), all it does is change the names of the methods. It
does nothing to mitigate the vulnerability of allowing any user with
sufficient credentials to call a single method in your action ("doView",
for example) the ability to call any method in that action.
If you're set up to allow anybody to execute "doView" but only admins to
execute "doUpdate", then "/viewFoo.do?method:update" could be a security
hole whether the actual method being called is "update()" or "doUpdate()".
It's precisely analogous to the "/viewFoo!update.do" vulnerability,
which is closable by disabling "allowDynamicMethodCalls". I'm simply
proposing that the same flag be used to address both vulnerabilities.
Another solution would be to add an explicit allow list (per action
class) for what methodNames would be allowed in "?method:methodName",
but I don't see a good place to put that whitelist, and recognize it's
just more configuration at a time when the struts developers are trying
to replace configuration with convention.
-Dale
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: [struts] s2 and DispatchAction
Posted by Dale Newfield <Da...@Newfield.org>.
Jeromy Evans wrote:
> It would be simple enough for the DefaultActionMapper to check a flag as
> well except I think this would also prevent the method="METHOD_NAME"
> notation from being used in struts.xml as well.
Urk! I didn't realize that. If true, that definitely means the simple
solution is not viable. Guess it's time to submit a JIRA bug and move
this conversation to the dev list.
-Dale
P.S.: https://issues.apache.org/struts/browse/WW-2316
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: [struts] s2 and DispatchAction
Posted by Jeromy Evans <je...@blueskyminds.com.au>.
Dale Newfield wrote:
> Dave Newton wrote:
>> Does that deal with the submit button name thing?
>
>> --- Jeromy Evans <je...@blueskyminds.com.au>
>> wrote:
>>> <action name="/home_*" method="do{1}">
>>> </action>
>
> I don't believe so. It just makes /home_update.do execute the
> doUpdate() method (assuming it does the camelcase stuff implied). It
> doesn't change the naming scheme for all methods on that action class,
> just provides a shorthand mapping from urls that begin "/home_".
>
Right. The only benefit is that it prevents requests using the wildcard
notation from calling methods without the do prefix because doMethod is
assumed.
> I would expect "/home_view.do?method:update" to call update(), not
> doUpdate(), although "/home_view.do?method:doUpdate" should call
> doUpdate().
>
> Note: I've read code and theorized here, but not tested the hypotheses.
>
Right. The DefaultActionMapper uses the method: prefix directly
irrespective of the other settings. Only the action:prefix accounts for
dynamicMethodCalls.
It would be simple enough for the DefaultActionMapper to check a flag as
well except I think this would also prevent the method="METHOD_NAME"
notation from being used in struts.xml as well.
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: [struts] s2 and DispatchAction
Posted by Dale Newfield <Da...@Newfield.org>.
Dave Newton wrote:
> Does that deal with the submit button name thing?
> --- Jeromy Evans <je...@blueskyminds.com.au>
> wrote:
>> <action name="/home_*" method="do{1}">
>> </action>
I don't believe so. It just makes /home_update.do execute the
doUpdate() method (assuming it does the camelcase stuff implied). It
doesn't change the naming scheme for all methods on that action class,
just provides a shorthand mapping from urls that begin "/home_".
I would expect "/home_view.do?method:update" to call update(), not
doUpdate(), although "/home_view.do?method:doUpdate" should call doUpdate().
Note: I've read code and theorized here, but not tested the hypotheses.
-Dale
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: [struts] s2 and DispatchAction
Posted by Dave Newton <ne...@yahoo.com>.
Does that deal with the submit button name thing?
d.
--- Jeromy Evans <je...@blueskyminds.com.au>
wrote:
> Dale Newfield wrote:
> > Don Brown wrote:
> >> Little known fact, but you can specify the method
> via:
> >>
> >> "?method:MY_METHOD_NAME"
> >>
> >> This code exists to support the method attribute
> on the submit tag,
> >> allowing you to submit the form to different
> methods based on what
> >> button is clicked.
> >
> > I wondered how the submit tag argument worked.
> > I would argue this is as big a security
> vulnerability as the
> > "action!method" capability. (As, for example, it
> can allow less
> > privileged users to access more privileged methods
> that the author
> > thought were protected via the url pattern by
> something like
> >
>
org.acegisecurity.intercept.web.FilterSecurityInterceptor.)
> Is there
> > any way to restrict which methods are valid there,
> or to turn this
> > capability off?
> >
> > -Dale
> I always use the following configuration to minimise
> the vulnerability::
>
> <action name="/home_*" method="do{1}">
>
> </action>
>
> With that setting, only methods with the prefix "do"
> in their name can
> be executed.
> ie. ?method:update calls doUpdate()
>
> cheers,
> Jeromy Evans
>
>
---------------------------------------------------------------------
> To unsubscribe, e-mail:
> user-unsubscribe@struts.apache.org
> For additional commands, e-mail:
> user-help@struts.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: [struts] s2 and DispatchAction
Posted by Jeromy Evans <je...@blueskyminds.com.au>.
Dale Newfield wrote:
> Don Brown wrote:
>> Little known fact, but you can specify the method via:
>>
>> "?method:MY_METHOD_NAME"
>>
>> This code exists to support the method attribute on the submit tag,
>> allowing you to submit the form to different methods based on what
>> button is clicked.
>
> I wondered how the submit tag argument worked.
> I would argue this is as big a security vulnerability as the
> "action!method" capability. (As, for example, it can allow less
> privileged users to access more privileged methods that the author
> thought were protected via the url pattern by something like
> org.acegisecurity.intercept.web.FilterSecurityInterceptor.) Is there
> any way to restrict which methods are valid there, or to turn this
> capability off?
>
> -Dale
I always use the following configuration to minimise the vulnerability::
<action name="/home_*" method="do{1}">
</action>
With that setting, only methods with the prefix "do" in their name can
be executed.
ie. ?method:update calls doUpdate()
cheers,
Jeromy Evans
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: [struts] s2 and DispatchAction
Posted by Dale Newfield <Da...@Newfield.org>.
Moved from user list.
Dale Newfield wrote:
>> "?method:MY_METHOD_NAME"
> Is there any way to restrict which methods are valid there, or to
> turn this capability off?
Reading the source
http://svn.apache.org/viewvc/struts/struts2/trunk/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java?view=markup
indicates the answer is "no".
I propose adding a check for "allowDynamicMethodCalls" in this code as
well (which if not set would effectively ignore the parameter). I
recognize that this may break some functionality (namely alternate
submit buttons in forms), but as this is really a vulnerability, I think
it is important to address...
...and we could regain the submit button functionality with javascript
that changes the form submission action url...
-Dale
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org