You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@struts.apache.org by Brian Relph <re...@gmail.com> on 2008/02/28 16:39:42 UTC

action security

Hi, I am concerned about security in my struts2 actions.  I am using spring
to auto-wire my actions by name, but this leads me to believe that a
malicious user can set action properties that i do not want them to.  For
example, i have a .jsp with a form input of "name".  My action has a
getter/setter for the String property "name".  this property is
automatically populated (by the parameterInterceptor?).  I also have a
userDao object on my action, also with getters/setters so that spring can
auto-wire it.  Is there anything that prevents a user from adding a form
input of "userDao.password" (just for example), and changing the password on
my userDao?  Do i need to do something to only make certain properties of my
action available to be set from request parameters?

Thanks,

-- 
Brian

Re: action security

Posted by Dave Newton <ne...@yahoo.com>.
--- Brian Relph <re...@gmail.com> wrote:
> I have also created a new AnnontationParameterInterceptor, along with a
> class-level annontation and a field-level annontation.  As of now, the
> annotations just store a boolean value of whether to allow the field to be
> set, and for the class, what the default policy is for object's fields.
> 
> Would anyone be interested in this code?  Would some of this go in the
> xwork distribution?  I would like to add regular expression support to the
> class-level annontation, but other than that, I prefer the annotations over
> the interface/method.

Your best bet would be to file a request-for-enhancement JIRA against XWork
and include the patch; I think any additional annotation support would be
welcomed and it would just be a matter of testing the patch etc. and
committing.

Dave


---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: action security

Posted by Brian Relph <re...@gmail.com>.
I was able to use the ParameterNameAware interface after all ... I needed to
rebuild my project, it wasn't updating in my workspace for some reason.

I have also created a new AnnontationParameterInterceptor, along with a
class-level annontation and a field-level annontation.  As of now, the
annotations just store a boolean value of whether to allow the field to be
set, and for the class, what the default policy is for object's fields.

Would anyone be interested in this code?  Would some of this go in the xwork
distribution?  I would like to add regular expression support to the
class-level annontation, but other than that, I prefer the annotations over
the interface/method.

On Thu, Feb 28, 2008 at 3:28 PM, Dave Newton <ne...@yahoo.com> wrote:

> --- Laurie Harper <la...@holoweb.net> wrote:
> > That would require a getMailSender() on the action, wouldn't it? I'd
> > strongly suggest not having getters for 'sensitive' internals like that
>
> It's pretty typical to have a service injected like that, though. The
> issue
> here is that a sensitive configuration parameter is being trivially
> exposed
> via a Spring-settable property.
>
> > >> --- Brian Relph <re...@gmail.com> wrote:
> > >>> So i guess this is a legitimate security concern.  Is there a
> > >>> cleaner way to do this?  Is there annotations support for it?
> > >> Not that I'm aware of.
>
> Have you solved your ParameterNameAware problem?
>
> I can't reproduce it; if I have a Spring-injected class (my test uses
> 'testService') with a property and my 'acceptableParameterName' method
> returns 'false' for parameters starting with the name of the service's
> parameter it's not being set.
>
> In other words, if the parameter name 'startsWith("testService")' I return
> false, the parameter in the service isn't being set on a request
> containing
> something like 'testService.senderName'.
>
> Dave
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
> For additional commands, e-mail: user-help@struts.apache.org
>
>


-- 
Brian

Re: action security

Posted by Dave Newton <ne...@yahoo.com>.
--- Laurie Harper <la...@holoweb.net> wrote:
> That would require a getMailSender() on the action, wouldn't it? I'd 
> strongly suggest not having getters for 'sensitive' internals like that

It's pretty typical to have a service injected like that, though. The issue
here is that a sensitive configuration parameter is being trivially exposed
via a Spring-settable property.

> >> --- Brian Relph <re...@gmail.com> wrote:
> >>> So i guess this is a legitimate security concern.  Is there a
> >>> cleaner way to do this?  Is there annotations support for it?
> >> Not that I'm aware of.

Have you solved your ParameterNameAware problem?

I can't reproduce it; if I have a Spring-injected class (my test uses
'testService') with a property and my 'acceptableParameterName' method
returns 'false' for parameters starting with the name of the service's
parameter it's not being set.

In other words, if the parameter name 'startsWith("testService")' I return
false, the parameter in the service isn't being set on a request containing
something like 'testService.senderName'.

Dave


---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: action security

Posted by Laurie Harper <la...@holoweb.net>.
Brian Relph wrote:
> Here is a better example of what i am concerned about ...
> 
> I send emails in my application, and i use spring to configure a
> JavaMailSender - this has a getter/setter for the"from" email address ... as
> well, i use a singleton bean for this object (this is the spring default
> nowadays), so a malicious user could send in a form parameter for
> mailSender.fromEmail, and then every email that my application sends would
> be from whatever string the submitted ...

That would require a getMailSender() on the action, wouldn't it? I'd 
strongly suggest not having getters for 'sensitive' internals like that :-)

You might also want to consider breaking your action into two pieces: 
the action that handles exposing data to pages / reading input from 
users, and a 'service' class which holds references to DAOs and utility 
classes like JavaMailSender. That could greatly reduce the surface area 
of what is addressable through the action.

L.

> I am implementing the ParameterNameAware interface in struts 2.0.11, and am
> checking doing parameterName.startsWith("mailSender") ? false : true;  --
> this is returning false, however, the value is still being set on my object,
> am i doing something wrong here?
> 
> 
> On Thu, Feb 28, 2008 at 10:35 AM, Dave Newton <ne...@yahoo.com> wrote:
> 
>> --- Brian Relph <re...@gmail.com> wrote:
>>> So i guess this is a legitimate security concern.  Is there a
>>> cleaner way to do this?  Is there annotations support for it?
>> Not that I'm aware of.
>>
>> Note that setting a DAO-style class with a string would most likely end in
>> an
>> exception.
>>
>> Dave
>>
>>
>> ---------------------------------------------------------------------
>> 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: action security

Posted by Brian Relph <re...@gmail.com>.
Here is a better example of what i am concerned about ...

I send emails in my application, and i use spring to configure a
JavaMailSender - this has a getter/setter for the"from" email address ... as
well, i use a singleton bean for this object (this is the spring default
nowadays), so a malicious user could send in a form parameter for
mailSender.fromEmail, and then every email that my application sends would
be from whatever string the submitted ...

I am implementing the ParameterNameAware interface in struts 2.0.11, and am
checking doing parameterName.startsWith("mailSender") ? false : true;  --
this is returning false, however, the value is still being set on my object,
am i doing something wrong here?


On Thu, Feb 28, 2008 at 10:35 AM, Dave Newton <ne...@yahoo.com> wrote:

> --- Brian Relph <re...@gmail.com> wrote:
> > So i guess this is a legitimate security concern.  Is there a
> > cleaner way to do this?  Is there annotations support for it?
>
> Not that I'm aware of.
>
> Note that setting a DAO-style class with a string would most likely end in
> an
> exception.
>
> Dave
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
> For additional commands, e-mail: user-help@struts.apache.org
>
>


-- 
Brian

Re: action security

Posted by Dave Newton <ne...@yahoo.com>.
--- Brian Relph <re...@gmail.com> wrote:
> So i guess this is a legitimate security concern.  Is there a 
> cleaner way to do this?  Is there annotations support for it?

Not that I'm aware of.

Note that setting a DAO-style class with a string would most likely end in an
exception.

Dave


---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org


Re: action security

Posted by Brian Relph <re...@gmail.com>.
So i guess this is a legitimate security concern.  Is there a cleaner way to
do this?  Is there annotations support for it?

On Thu, Feb 28, 2008 at 10:05 AM, Daniel Baldes <db...@open.ch> wrote:

> Brian Relph wrote:
> > Hi, I am concerned about security in my struts2 actions.  I am using
> spring
> > to auto-wire my actions by name, but this leads me to believe that a
> > malicious user can set action properties that i do not want them to.
>  For
> > example, i have a .jsp with a form input of "name".  My action has a
> > getter/setter for the String property "name".  this property is
> > automatically populated (by the parameterInterceptor?).  I also have a
> > userDao object on my action, also with getters/setters so that spring
> can
> > auto-wire it.  Is there anything that prevents a user from adding a form
> > input of "userDao.password" (just for example), and changing the
> password on
> > my userDao?  Do i need to do something to only make certain properties
> of my
> > action available to be set from request parameters?
> >
> > Thanks,
> >
>
> Hi Brian,
>
> you can implement the interface "ParameterNameAware". Then, every
> parameter name is passed to the method "boolean
> acceptableParameterName(String name)" and the parameter is only set when
> it returns true.
> Cheers,
> Daniel
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
> For additional commands, e-mail: user-help@struts.apache.org
>
>


-- 
Brian

Re: action security

Posted by Daniel Baldes <db...@open.ch>.
Brian Relph wrote:
> Hi, I am concerned about security in my struts2 actions.  I am using spring
> to auto-wire my actions by name, but this leads me to believe that a
> malicious user can set action properties that i do not want them to.  For
> example, i have a .jsp with a form input of "name".  My action has a
> getter/setter for the String property "name".  this property is
> automatically populated (by the parameterInterceptor?).  I also have a
> userDao object on my action, also with getters/setters so that spring can
> auto-wire it.  Is there anything that prevents a user from adding a form
> input of "userDao.password" (just for example), and changing the password on
> my userDao?  Do i need to do something to only make certain properties of my
> action available to be set from request parameters?
> 
> Thanks,
> 

Hi Brian,

you can implement the interface "ParameterNameAware". Then, every 
parameter name is passed to the method "boolean 
acceptableParameterName(String name)" and the parameter is only set when 
it returns true.
Cheers,
Daniel

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org