At 3:37 PM -0500 1/23/06, Frank W. Zammetti wrote:
I'm still not sure I see how this suggestion solves the problem... but,
since three people seem to think it will at this point, I have to assume
I'm just being dense... It happens from time to time ;) Can you walk me
(and I assume Paul and Rick, who have been in on this discussion) through
it again?
The problem is that a user can pass a well-known request parameter to
any request and disable validation.
The fact that you can bypass validation and still have your action
execute is a "feature" not a "bug", at least in so far as at some
time, someone saw a need for it, and documented clearly the behavior.
If someone were using this feature, one would assume that they had
coded for the possibility of the action being cancelled and the
action being presented with a non-validated form.
The proposed solution requires that the person intentionally using
this feature make one further indication in the struts-config file,
reinforcing the logical contract that says "it is ok to invoke this
action with an invalid form even though I have set "validate='true'".
The key point here is that you are explicitly turning on attention to
the "cancel" parameter for any action (path) that accepts it, leaving
all other actions invulnerable to the "attack".
I certainly don't think it's a very elegant solution, but it seems to
be relatively low impact. I could probably brainstorm some more
elegant approaches, but in the absence of champions for the basic
functionality, I would save that for a time when there was at least
beer, which alas, is not now.
Joe
--
Frank W. Zammetti
Founder and Chief Software Architect
Omnytex Technologies
http://www.omnytex.com
AIM: fzammetti
Yahoo: fzammetti
MSN: [EMAIL PROTECTED]
On Mon, January 23, 2006 3:16 pm, Joe Germuska said:
At 3:11 PM -0500 1/23/06, James Mitchell wrote:
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?
The hitch to this is that "arbitrary properties" haven't been applied
to Struts 1.2.x, although it shouldn't be too hard to port that over.
Otherwise to do this in Struts 1.2 would require defining a new bean
property on ActionMapping, or finding a different place to put the
config.
Joe
--
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]
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
--
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]
