While in my own work, I don't use html:cancel as a firewall to bad data, I
can see where this has a potential risk for a dos attack. Such an attack
might utilize database resources that would otherwise have been blocked on
the client (javascript) or at the action class.
I think we should change the behavior to *not* populate *unless overridden*
with Joe's proposed configuration.
I propose we fix this in the 1.2.x branch, apply it to head as well, and
roll another 1.2.x.
Your thoughts?
--
James Mitchell
Software Engineer / Open Source Evangelist
Consulting / Mentoring
678.910.8017
----- Original Message -----
From: "Joe Germuska" <[EMAIL PROTECTED]>
Newsgroups: gmane.comp.jakarta.struts.devel
Sent: Monday, January 23, 2006 10:10 AM
Subject: Re: Validation Security Hole?
Here's my first thoughts on possible approaches to addressing the
problem, to kick off further discussion on the dev list:
- SAF1.2 and before: ? document, don't fix? add config req'm'ts on
action mapping? Refer to discussion on user list for various options.
- SAF1.3+: make cancel processing a command which you have to include in
your request processing chain, and perhaps disclude it by default? [I'm
not familiar enough with how you deploy chains on a per-action basis to
know if this is the right way to do it...]
I think this would be affected by what is done with 1.2... if it is
modified to by default not allow Actions to be cancelable for instance, I
would think it would be better to replicate that change into 1.3, which
would likely entail changing the default chain. Open for discussion
obviously :)
To be honest, I've never really picked up on the use case for the "cancel"
functionality. Is this something that a lot of people use?
In any case, I don't think we want to make an entirely new chain or even
command out of this; all we need to do is change the behavior of the
"handleCancelled" method in AbstractPopulateActionForm.
(http://struts.apache.org/struts-action/xref/org/apache/struts/chain/commands/AbstractPopulateActionForm.html#144)
I would propose that the simplest approach would be to change it to
consult the ActionMapping for an arbitrary string property with a key like
"CANCEL_PARAMETER". It would use this parameter name instead of the
Globals.CANCEL (perhaps also adding a check for the same param_name + ".x"
to catch the case of an image button, as does the current hard-coded
parameter check.
If we want to make things easiest for compatibility, we could add a
boolean property to the AbstractPopulateActionForm like
"legacyCancelSemantics" -- if this value were true, things would work as
they always have; if false, they would work as I've described.
Depending on the sense of urgency between closing this gap and not
breaking existing code, the default value of this property could be true
or false, but it would be easy to change in a local copy of the
chain-config.xml (This is the kind of thing that starts to push whether
it's suitably easy to change chain-config.xml for local purposes,
something about which I have some reservations for novice users.)
Reactions?
Joe
--
Joe Germuska
[EMAIL PROTECTED] * http://blog.germuska.com
"You really can't burn anything out by trying something new, and
even if you can burn it out, it can be fixed. Try something new."
-- Robert Moog
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
