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, [EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
--
Joe Germuska [EMAIL PROTECTED] http://blog.germuska.com "Narrow minds are weapons made for mass destruction" -The Ex
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
