struts cancel and validation security problem

[er...@swapsimple.com]

	I've identified a potential security problem in how struts handles the
interaction between form validation and cancel buttons.

	The html:cancel tag works by creating an html submit button that has
a magic name (org.apache.struts.taglib.html.CANCEL).  When this parameter
is set the struts Action class _unconditionally_ skips form validation.
Since this is simply an additional parameter it is trivial for anyone
to craft an otherwise valid form submission that bypasses validation.
This, obviously, can cause all sort of problems.

	It is possible to work around this problem in various ways.  Code
can be added to the execute() method (or in each non-cancel method, in
the case of a DispatchAction class) to check for the magic parameter and
to return an error if it is set inappropriately.  Alternately, a
similar check can be placed in the validate() method before called
super.validate().  Both of these have the drawback that this change
must be made to _each_ _and_ _every_ action, even if the developer has
no intention of the allowing the action to be "cancellable".

	There are better ways to do this.  I found a reference to a way to
set the form parameter and expected value that would indicate a cancel
in something called FormTool, part of VelocityStruts.  I'm not quite
sure what these are, but that approach seems to be a more secure way
of handling cancels.  I propose that the unconditional validation bypass
be removed and replaced with a way for the developer to specify both
the parameter name and value that should skip validation.
	I'm thinking of a couple of additional sub-elements to the action
element:

<action ... >
	<cancel-property formparam="...name of the form parameter...">
		<valid-value key="...a key from the message resources..."/>
		<valid-value value="...a string..."/>
	</cancel-property>
	...
</action>
formparam is optional, and defaults to "org.apache.struts.taglib.html.CANCEL"
The list of valid values is empty by default.  The code in RequestProcessor
would then check the list of valid values when deciding whether to skip
validation or not.

Or, a simpler but less useful alternative would be to add a
"cancellable" attribute to the action element.  Iff this is set to
true then the presence of the magic form parameter would cause
validation to be skipped.

Thoughts?

eric


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

Re: struts cancel and validation security problem

[Joe Germuska <Jo...@Germuska.com>]

Hm, it looks like you're on to something.  This should probably be 
filed as an issue in Bugzilla for better tracking. 
http://issues.apache.org/bugzilla/

Any changes which would break existing installations should probably 
go through a cycle of warning and deprecation.  I suppose given 
potential security implications, we might be willing to be quick 
about things.

Not being a big user of the cancellation functionality, i prefer the 
simpler implementation as being more straightforward in the 
configuration as well as being quicker to implement, but I can see 
where it would be more secure to make the magic parameter and value 
configurable.

Joe


At 8:57 PM -0600 12/17/04, erh+struts@swapsimple.com wrote:
>	I've identified a potential security problem in how struts handles the
>interaction between form validation and cancel buttons.
>
>	The html:cancel tag works by creating an html submit button that has
>a magic name (org.apache.struts.taglib.html.CANCEL).  When this parameter
>is set the struts Action class _unconditionally_ skips form validation.
>Since this is simply an additional parameter it is trivial for anyone
>to craft an otherwise valid form submission that bypasses validation.
>This, obviously, can cause all sort of problems.
>
>	It is possible to work around this problem in various ways.  Code
>can be added to the execute() method (or in each non-cancel method, in
>the case of a DispatchAction class) to check for the magic parameter and
>to return an error if it is set inappropriately.  Alternately, a
>similar check can be placed in the validate() method before called
>super.validate().  Both of these have the drawback that this change
>must be made to _each_ _and_ _every_ action, even if the developer has
>no intention of the allowing the action to be "cancellable".
>
>	There are better ways to do this.  I found a reference to a way to
>set the form parameter and expected value that would indicate a cancel
>in something called FormTool, part of VelocityStruts.  I'm not quite
>sure what these are, but that approach seems to be a more secure way
>of handling cancels.  I propose that the unconditional validation bypass
>be removed and replaced with a way for the developer to specify both
>the parameter name and value that should skip validation.
>	I'm thinking of a couple of additional sub-elements to the action
>element:
>
><action ... >
>	<cancel-property formparam="...name of the form parameter...">
>		<valid-value key="...a key from the message resources..."/>
>		<valid-value value="...a string..."/>
>	</cancel-property>
>	...
></action>
>formparam is optional, and defaults to "org.apache.struts.taglib.html.CANCEL"
>The list of valid values is empty by default.  The code in RequestProcessor
>would then check the list of valid values when deciding whether to skip
>validation or not.
>
>Or, a simpler but less useful alternative would be to add a
>"cancellable" attribute to the action element.  Iff this is set to
>true then the presence of the magic form parameter would cause
>validation to be skipped.
>
>Thoughts?
>
>eric
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
>For additional commands, e-mail: dev-help@struts.apache.org


-- 
Joe Germuska            
Joe@Germuska.com  
http://blog.germuska.com    
"Narrow minds are weapons made for mass destruction"  -The Ex

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