You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@struts.apache.org by Ha...@alcatel.com.au on 2006/01/20 05:37:51 UTC
error messages doesn't appear if validate is called programatically
Hi,
I have started using struts recently.
I don't want to use default validation checking so I changed
struts-config.xml.
.
.
<action path="/SomethingAction" ...
.
.
.
validate="false"
.
.
In the SomethingAction class's execute method I call
if (form.validate().size>0 )
// forward to input page
In auto validation case ( validate="true") error messages I configured
appears on Input JSP
but if validate="false" as I configured above then messages soesn't
appear on the Input JSP.
Could anyone point me to some documents or provide hint about this
problem?
Thanks and regards
Hakan
This email may contain privileged/confidential information. You may not copy or disclose this email to anyone without the written permission of the sender. If you have received this email in error please kindly delete this message and notify the sender. Opinions expressed in this email are those of the sender and not necessarily the opinions of the employer.
This email and any attached files should be scanned to detect viruses. No liability will be accepted by the employer for loss or damage (whether caused by negligence or not) as a result of email transmission.
Re: Validation Security Hole?
Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
P.S., Paul, I'd suggest going ahead and opening a ticket for this and
reference this thread... say that a couple of different solutions were
suggested and patches can be easily created once a consensus on the
right answer is reached (I know you said you'd create a patch, and I
would too if necessary once we can all come together to decide which way
to go). This will put it more officially in front of the committers for
their consideration, since ultimately they are going to have to decide
if they agree this is an issue or not, and then agree with how to
resolve it. The first step in all that is generally a ticket being
opened. This is your baby, so go for it! :)
Frank
Paul Benedict wrote:
>>> If everyone used dispatch-type Actions, I would disagree because then it would just be a matter
> of providing a cancel() method and making sure that got called.
>
> There's actually a funny bug here. cancel() method ALWAYS gets called in a dispatch action BUT
> it's default behavior is to return null; and if null is returned then it considers it
> non-implemented and goes to the intended method.
>
> That's actually a bug too because NULL is a legitmate return value from a struts action. If you
> handled the request directly, say by writing directly to the servlet OutputStream or
> HttpServletResponse.sendRedirect(), you're supposed to retun null. That's what the docs say =o)
> But in this case NULL means "not implemented".
>
> Anyway, it must be the weekend because no one from the Struts team have chimed in -- but they
> deserve days off too. :) .... maybe ;) I'd like to actually have a solution to submit; I am going
> to implement my suggestion privately in my applications but I would like an "official" one get
> into Struts too.
>
> Paul
>
>
> __________________________________________________
> Do You Yahoo!?
> Tired of spam? Yahoo! Mail has the best spam protection around
> http://mail.yahoo.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
> For additional commands, e-mail: user-help@struts.apache.org
>
>
>
> .
>
--
Frank W. Zammetti
Founder and Chief Software Architect
Omnytex Technologies
http://www.omnytex.com
AIM: fzammetti
Yahoo: fzammetti
MSN: fzammetti@hotmail.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
Paul Benedict wrote:
>>> Cancelable Actions (independently on the Action type: normal Actions, DispatchActions) could
> even implement a Cancelable interface with a cancel method.
>
> Tamas, good one. I thought of this too but never mentioned it because implementing interfaces
> doesn't seem too cool/accepted in the Struts framework yet. (But that will change in 1.3.) If I
> add a Cancelable interface to my actions, then that could indicate to the framework that these
> actions deserve to be canceled; otherwise ignore the request parameter.
I didn't think it was a good idea at first, but now I don't think it's a
bad one.
The first question is whether backwards-compatibility *should* be
maintained... my usual stance is yes, it should be, and that was driving
my suggestions before.
Thinking about it more though, that's like Microsoft saying "yes, we
know this WMF flaw is a security issue, so we're going to provide a
mechanism to patch the hole, but it's going to be optional on the part
of developers... by default, the hole will continue to exist so that
backwards-compatibility is maintained". We'd all yell they are crazy if
they said that!
No, in this case I think it makes sense to break compatibility. The
question then becomes what is the least onerous way to do it so that
someone upgrading an existing app has as little to do as possible while
still being architecturally sound.
Well, the config parameters would probably be the easiest way, but the
interface seems more elegant architecturally. If I was maintaining an
app and I wanted to close this security hole with the least amount of
effort though, I would contend Paul's approach is probably better... If
we assume the new "cancelable" attribute on the <controller> defaults to
false, then I really just need to go through and add cancelable="true"
to any mappings that correspond to legitimately cancelable Actions,
which chances are isn't many. Contrast this to having to go through and
implement an interface and rebuild the app. Not a huge difference
perhaps, but enough (and don't forget that some shops have rather
onerous requirements before you could deploy the new build).
The interface isn't a bad suggestion though, after thinking about it a
bit. Either way :)
Frank
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
>> Cancelable Actions (independently on the Action type: normal Actions, DispatchActions) could
even implement a Cancelable interface with a cancel method.
Tamas, good one. I thought of this too but never mentioned it because implementing interfaces
doesn't seem too cool/accepted in the Struts framework yet. (But that will change in 1.3.) If I
add a Cancelable interface to my actions, then that could indicate to the framework that these
actions deserve to be canceled; otherwise ignore the request parameter.
Rick, your suggestion is good too. Did you know Tapestry works the same way? You're in charge of
calling the validator and determining what it should do. In your case you would always want the
form populated so you could determine if to validate or not.
Frank, I agree: I don't think it makes sense populating the form if validate="true" and the cancel
token is in the parameter. Besides, the extra benefit here is that if you're in a form wizard and
you have 2 out of 3 pages validated, if they cancel on the 3rd page, you at least have 2 pages of
data that you know is good; re-populating the form without validation then looses this confidence.
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
>> If everyone used dispatch-type Actions, I would disagree because then it would just be a matter
of providing a cancel() method and making sure that got called.
There's actually a funny bug here. cancel() method ALWAYS gets called in a dispatch action BUT
it's default behavior is to return null; and if null is returned then it considers it
non-implemented and goes to the intended method.
That's actually a bug too because NULL is a legitmate return value from a struts action. If you
handled the request directly, say by writing directly to the servlet OutputStream or
HttpServletResponse.sendRedirect(), you're supposed to retun null. That's what the docs say =o)
But in this case NULL means "not implemented".
Anyway, it must be the weekend because no one from the Struts team have chimed in -- but they
deserve days off too. :) .... maybe ;) I'd like to actually have a solution to submit; I am going
to implement my suggestion privately in my applications but I would like an "official" one get
into Struts too.
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Tamas Szabo <sz...@gmail.com>.
>
> Interestingly, the doc for the cancel tag *does* say that validate()
> won't be called and that the Action will be called normally. I never
> noticed this before. So, at least no one can claim this behavior isn't
> documented :)
Yes, but if you don't want to use the cancel tag you probably don't read its
doc.
And if you don't use it you wont put the if (isCancelled(request) ) check in
your execute.
And this is exactly the case when you could get in trouble ...
There's really two problems here... one revolves around how this feature
> maybe should have been designed in the first place. Tamas may be right
> in that regard. The second problem though is you have apps built with
> it the way it is now, for better or worse, and breaking those to close
> this hole isn't really the best idea.
Yes, it's very easy to criticize something after it was implemented.
But I admit a solution that isn't backward compatible isn't viable.
With that in mind, I'm thinking something along the lines of Paul's
> original suggestion may in fact be the best... add a "cancelable"
> attribute to the <action> element in config. Default to true. When set
> to false and the Action is called, that's the "hacker" case we want to
> avoid... maybe throw an IllegalStateException? Maybe look for an
> "illegalCancel" forward? Not sure what's best, but the point is to
> avoid calling execute() in that case. This would maintain the existing
> behavior by default too.
>
> No, actually, better yet, as Paul suggested, add cancelable to the
> <processor> element as well. I think you really need it to be on both
> <processor> and <action> though... set it to false on the <processor> to
> globally close the security hole, then set it to true on the <action>'s
> where it applies.
Well this seems to be backward compatible. But you will still have to add
tha cancelable= "true" for all the mappings that can be cancelled in your
existing
applications.
Frank
Tamas
Re: Validation Security Hole?
Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
If everyone used dispatch-type Actions, I would disagree because then it
would just be a matter of providing a cancel() method and making sure
that got called. But, since not everyone does (including me whenever I
can avoid it), that's not the end of the story.
Interestingly, the doc for the cancel tag *does* say that validate()
won't be called and that the Action will be called normally. I never
noticed this before. So, at least no one can claim this behavior isn't
documented :)
There's really two problems here... one revolves around how this feature
maybe should have been designed in the first place. Tamas may be right
in that regard. The second problem though is you have apps built with
it the way it is now, for better or worse, and breaking those to close
this hole isn't really the best idea.
With that in mind, I'm thinking something along the lines of Paul's
original suggestion may in fact be the best... add a "cancelable"
attribute to the <action> element in config. Default to true. When set
to false and the Action is called, that's the "hacker" case we want to
avoid... maybe throw an IllegalStateException? Maybe look for an
"illegalCancel" forward? Not sure what's best, but the point is to
avoid calling execute() in that case. This would maintain the existing
behavior by default too.
No, actually, better yet, as Paul suggested, add cancelable to the
<processor> element as well. I think you really need it to be on both
<processor> and <action> though... set it to false on the <processor> to
globally close the security hole, then set it to true on the <action>'s
where it applies.
Frank
Adam Hardy wrote:
> Tamas Szabo on 22/01/06 07:30, wrote:
>>> There is a legitimate case: when an form can be cancelled, you do
>>> want to skip client-side and server-side validation. That's just
>>> fine because in these case you do want to call the cancelled() method
>>> from DispatchAction, dump out any state you collected, and
>>> go to the appropriate forward.
>>>
>>
>>
>> In my oppinion it isn't ahndled correctly and I don't think that it
>> should be solved by configuration script. If the user presses Cancel
>> then the processing should go on another path. It doesn't make sense
>> to populate and to validate if cancel was pressed.
>
> I agree. I _do_ use the cancel button on many forms and this is what I
> implement - if Struts did it itself, it would save me time and effort.
>
> Plus it would be safer on other actions where I don't use the cancel
> button.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
> For additional commands, e-mail: user-help@struts.apache.org
>
>
>
>
--
Frank W. Zammetti
Founder and Chief Software Architect
Omnytex Technologies
http://www.omnytex.com
AIM: fzammetti
Yahoo: fzammetti
MSN: fzammetti@hotmail.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Adam Hardy <ah...@cyberspaceroad.com>.
Tamas Szabo on 22/01/06 07:30, wrote:
>> There is a legitimate case: when an form can be cancelled, you do
>> want to skip client-side and server-side validation. That's just
>> fine because in these case you do want to call the cancelled()
>> method from DispatchAction, dump out any state you collected, and
>> go to the appropriate forward.
>>
>
>
> In my oppinion it isn't ahndled correctly and I don't think that it
> should be solved by configuration script. If the user presses Cancel
> then the processing should go on another path. It doesn't make sense
> to populate and to validate if cancel was pressed.
I agree. I _do_ use the cancel button on many forms and this is what I
implement - if Struts did it itself, it would save me time and effort.
Plus it would be safer on other actions where I don't use the cancel
button.
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Tamas Szabo <sz...@gmail.com>.
Hi!
Very interesting problem!
In my oppinion is clearly a bug!
On 1/22/06, Paul Benedict <paul4christ79@yahoo.com > wrote:
>
> >>I can't think of a good reason it wouldn't call validate() too, like any
> other request, just
> because the action was canceled. Like I said, maybe someone can come up
> with a reasonable
> explanation for that behavior, but I can't see it :)
>
> There is a legitimate case: when an form can be cancelled, you do want to
> skip client-side and
> server-side validation. That's just fine because in these case you do want
> to call the cancelled()
> method from DispatchAction, dump out any state you collected, and go to
> the appropriate forward.
>
> I want to make sure we don't lose focus here. The problem is not that
> cancelled doesn't validate,
> but that ANY action can have its validation bypassed. This is not
> semantically correct for some
> actions, especially GET actions whose sole job is to retrieve data without
> any state except for
> the request parameters they receive. Usually people cancel form POST
> input, but not GET input
> because those could be links in emails, bookmarks, etc. Explain the
> semantics of canceling a
> bookmark or a link from an email? It's pretty difficult to think of a
> general case.
>
> So, as I've said before, I do believe there needs to be a switch that
> determines when by-passing
> validation IS A VALID USE CASE. Having that feature always be on is
> logically wrong and creates a
> world where many stateless requests which use form validation to defend
> against bad user input is
> suddenly defeated.
In my oppinion it isn't ahndled correctly and I don't think that it should
be solved by configuration
script.
If the user presses Cancel then the processing should go on another path.
It doesn't make sense to populate and to validate if cancel was pressed.
I would like to go one step further and say that it doesn't make sense to
call execute either!
If cancel was pressed a cancel() method should be called.
Currently in an Action's execute you would handle cancellation like this:
if (isCancelled(request)) {
// process the cancellation
// clear data from session
// forward to the "cancel"
}
// do the normal logic of the execute.
If we look at the processing of the cancellation the code looks exactly like
you had
another execute() in the Action's "normal" execute. It does some processing
and then
it forwards somewhere else. (Beside this Struts relies on the
developer to put in the if (isCancelled(request)) otherwise it will just
call execute()
which is a problem in my oppinion).
So it would make sense to have a execute-like cancel() method that is called
if the Action was cancelled just like in DispatchAction.
Cancelable Actions (independently on the Action type: normal Actions,
DispatchActions) could even implement a Cancelable interface with a cancel
method.
However for Actions when you traditionaly have only one execute method I
would always
have a CancelAction and in its execute method I would handle the
cancellation.
This cancel feature was probably added with DispatchAction in mind(for which
it works almost perfectly (it shouldn't populate the form in my oppinion))
and to support in Action too they added
this isCancelled() hack which migt look cool but hacks usually break things
:-)
> Paul
Tamas
Re: Validation Security Hole?
Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
I'm not sure this solves the problem Rick... partially it does...
As Paul pointed out, the cancel function itself is a legitimate case.
In that situation, you wouldn't want the form to be populated (you
wouldn't care if it was or wasn't I suppose, but ideally why bother
doing the extra work?) and you wouldn't want validate() to be called
because it might reject the request when it shouldn't...
For example... say I have a 3-page wizard flow. After each page I stick
the entered values in session. On the third page I have the capability
of clicking cancel. When this occurs, I'm going to want to clear out
the stored values from session. Imagine that the last page *also* has
some entry fields that are usually validated, and let's say one of them
is required. If the user clicks cancel, whether you call validate from
the Action or not, the user is forced to enter a value just to get past
validation, even if they click cancel! The UI would appear broken to
the user, and rightly so.
Now, you may say that "yeah, but I would call isCancelled() before I do
the validation", and indeed, that would make the UI work as it should.
The problem though is that you will have to remember to do that check IN
EVERY ACTION, or risk having a similar UI issue elsewhere. It's just
extra code you have to copy-and-paste, or have your own base Action
class to extend from.
I agree, if everyone did as you suggest this issue wouldn't be as
severe. In fact, you would take care of the security issue (assuming
*every* Action called validate()), but you would trade it in for a
broken UI mechanism anytime the cancel button was involved. Not as big
a deal, but still not the best answer :)
Frank
Rick Reumann wrote:
> All of this just adds *ONE MORE REASON* to my list of *NEVER EVER* use
> validate="true". I always call validation manually from my Action class
> and the sooner people get into a habit of this the way better off they
> will be. If you validate or call the form's validate method manually
> from your Action class all these problems go away including the problem
> we get posted here on the list at least a couple times a week concerning
> list items going out of scope when validation fails. Validating manually
> also solves the problem Paul addresses as form validation ALWAYS takes
> place.
>
> I think the bug is sort of in validate='true' :) I think that should
> just be dropped and when people need to validate they just call it from
> their Action class.
> http://www.learntechnology.net/validate-manually.do
>
> The code is this simple to add:
> ActionErrors errors = form.validate( mapping, request );
> if ( errors != null && !errors.isEmpty ) {
> saveErrors(request, errors);
> setUp(request); //preps request for lists and stuff
> return (mapping.findForward(UIconstants.VALIDATION_FAILURE));
> }
>
> You could even add that to a utility method so it's just one line of code.
>
> Now you have either one line of code in your Action or one line of code
> in your ActionMapping, yet the first scenario solves all these problems.
>
> (side note, I don't even really like using the validator so I don't even
> call form.validate() but call a validation method in my action class
> that does my validation, but I know most like to use the validator so
> I'm showing how easy it is to call manually).
>
>
> Frank W. Zammetti wrote:
>> Rick Reumann wrote:
>>> Maybe I'm missing how the above would happen. How would passing in
>>> the canceled parameter end up getting them access to a table? Oh
>>> wait, maybe this is with regular Actions with just an execute? It's
>>> been so long since I used a non Dispatch Action I'm not aware of the
>>> behavior. So I have an Action called "GetMeATable" with an execute
>>> method that returns a table from the db, if I pass in through a get
>>> parameter the canceled param it will still execute that action?
>>
>> Right, ordinary Action we're talking about. All Struts does is put a
>> request attribute in place to indicate the Action is being canceled.
>> It then goes on to populate the form, but *NOT* call validate() on
>> it. So, the problem arises if your Action doesn't check for the
>> canceled attribute and validate() is meant to stop a given request (as
>> in my scenario where it was checking for one of the allowed table names).
>>
>> As I Wrote this, I see that Paul responded and said it's a problem
>> with DispatchActions too, and I think he's right. In fact, his
>> response sums up my own pretty well :)
>>
>>> Even if the above is true, I'm not so sure that's a big deal as far
>>> as Struts goes in regard to it being a 'security problem with
>>> Struts'. You can always find a ton of security loop holes if you
>>> don't truly check that the logged in user has access to some backend
>>> procedure. For example if you have an update dispatch method or an
>>> "UpdateAction" you could easily type in the url
>>> someUrl?dispath=update&id=42&whatever=something etc and have it go
>>> through and be processed.
>>
>> Well, let me put it this way... if I worked for Microsoft, to me at
>> least, this would be a non-critical bug. Patch it on the regular
>> second Tuesday of the month cycle :) But I *do* think it is a bug (a
>> logic bug in this case) and *is* a security hole, even if arguably a
>> small one. Your absolutely right though, there are far more severe
>> security holes that a developer can open up without help from Struts
>> :) I don't think that should make anyone less interested in plugging
>> this one though.
>>
>>> Bottom line is if security is an issue, no one should ever rely on
>>> the validate method. That's just silly. I'm sure the docs even state
>>> that the purpose of the validate method is to just validate data
>>> entry (required fields, no Strings in number fields, etc.).
>>
>> But, how someone defines "data entry" can come into play... for
>> instance, in the scenario I described, selecting a table, from a
>> select say, would count as data entry, wouldn't it? Try this... let's
>> say we're talking about a *REALLY* bad developer and for some reason
>> he has a *TEXT* field for entering the table name, and then goes on to
>> check if it is an allowed value in validate(). Stupid design, in the
>> extreme! But an improper use of validate()? Probably not (debatable
>> at least).
>>
>> Let me try and come up with a better example...
>>
>> Let's say my bank's web site allows me to have a primary user account,
>> and I can link to this account all my checking accounts and savings
>> account to see them all in one "dashboard" display. I have to fill
>> out a form to have an account added. The form has account number on it.
>>
>> Let's say one of the validations is to be sure the checksum of the
>> entered account number matches one that would only be valid for me.
>> It's a relatively simple math check, so it's done in validate()
>> because really we're checking that the entered value is "valid".
>>
>> Now, the Action behind it calls some DAO to add the account for me.
>> But, it reasonably assumes that the data it gets in the form is valid.
>> So, if validate() is not executed, I could conceivably enter an
>> account number that isn't mine and have it attached to my user account
>> and see its details.
>>
>> None of this sounds especially out of the ordinary to me (I work in
>> the financial industry), and I can certainly see where some developers
>> would do it exactly this way (assuming my mythical checksum thing were
>> real- I've never seen that though). I doubt too many people would say
>> it's a "wrong" approach (we could of course argue architecture, that's
>> always the case, but nothing stands out as patently "wrong" to me
>> about this design).
>>
>>> Yea there are plenty. I remember someone on the list here a while
>>> back came up with this pretty cool scheme to mask dispatch action
>>> method names. It was pretty cool even though I never needed it. If I
>>> remember correctly, it acted sort of like the token mechanism - a
>>> random string was created before going to a page and that string was
>>> decoded on the server side into finding the proper dispatch method to
>>> call. To me it's just much simpler to make sure you validate the user
>>> logged in really can do what they are attempting to do somewhere on
>>> the server side or with filters.
>>
>> I think the problem here though is that this is a fairly subtle thing,
>> and something that could possibly be exploited in such a way that the
>> request looks legitimate, and in fact is, as far as the filters and
>> security checks go, but are not in terms of the logic of the application.
>>
>> I don't want to blow this out of proportion... I think the way most
>> people architect their solutions, this probably wouldn't be a problem
>> for most people.
>>
>> But, even if we all agreed that it wasn't a security problem per se,
>> it just logically does not make sense as far as I can see... if Struts
>> is going to populate the form, I can't think of a good reason it
>> wouldn't call validate() too, like any other request, just because the
>> action was canceled. Like I said, maybe someone can come up with a
>> reasonable explanation for that behavior, but I can't see it :)
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
> For additional commands, e-mail: user-help@struts.apache.org
>
>
>
>
--
Frank W. Zammetti
Founder and Chief Software Architect
Omnytex Technologies
http://www.omnytex.com
AIM: fzammetti
Yahoo: fzammetti
MSN: fzammetti@hotmail.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Rick Reumann <ri...@gmail.com>.
All of this just adds *ONE MORE REASON* to my list of *NEVER EVER* use
validate="true". I always call validation manually from my Action class
and the sooner people get into a habit of this the way better off they
will be. If you validate or call the form's validate method manually
from your Action class all these problems go away including the problem
we get posted here on the list at least a couple times a week concerning
list items going out of scope when validation fails. Validating manually
also solves the problem Paul addresses as form validation ALWAYS takes
place.
I think the bug is sort of in validate='true' :) I think that should
just be dropped and when people need to validate they just call it from
their Action class.
http://www.learntechnology.net/validate-manually.do
The code is this simple to add:
ActionErrors errors = form.validate( mapping, request );
if ( errors != null && !errors.isEmpty ) {
saveErrors(request, errors);
setUp(request); //preps request for lists and stuff
return (mapping.findForward(UIconstants.VALIDATION_FAILURE));
}
You could even add that to a utility method so it's just one line of code.
Now you have either one line of code in your Action or one line of code
in your ActionMapping, yet the first scenario solves all these problems.
(side note, I don't even really like using the validator so I don't even
call form.validate() but call a validation method in my action class
that does my validation, but I know most like to use the validator so
I'm showing how easy it is to call manually).
Frank W. Zammetti wrote:
> Rick Reumann wrote:
>> Maybe I'm missing how the above would happen. How would passing in the
>> canceled parameter end up getting them access to a table? Oh wait,
>> maybe this is with regular Actions with just an execute? It's been so
>> long since I used a non Dispatch Action I'm not aware of the behavior.
>> So I have an Action called "GetMeATable" with an execute method that
>> returns a table from the db, if I pass in through a get parameter the
>> canceled param it will still execute that action?
>
> Right, ordinary Action we're talking about. All Struts does is put a
> request attribute in place to indicate the Action is being canceled. It
> then goes on to populate the form, but *NOT* call validate() on it. So,
> the problem arises if your Action doesn't check for the canceled
> attribute and validate() is meant to stop a given request (as in my
> scenario where it was checking for one of the allowed table names).
>
> As I Wrote this, I see that Paul responded and said it's a problem with
> DispatchActions too, and I think he's right. In fact, his response sums
> up my own pretty well :)
>
>> Even if the above is true, I'm not so sure that's a big deal as far as
>> Struts goes in regard to it being a 'security problem with Struts'.
>> You can always find a ton of security loop holes if you don't truly
>> check that the logged in user has access to some backend procedure.
>> For example if you have an update dispatch method or an "UpdateAction"
>> you could easily type in the url
>> someUrl?dispath=update&id=42&whatever=something etc and have it go
>> through and be processed.
>
> Well, let me put it this way... if I worked for Microsoft, to me at
> least, this would be a non-critical bug. Patch it on the regular second
> Tuesday of the month cycle :) But I *do* think it is a bug (a logic bug
> in this case) and *is* a security hole, even if arguably a small one.
> Your absolutely right though, there are far more severe security holes
> that a developer can open up without help from Struts :) I don't think
> that should make anyone less interested in plugging this one though.
>
>> Bottom line is if security is an issue, no one should ever rely on the
>> validate method. That's just silly. I'm sure the docs even state that
>> the purpose of the validate method is to just validate data entry
>> (required fields, no Strings in number fields, etc.).
>
> But, how someone defines "data entry" can come into play... for
> instance, in the scenario I described, selecting a table, from a select
> say, would count as data entry, wouldn't it? Try this... let's say
> we're talking about a *REALLY* bad developer and for some reason he has
> a *TEXT* field for entering the table name, and then goes on to check if
> it is an allowed value in validate(). Stupid design, in the extreme!
> But an improper use of validate()? Probably not (debatable at least).
>
> Let me try and come up with a better example...
>
> Let's say my bank's web site allows me to have a primary user account,
> and I can link to this account all my checking accounts and savings
> account to see them all in one "dashboard" display. I have to fill out
> a form to have an account added. The form has account number on it.
>
> Let's say one of the validations is to be sure the checksum of the
> entered account number matches one that would only be valid for me. It's
> a relatively simple math check, so it's done in validate() because
> really we're checking that the entered value is "valid".
>
> Now, the Action behind it calls some DAO to add the account for me. But,
> it reasonably assumes that the data it gets in the form is valid. So, if
> validate() is not executed, I could conceivably enter an account number
> that isn't mine and have it attached to my user account and see its
> details.
>
> None of this sounds especially out of the ordinary to me (I work in the
> financial industry), and I can certainly see where some developers would
> do it exactly this way (assuming my mythical checksum thing were real-
> I've never seen that though). I doubt too many people would say it's a
> "wrong" approach (we could of course argue architecture, that's always
> the case, but nothing stands out as patently "wrong" to me about this
> design).
>
>> Yea there are plenty. I remember someone on the list here a while back
>> came up with this pretty cool scheme to mask dispatch action method
>> names. It was pretty cool even though I never needed it. If I remember
>> correctly, it acted sort of like the token mechanism - a random string
>> was created before going to a page and that string was decoded on the
>> server side into finding the proper dispatch method to call. To me
>> it's just much simpler to make sure you validate the user logged in
>> really can do what they are attempting to do somewhere on the server
>> side or with filters.
>
> I think the problem here though is that this is a fairly subtle thing,
> and something that could possibly be exploited in such a way that the
> request looks legitimate, and in fact is, as far as the filters and
> security checks go, but are not in terms of the logic of the application.
>
> I don't want to blow this out of proportion... I think the way most
> people architect their solutions, this probably wouldn't be a problem
> for most people.
>
> But, even if we all agreed that it wasn't a security problem per se, it
> just logically does not make sense as far as I can see... if Struts is
> going to populate the form, I can't think of a good reason it wouldn't
> call validate() too, like any other request, just because the action was
> canceled. Like I said, maybe someone can come up with a reasonable
> explanation for that behavior, but I can't see it :)
>
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
>>(Some of?) the DispactAction variants dispatch to a special method and aren't subject to the
consequences listed above, but most action implementations don't.
Rick, let me correct something here:
The DispatchAction variants are also subject to the problem with validate="true" WHEN there is no
implementation for cancelled(). The cancelled() is called but a return value of "null" indicates
it is "not implemented" (bad design) and then proceeds on to the intended method.
Since 99% of my MappingDispatchAction classes don't have any semantic necessity for canceling, I
don't implement the method to return a forward, and thus the problem roars its ugly head.
Laurie, I would like to see this fixed in 1.2; it is easier for me to drop in a patch than upgrade
a whole library. Not many managers can buy total upgrades, but they can buy incremental patches.
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
Rick,
It's a security risk because you're allowing in non-validated data. You could pass in good data,
bad data, malicious data, etc. You could pass in a string that's a million characters to your
database, perhaps characters that will appear in SQL, wrong ranges of numbers, constantly causing
exception handling, etc. Those kind of things I am very interested in preventing.
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Rick Reumann <ri...@gmail.com>.
Paul Benedict wrote:
> I don't do any business validation with the Validator; I just make sure I get proper data formats
> so that everything is in proper format when going into the service layers. I want XYZ to be
> integers and ABC to be strings.
Then where is the big 'security' risk? Worst case scenario in a 'bad
format' regard is the user will probably get some nasty error page back
when the user tried to execute the action through a URL since the data
wouldn't be in the correct format. Why do you care if this user that is
mangling the url gets an error page? He/she was doing a no-no in the
first place so they deserve the error page.
--
Rick
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
Rick,
I don't do any business validation with the Validator; I just make sure I get proper data formats
so that everything is in proper format when going into the service layers. I want XYZ to be
integers and ABC to be strings.
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
Rick Reumann wrote:
> The approach isn't invalid as far as basic validation, but for security
> I'd say it was not the best way to handle it. Let me ask you and Paul
> this... how many times do you see Struts application's built where if
> you are "Role A" you see X Y Z links but if you are "Role B" you see A B
> C links. Maybe if you are a super Admin you can see X Y Z. Now maybe
> link Y link calls /myAdminAction?someparam=1234 which is supposed to be
> an admin only action. How many developers assume that "Well only admins
> are going to see this page with that link so I don't have to check in
> that Action if they have the correct role." I'm guessing a lot do, and
> depending on the app (intranet vs external), I might opt to let it
> slide. But the reality is there are plenty of times you can type in a
> URL (if you remember the URL from some day when you were an Admin) and
> you might be able to perform admin functions all from typing in URls.
> You'd probably say, that isn't a secure system. Well the same thing I
> see happening in this validate situation. If actual processing of the
> URL without going through validate is that much of a security risk, then
> I think you'd want to push that security check further back into the
> system. Like I mentioned earlier, maybe down the line someone is going
> to implement your front end in Flash or an Applet. Wouldn't you feel
> safer knowing that back end components aren't going to let anything
> really bad happen?
I don't disagree with any of that :) And yet...
Once thing I remember from years ago when I was a bit of a hacker (not
an especially good one, but still!) is that when you leave *any* hole,
the good hackers will find a way to exploit it. It's almost as
inevitable as taxes in this day and age. I agree, with all the
discussion we've all had on this, I don't view it as a "critical" flaw
either. But "non-critical" flaws have a nasty habit of becoming
critical ones when a good hacker takes a lool :)
> Well, again, this is the kind of stuff I would never do in the validate
> method. You might like to do it there, but going way back to when I was
> first using Struts, it was always advised to NOT do this kind of
> calculation in the validate method. Would you do credit card validation
> there? I wouldn't. (Think of the mess that could happen in just this
> case if your Action assumed it passed credit card validation in order to
> ship a product and you bypassed the validate method by figuring out how
> to add the canceled param ot the URL).
I think it's a very fine line between what is "basic" input validation
and what is business rules. I think your right, this is probably a case
where doing it in validate() wouldn't be the right answer because it
probably is a business rule... but it is at least debatable. Your
right, I wouldn't do credit card validation in validate() either, but
what about a simple checksum to see if the credit card is even in a
proper form? For instance, there is a formula to determine whether an
CC # is a Visa, Master Card, Discover, etc. Is this really a business
rule? You may argue it is, and that's not unreasonable, but I could
argue it's just a simple input validation, and I think that's just as valid.
I think unfortunately we've gone a little bit off-topic though, so we
should probably both stop :)
> Because I don't think that's the place for this type of secure
> validation. I've always taken the advice that the form's validate method
> should ONLY check for proper structure of form entry elements - nothing
> more.
Even though I said we should stop :) ... this is exactly my point from
above. The determination of what is a check for "proper structure" and
what is something more isn't clear-cut.
> All the big wigs back when I was first learning Struts said don't
> perform business-type checks in the validate method and that's what I
> think you are attempting to. To me that type of security check should
> ALWAYS be done deeper in the system and it's up to the backend/midtier
> guys to make sure that always gets checked. You obviously disagree on
> setting up the architecture that way which is fine. (Heck I could be way
> off, I'm just a code monkey - nuttin' more:).
I don't disagree... but I don't think there is really a right and wrong
answer. If you ask my *opinion*, yes, any non-trivial validation should
be done at a lower level, absolutely. How you define non-trivial is the
question :)
I'm a code monkey too by the way, I just happen to be one that straddles
the coder/architect fence a whole lot. My job entails a pretty equal
dose of both these days.
> You are correct, I probably shouldn't have brought up the manual
> validate thing, since in this context it's not that relevant so I
> apologize for that.
No need to apologize, not to me anyway :) It's not completely
unrelated... I'm just not sure it really helps solve the problem at
hand, not if we're trying to come up with a solution that will work for
everyone that is.
> Hope you and Pual aren't taking this as I don't see your concern with
> the issue you brought up. I do see your point. I wasn't aware myself of
> the behavior brought up so thanks for bringing to my attention.
Not at all, I think this is valuable input into the discussion. Like I
said though, we *are* moving away from the core topic though, so we both
want to be cognizant of that.
> Frank, do you have an Ajax validation tag for me to try out:) ?
No, but that's not a bad idea :) Then again, with what AjaxTags offers
now, it should be pretty trivial to tie in to Commons Validator and get
what you want. That might make for a really nice cookbook recipe :)
Frank
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Rick Reumann <ri...@gmail.com>.
Frank W. Zammetti wrote:
>
> You may be 100% right, but it's not especially relevant to this... I'm
> sure you wouldn't say the solution is for everyone who is using
> automatic validation now to rewrite their apps, are you? :)
No you are right there. I know people like the automatic validation
stuff:) and I do see the problem Paul brought up and I do see how it
could affect dispatch actions also if someone typed in the URL with
canceled. But I don't see this as a 'big' security concern was my only
point (More in a bit on that)...
> I think your getting hung up on the design of the thought experiment
> rather than the underlying concept it's trying to demonstrate... yes,
> there are almost certainly better ways to implement such a function, but
> that doesn't mean what I outlined is an invalid approach...
The approach isn't invalid as far as basic validation, but for security
I'd say it was not the best way to handle it. Let me ask you and Paul
this... how many times do you see Struts application's built where if
you are "Role A" you see X Y Z links but if you are "Role B" you see A B
C links. Maybe if you are a super Admin you can see X Y Z. Now maybe
link Y link calls /myAdminAction?someparam=1234 which is supposed to be
an admin only action. How many developers assume that "Well only admins
are going to see this page with that link so I don't have to check in
that Action if they have the correct role." I'm guessing a lot do, and
depending on the app (intranet vs external), I might opt to let it
slide. But the reality is there are plenty of times you can type in a
URL (if you remember the URL from some day when you were an Admin) and
you might be able to perform admin functions all from typing in URls.
You'd probably say, that isn't a secure system. Well the same thing I
see happening in this validate situation. If actual processing of the
URL without going through validate is that much of a security risk, then
I think you'd want to push that security check further back into the
system. Like I mentioned earlier, maybe down the line someone is going
to implement your front end in Flash or an Applet. Wouldn't you feel
safer knowing that back end components aren't going to let anything
really bad happen?
>
> I can tell you from experience, I once wrote an app where you entered an
> account number and we had a validation to tell if it was valid based on
> nothing else but the account number. It's 100% an input validation...
> the user entered it, we need to know if it's valid, and we can determine
> that just by running it through some calculations.
Well, again, this is the kind of stuff I would never do in the validate
method. You might like to do it there, but going way back to when I was
first using Struts, it was always advised to NOT do this kind of
calculation in the validate method. Would you do credit card validation
there? I wouldn't. (Think of the mess that could happen in just this
case if your Action assumed it passed credit card validation in order to
ship a product and you bypassed the validate method by figuring out how
to add the canceled param ot the URL).
> Now, what if there was some component to that validation that included
> the identity of the user, like in the theoretical app? Why would it
> suddenly not be a good idea to do that as part of input validation?
> Since I get request in validate(), I can get the info I need from
> session as you suggest, and therefore I can do everything I need to in
> validate(). Why would I incur the overhead of executing the Action,
> executing whatever business helper I have, maybe more, when I can do it
> all *before* any of that is hit?
Because I don't think that's the place for this type of secure
validation. I've always taken the advice that the form's validate method
should ONLY check for proper structure of form entry elements - nothing
more. All the big wigs back when I was first learning Struts said don't
perform business-type checks in the validate method and that's what I
think you are attempting to. To me that type of security check should
ALWAYS be done deeper in the system and it's up to the backend/midtier
guys to make sure that always gets checked. You obviously disagree on
setting up the architecture that way which is fine. (Heck I could be way
off, I'm just a code monkey - nuttin' more:).
> Again, I don't want to start debating whether your approach in terms of
> manually calling validate() is better or not because it doesn't
> ultimately matter in this context... many people don't do that, and
> expect that the framework will do it (which isn't completely
> unreasonable), and that's when this comes into play.
You are correct, I probably shouldn't have brought up the manual
validate thing, since in this context it's not that relevant so I
apologize for that. I'm only now harping on the idea that the issue
shouldn't be that big of a security concern since I don't think the
Struts designers ever envisioned doing hard core security checks in the
validate method. Think of how scary it would be if by accident someone
made a small change from validate="true" to validate="false" and the
release went out and you were doing major security checks in your
validate method.
>
> You always run into questions about where the different tiers begin and
> end though. Besides, should an app throw an exception if I enter an
> invalid account number? If it does, you have one very unhappy customer!
> It should handle it gracefully, relatively speaking at least, and a big
> part of the point of any framework is to facilitate that sort of thing.
> If I have to do it all myself anyway, why am I using Struts in the first
> place? :)
You can handle the exception very gracefully. To the user it will look
exactly like a typical form validation error. You can either user a
declarative exception for that mapping or handle it directly in your
Action call. Point is it, it will still be just as graceful to the user.
(The only time it's not 'perfectly' graceful is if say you have a bunch
of text inputs and that account number. Maybe you are using the
automatic validation to check that a number field was entered correctly.
What could happen is the user submits the form once and has to fix some
formatting errors, then submit again only to get another error about the
account number. To me, that's only a small price to pay. If it becomes
super serious you can always add the extra check (which just be some
utility method) in the validate method, but yes, then you are doing the
check twice - but remember this is an application where security is a
concern so to me that's not a big deal - what's it going to cost you an
extra few milliseconds for the extra check on the backend to make ensure
security?)
> I wonder if your assessment might be a bit skewed by the approach you
> use, which does seem to insulate you from this?
Well sure I do think my approach would insulate you a bit better, but
even using my approach (validate manually, etc), I still wouldn't be
putting the security checks in the validate method (at least not 'just'
there).
>> I agree though that behavior should be looked into, but I'm not sure
>> of a good answer. All of these problems stem from Struts' lame-o
>> validation procedure - the weakest part of Struts imo (all the other
>> newer frameworks handle this much better). Bottom line is not setting
>> validate='true' is the best thing a user can do:)
>
> ...but it won't help a user who needs to remediate an existing app :)
I think the answer to existing apps is to just push the security up a
layer. Yea I know the tiers get blended a bit, but in a typical app my
structure might look like...
AcionForm --- Action
Action makes calls to a "Service" class
The Service class has NO ties to Struts jars
So it can be reused and called by any front end
Service ---> calls Daos for CRUD
So I'd simply do a check in the Service class..
//FooBarService
public void updateFooBar( User user, FooBar fooBar) {
boolean ok = SomeOtherClass.canFooBarDoThisUpdate(user, fooBar);
if ( ok ) { dao.update( foobar ); }
else {
throw Exception that will be caught by Front end (Action in case
of Struts)
}
The benefit to this approach again is maybe you have a JSF lover like
Dakota Jack wanting to port your app over to JSF. Well as a
backend/midtier guy you can be sure that at least the security contracts
are met from your side of things. Sure fooBar above might have an
invalid date if the front end guys didn't check for that, but at least
it won't be a security violation.
Hope you and Pual aren't taking this as I don't see your concern with
the issue you brought up. I do see your point. I wasn't aware myself of
the behavior brought up so thanks for bringing to my attention.
Frank, do you have an Ajax validation tag for me to try out:) ?
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
Rick Reumann wrote:
> 2) Use a DispatchAction. I really don't get why some of you are against
> them:) Do you make separate "DaoForCreate" and "DaoForUpdate" objects?
> My guess is you have one Dao that handles CRUD for a particular type of
> concern. To me it's just annoying having separate actions for related
> events, and it makes it more difficult when you want to do things
> related to these events. To each his/her own I guess:)
But, I believe Paul said this issue affects DispatchAction too... am I
right Paul? Perhaps you could explain that part again? I'd rather not
explain incorrectly if this is true.
> 3) Call the validation framework manually. I can give several reasons
> why this is a good idea and saves the developer a lot of headaches, but
> I've been over it before (happy to readdress if someone wants me to:).
> It's not much more code to write and actually simplifies things (very
> easy to see what's going on by simply looking at your Action class).
You may be 100% right, but it's not especially relevant to this... I'm
sure you wouldn't say the solution is for everyone who is using
automatic validation now to rewrite their apps, are you? :)
> Anyway, that's my advice for anyone having trouble with Cancel issues.
Oops, I guess you are :) I kind of assume you mean this with the
qualification "going forward". Many people use automatic validation and
frankly love it, I doubt they would be in a rush to redo their existing
apps.
>> Let's say my bank's web site allows me to have a primary user account,
>> and I can link to this account all my checking accounts and savings
>> account to see them all in one "dashboard" display. I have to fill
>> out a form to have an account added. The form has account number on it.
>>
>> Let's say one of the validations is to be sure the checksum of the
>> entered account number matches one that would only be valid for me.
>> It's a relatively simple math check, so it's done in validate()
>> because really we're checking that the entered value is "valid".
>>
>> Now, the Action behind it calls some DAO to add the account for me.
>> But, it reasonably assumes that the data it gets in the form is valid.
>> So, if validate() is not executed, I could conceivably enter an
>> account number that isn't mine and have it attached to my user account
>> and see its details.
>
> See I think that's a poor design. Why would you rely on a Struts
> validate method to handle that? For a financial application wouldn't you
> always be checking the account number vs the supposed user in Session
> scope somewhere in a non-Struts layer?
I think your getting hung up on the design of the thought experiment
rather than the underlying concept it's trying to demonstrate... yes,
there are almost certainly better ways to implement such a function, but
that doesn't mean what I outlined is an invalid approach...
I can tell you from experience, I once wrote an app where you entered an
account number and we had a validation to tell if it was valid based on
nothing else but the account number. It's 100% an input validation...
the user entered it, we need to know if it's valid, and we can determine
that just by running it through some calculations.
Now, what if there was some component to that validation that included
the identity of the user, like in the theoretical app? Why would it
suddenly not be a good idea to do that as part of input validation?
Since I get request in validate(), I can get the info I need from
session as you suggest, and therefore I can do everything I need to in
validate(). Why would I incur the overhead of executing the Action,
executing whatever business helper I have, maybe more, when I can do it
all *before* any of that is hit?
Again, I don't want to start debating whether your approach in terms of
manually calling validate() is better or not because it doesn't
ultimately matter in this context... many people don't do that, and
expect that the framework will do it (which isn't completely
unreasonable), and that's when this comes into play.
> The whole idea of component stuff
> is if later on you coded the front end in swing or flash that you would
> still have the security stuff ensured on the backend. I don't think
> letting a framework handle this type of security check is a good idea.
That's a pretty good argument :)
> I'd say it's wrong from the standpoint that the backend should be
> checking this and throwing an exception. This whole thread is a perfect
> example why:) - don't assume the front-end is making things secure.
> Maybe later on you needed a flash front end they have other security
> concerns, at least if you code the backend part to make it's security
> check things are much better. (Granted someone might find a way to spoof
> a logged in user, but that's not what we are talking about here).
You always run into questions about where the different tiers begin and
end though. Besides, should an app throw an exception if I enter an
invalid account number? If it does, you have one very unhappy customer!
It should handle it gracefully, relatively speaking at least, and a
big part of the point of any framework is to facilitate that sort of
thing. If I have to do it all myself anyway, why am I using Struts in
the first place? :)
> Agreed that it probably should be handled better. My only argument is
> that I think it's level of a 'security' risk is pretty low, unless you
> are coding for security in your validate method, which I don't think is
> a good idea.
I wonder if your assessment might be a bit skewed by the approach you
use, which does seem to insulate you from this?
>> But, even if we all agreed that it wasn't a security problem per se,
>> it just logically does not make sense as far as I can see... if Struts
>> is going to populate the form, I can't think of a good reason it
>> wouldn't call validate() too, like any other request, just because the
>> action was canceled.
>
> Well the problem is the user should be able to cancel and leave the form
> without having to fill out everything. You certainly don't want it
> validated simply to 'cancel' and leave the form. Why would you want to
> require the user to fully completely a form that he wants to cancel out of?
I agree, I don't think I had really thought that scenario out as well as
I could have when I originally wrote that paragraph.
> I agree though that behavior should be looked into, but I'm not sure of
> a good answer. All of these problems stem from Struts' lame-o validation
> procedure - the weakest part of Struts imo (all the other newer
> frameworks handle this much better). Bottom line is not setting
> validate='true' is the best thing a user can do:)
...but it won't help a user who needs to remediate an existing app :)
--
Frank W. Zammetti
Founder and Chief Software Architect
Omnytex Technologies
http://www.omnytex.com
AIM: fzammetti
Yahoo: fzammetti
MSN: fzammetti@hotmail.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Rick Reumann <ri...@gmail.com>.
Not going to rehash the issues including the issues in the new thread
started by Michael and the summary brought up by Laurie in that thread
(and yes Franks this thread is nested, somehow michael's message started
with a new messageId which breaks into a new top level thread for those
clients that base a thread on messageID - which they should:)
Lastly, I never run into these problems because
1) I never liked the cancel button and its behavior. It's 'too behind
the scenes' for me in the whole setting of an isCanceled param and all
that jazz. To me it always made since to handle cancel with a true
submit of the form and set the dispatch param to cancel or if using a
Michael/McGrady/or Mapipng type of dispatch some other key. Then you
simply handle it like any other Action. It's clean and easy to
understand and you never have to worry about these under-the-cover things.
2) Use a DispatchAction. I really don't get why some of you are against
them:) Do you make separate "DaoForCreate" and "DaoForUpdate" objects?
My guess is you have one Dao that handles CRUD for a particular type of
concern. To me it's just annoying having separate actions for related
events, and it makes it more difficult when you want to do things
related to these events. To each his/her own I guess:)
3) Call the validation framework manually. I can give several reasons
why this is a good idea and saves the developer a lot of headaches, but
I've been over it before (happy to readdress if someone wants me to:).
It's not much more code to write and actually simplifies things (very
easy to see what's going on by simply looking at your Action class).
Anyway, that's my advice for anyone having trouble with Cancel issues.
I do now see though the issue that Paul brings up and that Frank
clarified and Laurie clarified further in the summary, although I still
don't see it as "high" security risk since I would never rely on the
validate method to handle something that could be that much of a risk. A
bit more on that below in reference to Frank's example below...
Frank W. Zammetti wrote:
> Let's say my bank's web site allows me to have a primary user account,
> and I can link to this account all my checking accounts and savings
> account to see them all in one "dashboard" display. I have to fill out
> a form to have an account added. The form has account number on it.
>
> Let's say one of the validations is to be sure the checksum of the
> entered account number matches one that would only be valid for me. It's
> a relatively simple math check, so it's done in validate() because
> really we're checking that the entered value is "valid".
>
> Now, the Action behind it calls some DAO to add the account for me. But,
> it reasonably assumes that the data it gets in the form is valid. So, if
> validate() is not executed, I could conceivably enter an account number
> that isn't mine and have it attached to my user account and see its
> details.
See I think that's a poor design. Why would you rely on a Struts
validate method to handle that? For a financial application wouldn't you
always be checking the account number vs the supposed user in Session
scope somewhere in a non-Struts layer? The whole idea of component stuff
is if later on you coded the front end in swing or flash that you would
still have the security stuff ensured on the backend. I don't think
letting a framework handle this type of security check is a good idea.
> I doubt too many people would say it's a
> "wrong" approach (we could of course argue architecture, that's always
> the case, but nothing stands out as patently "wrong" to me about this
> design).
I'd say it's wrong from the standpoint that the backend should be
checking this and throwing an exception. This whole thread is a perfect
example why:) - don't assume the front-end is making things secure.
Maybe later on you needed a flash front end they have other security
concerns, at least if you code the backend part to make it's security
check things are much better. (Granted someone might find a way to spoof
a logged in user, but that's not what we are talking about here).
> I think the problem here though is that this is a fairly subtle thing,
> and something that could possibly be exploited in such a way that the
> request looks legitimate, and in fact is, as far as the filters and
> security checks go, but are not in terms of the logic of the application.
Agreed that it probably should be handled better. My only argument is
that I think it's level of a 'security' risk is pretty low, unless you
are coding for security in your validate method, which I don't think is
a good idea.
> I don't want to blow this out of proportion... I think the way most
> people architect their solutions, this probably wouldn't be a problem
> for most people.
Agreed.
> But, even if we all agreed that it wasn't a security problem per se, it
> just logically does not make sense as far as I can see... if Struts is
> going to populate the form, I can't think of a good reason it wouldn't
> call validate() too, like any other request, just because the action was
> canceled.
Well the problem is the user should be able to cancel and leave the form
without having to fill out everything. You certainly don't want it
validated simply to 'cancel' and leave the form. Why would you want to
require the user to fully completely a form that he wants to cancel out of?
I agree though that behavior should be looked into, but I'm not sure of
a good answer. All of these problems stem from Struts' lame-o validation
procedure - the weakest part of Struts imo (all the other newer
frameworks handle this much better). Bottom line is not setting
validate='true' is the best thing a user can do:)
--
Rick
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
>>I can't think of a good reason it wouldn't call validate() too, like any other request, just
because the action was canceled. Like I said, maybe someone can come up with a reasonable
explanation for that behavior, but I can't see it :)
There is a legitimate case: when an form can be cancelled, you do want to skip client-side and
server-side validation. That's just fine because in these case you do want to call the cancelled()
method from DispatchAction, dump out any state you collected, and go to the appropriate forward.
I want to make sure we don't lose focus here. The problem is not that cancelled doesn't validate,
but that ANY action can have its validation bypassed. This is not semantically correct for some
actions, especially GET actions whose sole job is to retrieve data without any state except for
the request parameters they receive. Usually people cancel form POST input, but not GET input
because those could be links in emails, bookmarks, etc. Explain the semantics of canceling a
bookmark or a link from an email? It's pretty difficult to think of a general case.
So, as I've said before, I do believe there needs to be a switch that determines when by-passing
validation IS A VALID USE CASE. Having that feature always be on is logically wrong and creates a
world where many stateless requests which use form validation to defend against bad user input is
suddenly defeated.
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
Rick Reumann wrote:
> Maybe I'm missing how the above would happen. How would passing in the
> canceled parameter end up getting them access to a table? Oh wait, maybe
> this is with regular Actions with just an execute? It's been so long
> since I used a non Dispatch Action I'm not aware of the behavior. So I
> have an Action called "GetMeATable" with an execute method that returns
> a table from the db, if I pass in through a get parameter the canceled
> param it will still execute that action?
Right, ordinary Action we're talking about. All Struts does is put a
request attribute in place to indicate the Action is being canceled. It
then goes on to populate the form, but *NOT* call validate() on it. So,
the problem arises if your Action doesn't check for the canceled
attribute and validate() is meant to stop a given request (as in my
scenario where it was checking for one of the allowed table names).
As I Wrote this, I see that Paul responded and said it's a problem with
DispatchActions too, and I think he's right. In fact, his response sums
up my own pretty well :)
> Even if the above is true, I'm not so sure that's a big deal as far as
> Struts goes in regard to it being a 'security problem with Struts'. You
> can always find a ton of security loop holes if you don't truly check
> that the logged in user has access to some backend procedure. For
> example if you have an update dispatch method or an "UpdateAction" you
> could easily type in the url
> someUrl?dispath=update&id=42&whatever=something etc and have it go
> through and be processed.
Well, let me put it this way... if I worked for Microsoft, to me at
least, this would be a non-critical bug. Patch it on the regular second
Tuesday of the month cycle :) But I *do* think it is a bug (a logic bug
in this case) and *is* a security hole, even if arguably a small one.
Your absolutely right though, there are far more severe security holes
that a developer can open up without help from Struts :) I don't think
that should make anyone less interested in plugging this one though.
> Bottom line is if security is an issue, no one should ever rely on the
> validate method. That's just silly. I'm sure the docs even state that
> the purpose of the validate method is to just validate data entry
> (required fields, no Strings in number fields, etc.).
But, how someone defines "data entry" can come into play... for
instance, in the scenario I described, selecting a table, from a select
say, would count as data entry, wouldn't it? Try this... let's say
we're talking about a *REALLY* bad developer and for some reason he has
a *TEXT* field for entering the table name, and then goes on to check if
it is an allowed value in validate(). Stupid design, in the extreme!
But an improper use of validate()? Probably not (debatable at least).
Let me try and come up with a better example...
Let's say my bank's web site allows me to have a primary user account,
and I can link to this account all my checking accounts and savings
account to see them all in one "dashboard" display. I have to fill out
a form to have an account added. The form has account number on it.
Let's say one of the validations is to be sure the checksum of the
entered account number matches one that would only be valid for me.
It's a relatively simple math check, so it's done in validate() because
really we're checking that the entered value is "valid".
Now, the Action behind it calls some DAO to add the account for me.
But, it reasonably assumes that the data it gets in the form is valid.
So, if validate() is not executed, I could conceivably enter an account
number that isn't mine and have it attached to my user account and see
its details.
None of this sounds especially out of the ordinary to me (I work in the
financial industry), and I can certainly see where some developers would
do it exactly this way (assuming my mythical checksum thing were real-
I've never seen that though). I doubt too many people would say it's a
"wrong" approach (we could of course argue architecture, that's always
the case, but nothing stands out as patently "wrong" to me about this
design).
> Yea there are plenty. I remember someone on the list here a while back
> came up with this pretty cool scheme to mask dispatch action method
> names. It was pretty cool even though I never needed it. If I remember
> correctly, it acted sort of like the token mechanism - a random string
> was created before going to a page and that string was decoded on the
> server side into finding the proper dispatch method to call. To me it's
> just much simpler to make sure you validate the user logged in really
> can do what they are attempting to do somewhere on the server side or
> with filters.
I think the problem here though is that this is a fairly subtle thing,
and something that could possibly be exploited in such a way that the
request looks legitimate, and in fact is, as far as the filters and
security checks go, but are not in terms of the logic of the application.
I don't want to blow this out of proportion... I think the way most
people architect their solutions, this probably wouldn't be a problem
for most people.
But, even if we all agreed that it wasn't a security problem per se, it
just logically does not make sense as far as I can see... if Struts is
going to populate the form, I can't think of a good reason it wouldn't
call validate() too, like any other request, just because the action was
canceled. Like I said, maybe someone can come up with a reasonable
explanation for that behavior, but I can't see it :)
--
Frank W. Zammetti
Founder and Chief Software Architect
Omnytex Technologies
http://www.omnytex.com
AIM: fzammetti
Yahoo: fzammetti
MSN: fzammetti@hotmail.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
Rick,
I think my concern is valid. I am sorry you don't find this a "big deal" but I wonder how many
sites actually use validation to make sure they defend against bad input, only to find out I can
pass in a request paramter to simply skip their checking. I mean, that's a pretty big deal in my
eyes.
I rarely use normal Action. 99% of my actions are MappingDispatchAction and I have the same
problem. I have many actions used as GET requests which retrieve data from the database, and I
validate to make sure I only accept ranges of values. Well, now that I learned it's a no-brainer
to hack my validation, I am pretty irate -- who would have thought that server-side validation
could be so easy to defeat?
I don't even need the "cancelled" feature in most of my actions, but because Struts thinks its OK
to skip validation whenever this parameter appears in my request, I am upset and I believe
rightfully so.
It's a hack to code isCancelled() in most of my cases because it's semantically non-sensical;
there's nothing to cancel. And using a form which isn't validated is even more non-sensical
because what good is data that can be ensures to be good?
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Rick Reumann <ri...@gmail.com>.
Frank W. Zammetti wrote:
>
> Now, imagine a hacker wants to get to a completely different table in
> the database (I suppose reading from a selected table would be a better
> example, but I digress). To do so, they can pass in the canceled
> parameter and whatever table name they want, assuming they can mangle
> the URL properly and establish a session first. Since Struts will
> populate the form and fire the Action *without* calling validate() in
> this case, the hacker has the "in" they want.
Maybe I'm missing how the above would happen. How would passing in the
canceled parameter end up getting them access to a table? Oh wait, maybe
this is with regular Actions with just an execute? It's been so long
since I used a non Dispatch Action I'm not aware of the behavior. So I
have an Action called "GetMeATable" with an execute method that returns
a table from the db, if I pass in through a get parameter the canceled
param it will still execute that action?
Even if the above is true, I'm not so sure that's a big deal as far as
Struts goes in regard to it being a 'security problem with Struts'. You
can always find a ton of security loop holes if you don't truly check
that the logged in user has access to some backend procedure. For
example if you have an update dispatch method or an "UpdateAction" you
could easily type in the url
someUrl?dispath=update&id=42&whatever=something etc and have it go
through and be processed.
Bottom line is if security is an issue, no one should ever rely on the
validate method. That's just silly. I'm sure the docs even state that
the purpose of the validate method is to just validate data entry
(required fields, no Strings in number fields, etc.).
Because the validate method was skipped when canceled was called and
someone could put that into an URL and still maybe have the execute
method called (which I'm not sure that's what happens anyway), is the
least of Strut's problems:)
>
> Like I said, I'm describing a scenario that would probably not happen
> too often, and certainly not by anyone with half a brain :) But that's
> the basic mechanism in a nutshell... now imagine how many innocent and
> less obvious ways to screw up that would be exploitable there may be :)
Yea there are plenty. I remember someone on the list here a while back
came up with this pretty cool scheme to mask dispatch action method
names. It was pretty cool even though I never needed it. If I remember
correctly, it acted sort of like the token mechanism - a random string
was created before going to a page and that string was decoded on the
server side into finding the proper dispatch method to call. To me it's
just much simpler to make sure you validate the user logged in really
can do what they are attempting to do somewhere on the server side or
with filters.
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
Rick Reumann wrote:
> Paul Benedict wrote:
>
>> The problem isn't that Struts allows the user to cancel an action, but
>> that EVERY action can be
>> cancelled.
>
> I must still be missing something.. what is the big deal here? If you
> don't code to handle the cancel nothing serious can happen, and if you
> do code for a cancel, what's the worst case scenario? - Since you are
> doing a "cancel" you would just typically be forwarded or redirected
> back to some input page or some other harmless page.
I think I can answer this... Actually, I think I outlined a scenario in
a previous message, but it's worth repeating to get the point across...
or if I happen to be wrong we can figure that out too :)
Imagine you have a page where a user can select a table in a database
from a choice of three that they want to save some entries to. Ignore
for the moment that this would, even as an admin application, probably
be viewed as a bad design in most cases. Forget that for now :)
If you were coding with *any* kind of security in mind, you would
validate that the selected table is one of the three acceptable ones.
You may well do this on the server via the typical validate() method of
the form.
Now, imagine a hacker wants to get to a completely different table in
the database (I suppose reading from a selected table would be a better
example, but I digress). To do so, they can pass in the canceled
parameter and whatever table name they want, assuming they can mangle
the URL properly and establish a session first. Since Struts will
populate the form and fire the Action *without* calling validate() in
this case, the hacker has the "in" they want.
Like I said, I'm describing a scenario that would probably not happen
too often, and certainly not by anyone with half a brain :) But that's
the basic mechanism in a nutshell... now imagine how many innocent and
less obvious ways to screw up that would be exploitable there may be :)
> (As a side note, I never use the cancel button anyway. I always use
> asubmit or regular button with some javascript setting a 'canceled'
> dispatch param for my cancel buttons. I had a reason at some point why I
> liked doing that, although, I forgot now what the exact reason was:)
I frankly don't recall *ever* using the canceled functionality either,
but I wouldn't assume I'm typical... I rarely am :)
--
Frank W. Zammetti
Founder and Chief Software Architect
Omnytex Technologies
http://www.omnytex.com
AIM: fzammetti
Yahoo: fzammetti
MSN: fzammetti@hotmail.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
That is of course the other angle on this... maybe you can argue that
this is only a substantial security problem in certain contrived
situations... ok, fine :) ... but, as Paul points out, it makes it
exceedingly easy to break an application. That's a Denial Of Service
risk, so it still falls in the category of security problem. DOS
attacks are generally considered a lower-tier security issue, but a
security issue none the less :)
Frank
Paul Benedict wrote:
> Rick you said:
>
>>> I must still be missing something.. what is the big deal here? If you don't code to handle the
> cancel nothing serious can happen, and if you do code for a cancel, what's the worst case
> scenario?
>
> Yes, you are missing something :) If you don't code the cancel, then your action gets called with
> non-validated data. How well do your actions work with a form that's filled with garbage??? :)
> Maybe you have some numeric fields; let's try passing in some characters to see how the app
> reacts.
>
> Paul
>
> __________________________________________________
> Do You Yahoo!?
> Tired of spam? Yahoo! Mail has the best spam protection around
> http://mail.yahoo.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
> For additional commands, e-mail: user-help@struts.apache.org
>
>
>
>
--
Frank W. Zammetti
Founder and Chief Software Architect
Omnytex Technologies
http://www.omnytex.com
AIM: fzammetti
Yahoo: fzammetti
MSN: fzammetti@hotmail.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
Rick you said:
>> I must still be missing something.. what is the big deal here? If you don't code to handle the
cancel nothing serious can happen, and if you do code for a cancel, what's the worst case
scenario?
Yes, you are missing something :) If you don't code the cancel, then your action gets called with
non-validated data. How well do your actions work with a form that's filled with garbage??? :)
Maybe you have some numeric fields; let's try passing in some characters to see how the app
reacts.
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Rick Reumann <ri...@gmail.com>.
Paul Benedict wrote:
> The problem isn't that Struts allows the user to cancel an action, but that EVERY action can be
> cancelled.
I must still be missing something.. what is the big deal here? If you
don't code to handle the cancel nothing serious can happen, and if you
do code for a cancel, what's the worst case scenario? - Since you are
doing a "cancel" you would just typically be forwarded or redirected
back to some input page or some other harmless page.
> This problem is heavily felt by GET requests because URLs are easy to mangle... and parameters can
> be added ad-hoc. I can take any action I use for a GET, add the CANCEL parameter to it, and then
> bypass all the validation I worked very hard to code :-)
What kind of form validation are you doing on a cancel request that is
such an issue?
> To me, this is a security concern and I
> think should be given a fix.
Can you elaborate a bit more on a real life example that could
realistically cause a security threat?
(As a side note, I never use the cancel button anyway. I always use
asubmit or regular button with some javascript setting a 'canceled'
dispatch param for my cancel buttons. I had a reason at some point why I
liked doing that, although, I forgot now what the exact reason was:)
--
Rick
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Michael Jouravlev <jm...@gmail.com>.
On 1/21/06, Paul Benedict <pa...@yahoo.com> wrote:
> Try it yourself!! Just add "?org.apache.struts.taglib.html.CANCEL=true" to any GET URL and your
> execute() method will magically be called as if you didn't have any validation added to your code.
Calling ActionForm.validate() explicitly from an action saves a lot of
trouble, and for me it is just simpler.
On 1/21/06, Frank W. Zammetti <fz...@omnytex.com> wrote:
> Now, imagine a hacker wants to get to a completely different table in
> the database (I suppose reading from a selected table would be a better
> example, but I digress). To do so, they can pass in the canceled
> parameter and whatever table name they want, assuming they can mangle
> the URL properly and establish a session first. Since Struts will
> populate the form and fire the Action *without* calling validate() in
> this case, the hacker has the "in" they want.
I say it again, do not rely on automatic validation ;-)
On 1/22/06, Rick Reumann <ri...@gmail.com> wrote:
> All of this just adds *ONE MORE REASON* to my list of *NEVER EVER* use
> validate="true". I always call validation manually from my Action class
> and the sooner people get into a habit of this the way better off they
> will be.
+1. Also, I would prefer populate() to be explicitly called from
action as well. I want framework to provide services for me and to do
heavy lifting, but I want to control the steering wheel and pedals.
Automatic behavior on request/response is a flaw, not a benefit. I can
write 3-5 lines of code to perform (or not) appropriate framework
function. I hate when stuff is done for me.
A framework should encapsulate chunks of job, but it must be me to
decide whether to perform these jobs or not.
Michael.
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
>> Arguably, by default you would want to say that all Actions are either cancelable or not,
rather than having to set something on all mappings. Just my superficial thinking about it though
:)
Agreed. That's why I recommend a <controller> property default with overrides at the <action>
level.
Better solutions are welcomed!
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
Frank,
I am getting your emails delivered twice to the list today. Are you click happy today? :) haha
Maybe mine are getting delivered twice; please tell me if so.
As for the bug/issue, I mainly use MappingDispatchAction and so there's no reason to specifically
code for isCancelled().... BUT I was looking at DispatchAction and that always calls the
cancelled() method, but if it returns "null" it continues to the expected method. I kind of find
that strange. "null" is an appropriate return value in Struts which means that you handled the
request directly. To check for null here and then continue going is also a bug, in my opinion.
That's another issue to debate.
I am sure someone may want this forwarded to the struts-dev boards, but I hate signing up to
boards just for sending one email....
Now for proposing fixing. I really believe it's an error to populate the form, bypass validation,
and invoke the destination in *all cases* for cancelling. What makes this a difficult problem to
solve is that it's been around for many eons of Struts :)
Ted's solution is good but do we really want to that? I know more than a few programs that require
cleaning up from a valid cancel before needing to forward off. Personally, for the time being
anyhow, I still prefer setting a controller level property that assumes nothing may be canceled
unless the action says so. Why? I have more actions that have no use for canceling than ones that
do.
Did you notice validation is skipped in processValidate() when the request attribute is set from
processPopulate()? We should try to determine what logic is appropriate to remove the cancel
attribute, if need be.
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
Paul Benedict wrote:
>> Hmm, maybe I don't understand the problem... as I understand it, cancel essentially has no
> effect on anything in Struts unless you manually check for it and act accordingly, correct? Or
> are you saying that everything happens *except* validation?
>
> Correct. Cancel has no effect unless you religiously check for it and do something with it..
> unless you use a DispatchAction in which the "cancelled" method is always invoked.
Right, at least I got that part of it :)
>> That's the part I guess I'm not clear on... how is validation bypassed? Is that *ALL* that is
> bypassed, or is form population bypassed too?
>
> This is the code from RequestProcessor.processPopulate:
-snip-
> According to the code the form is always populated. If the cancel button is in the request, the
> cancel request attribute is added; this is then used in processValidate() to skip validation.
Yes, I see what you mean. Unless someone can point out a reason you
would want it to function that way, then I would agree it's a bug. I
for one (or two I suppose!) would not have expected it to work that way,
and it doesn't seem right to me either.
> Try it yourself!! Just add "?org.apache.struts.taglib.html.CANCEL=true" to any GET URL and your
> execute() method will magically be called as if you didn't have any validation added to your code.
I'll take your word for it :) The code you showed certainly does seem
to bear this out. I did look at processValidate(), and I certainly
concur with your analysis.
Going back to Ted's pointer, if one had that in all their Actions
religiously, than arguably this wouldn't really be a problem. The fact
that the form gets populated without being validated afterwards, while I
would say isn't optimal, probably wouldn't matter much.
But if you forgot that somewhere, then one could say this is a security
hole... for instance, if someone had a given parameter that was used to
select a database table for updating, and part of the form validation
was to ensure it was only one of three acceptable values, then this
clearly could be a security problem (although I'd say the bad
application design was a bigger issue!)
Now that I agree (and I think understand!), the question is how best to
address it... I would think the solution should look something like
this, in RequestProcessor.java:
// Set the cancellation request attribute if appropriate
if ((request.getParameter(Constants.CANCEL_PROPERTY) != null) ||
(request.getParameter(Constants.CANCEL_PROPERTY_X) != null)) {
request.setAttribute(Globals.CANCEL_KEY, Boolean.TRUE);
ActionForward f = mapping.findForward("cancel");
if (f != null) {
doForward(f.getPath(), request, response);
}
}
RequestUtils.populate(form, mapping.getPrefix(), mapping.getSuffix(),
request);
Note the form is only populated AFTER we've determined whether we should
cancel or not. This would also maintain the current behavior, although
arguably that is not desirable. What else would you want to do if there
was no cancel forward defined though? I'm not sure.
Your solution of defining what mappings are cancelable or not would also
do the trick of course, but that seems to me like a bit more work than
it really should be. Arguably, by default you would want to say that
all Actions are either cancelable or not, rather than having to set
something on all mappings. Just my superficial thinking about it though :)
> Paul
Frank
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
> Hmm, maybe I don't understand the problem... as I understand it, cancel essentially has no
effect on anything in Struts unless you manually check for it and act accordingly, correct? Or
are you saying that everything happens *except* validation?
Correct. Cancel has no effect unless you religiously check for it and do something with it..
unless you use a DispatchAction in which the "cancelled" method is always invoked.
> That's the part I guess I'm not clear on... how is validation bypassed? Is that *ALL* that is
bypassed, or is form population bypassed too?
This is the code from RequestProcessor.processPopulate:
========================================================
RequestUtils.populate(form, mapping.getPrefix(), mapping.getSuffix(), request);
// Set the cancellation request attribute if appropriate
if ((request.getParameter(Constants.CANCEL_PROPERTY) != null) ||
(request.getParameter(Constants.CANCEL_PROPERTY_X) != null)) {
request.setAttribute(Globals.CANCEL_KEY, Boolean.TRUE);
}
========================================================
According to the code the form is always populated. If the cancel button is in the request, the
cancel request attribute is added; this is then used in processValidate() to skip validation.
Try it yourself!! Just add "?org.apache.struts.taglib.html.CANCEL=true" to any GET URL and your
execute() method will magically be called as if you didn't have any validation added to your code.
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
Paul Benedict wrote:
> The problem isn't that Struts allows the user to cancel an action, but that EVERY action can be
> cancelled.
Understood...
> This problem is heavily felt by GET requests because URLs are easy to mangle... and parameters can
> be added ad-hoc. I can take any action I use for a GET, add the CANCEL parameter to it, and then
> bypass all the validation I worked very hard to code :-)
Hmm, maybe I don't understand the problem... as I understand it, cancel
essentially has no effect on anything in Struts unless you manually
check for it and act accordingly, correct? Or are you saying that
everything happens *except* validation?
In either case, wouldn't using Ted's suggestion in all your Actions
alleviate the problem? If so, I agree that's not an optimal situation,
so having the framework do that *before* form population or validation
would seem right to me.
Am I misunderstanding?
> I think this is an obvious bug: cancellations make sense during form driven input (or across many
> forms like a wizard), but cancelling with a link? Sure it could be useful but not in any
> applications I am dealing with.
I would agree, it does seem a pointless capability. But I don't think
it would be smart to remove something because it doesn't seem useful, so
it needs to be made to work in a way that is reasonable I think is all.
> It's not so much a matter of finding a "cancel" forward. The problem is actions should control if
> they CAN be cancelled so their validation isn't bypassed. To me, this is a security concern and I
> think should be given a fix.
That's the part I guess I'm not clear on... how is validation bypassed?
Is that *ALL* that is bypassed, or is form population bypassed too?
> Paul
Frank
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
Frank,
Good response. Let me add some to it:
The problem isn't that Struts allows the user to cancel an action, but that EVERY action can be
cancelled.
This problem is heavily felt by GET requests because URLs are easy to mangle... and parameters can
be added ad-hoc. I can take any action I use for a GET, add the CANCEL parameter to it, and then
bypass all the validation I worked very hard to code :-)
I think this is an obvious bug: cancellations make sense during form driven input (or across many
forms like a wizard), but cancelling with a link? Sure it could be useful but not in any
applications I am dealing with.
It's not so much a matter of finding a "cancel" forward. The problem is actions should control if
they CAN be cancelled so their validation isn't bypassed. To me, this is a security concern and I
think should be given a fix.
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by "Frank W. Zammetti" <fz...@omnytex.com>.
Hmm... is it truly a security hole? That's an interesting question...
I'm not so sure I would consider it one because in general you can play
a lot of games with a Struts-based app (and really just about any
webapp, although JSF is nice in this regard in that, AFAIK, everything
is POST-based) by fiddling with URLs without even considering the cancel
ability at all.
I *would* however contend that this is something the framework could
reasonably be expected to handle. It might not be an especially big
deal that it doesn't now, but it could make for a nice enhancement none
the less.
Ted Husted points out a suggested way of dealing with this in an Action:
forward = mapping.findForward("cancel");
if ((forward!=null) && (isCancelled(request))) {
// Post token error message
ActionErrors errors = new ActionErrors();
errors.add(ActionErrors.GLOBAL_ERROR, new ActionError("error.cancel"));
saveErrors(request,errors);
return (forward);
}
(Referenced from here: http://husted.com/struts/tips/014.html)
You could add this to the RP without much trouble, or in the 1.3 world
just modify the RP chain. I suppose you would logically want to do it
before the form is populated, probably as early on as possible. It
would just be a case of documenting the need to specify a forward named
"cancel" for an app, either globally or per-mapping (IIRC, Struts will
always use a matching local forward before looking for a global forward
with the same name, so you have the ability to override the global).
The default if no "cancel" forward is found would be to continue the
request, which would maintain compatibility with how it works now.
Anyone else?
Frank
Paul Benedict wrote:
> I'd like to know if this is considered a security hole to other people besides me. I saved an
> email off this group back in July and finally went back to investigate it:
>
> It seems that every action in Struts is cancellable, which means for Struts actions that do not
> religiously check for isCancelled(), a hacker can bypass validation simply by passing in the
> cancel key ("org.apache.struts.action.CANCEL"). This seems entirely possible through Jakarta
> HttpClient, or just modifying the URL when possible.
>
> So, in my opinion, it doesn't seem like data from the form is every truely reliable without the
> isCancelled() check.
>
> I propose the Controller address this somehow. Maybe by using <set-property> there can be an
> attribute set at the action to allow validation to be legitimately skipped or make this
> configurable at the <controller> level.
>
> Any ideas?
>
> Paul
>
> __________________________________________________
> Do You Yahoo!?
> Tired of spam? Yahoo! Mail has the best spam protection around
> http://mail.yahoo.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
> For additional commands, e-mail: user-help@struts.apache.org
>
>
>
> .
>
--
Frank W. Zammetti
Founder and Chief Software Architect
Omnytex Technologies
http://www.omnytex.com
AIM: fzammetti
Yahoo: fzammetti
MSN: fzammetti@hotmail.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
I want to correct a statement here. What can be passed in is
"org.apache.struts.taglib.html.CANCEL" or "org.apache.struts.taglib.html.CANCEL.X" which will set
the cancelled flag.
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Validation Security Hole?
Posted by Paul Benedict <pa...@yahoo.com>.
I'd like to know if this is considered a security hole to other people besides me. I saved an
email off this group back in July and finally went back to investigate it:
It seems that every action in Struts is cancellable, which means for Struts actions that do not
religiously check for isCancelled(), a hacker can bypass validation simply by passing in the
cancel key ("org.apache.struts.action.CANCEL"). This seems entirely possible through Jakarta
HttpClient, or just modifying the URL when possible.
So, in my opinion, it doesn't seem like data from the form is every truely reliable without the
isCancelled() check.
I propose the Controller address this somehow. Maybe by using <set-property> there can be an
attribute set at the action to allow validation to be legitimately skipped or make this
configurable at the <controller> level.
Any ideas?
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: error messages doesn't appear if validate is called programatically
Posted by Ha...@alcatel.com.au.
Thank you very much Paul. It was a big time save for me.
Regards
Hakan
Paul Benedict <pa...@yahoo.com>
20/01/2006 04:07 PM
Please respond to
"Struts Users Mailing List" <us...@struts.apache.org>
To
Struts Users Mailing List <us...@struts.apache.org>
cc
Subject
Re: error messages doesn't appear if validate is called programatically
Hakan,
You need to store the error messages in the request or session after you
retrieve them:
ActionErrors errors = form.validate();
if ((errors != null) && !errors.isEmpty()) {
saveErrors(request, errors);
return mapping.getInputForward();
}
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
This email may contain privileged/confidential information. You may not copy or disclose this email to anyone without the written permission of the sender. If you have received this email in error please kindly delete this message and notify the sender. Opinions expressed in this email are those of the sender and not necessarily the opinions of the employer.
This email and any attached files should be scanned to detect viruses. No liability will be accepted by the employer for loss or damage (whether caused by negligence or not) as a result of email transmission.
Cannot create iterator for this collection
Posted by Tony Smith <qu...@yahoo.com>.
Hi, I am using struts taglib. I have a object like
public class myObject{
List myList;
List getList(){
return myList;
}
...
}
The myObject is send through aciton to the jsp. In my
jsp, I have:
...
<bean:define id="l" value="myObject.list"/>
<logic:iterate id="obj" collection="l">
...
</logic:iterate>
But it complains that " Cannot create iterator for
this collection" which means myList is not reconginzed
as a list.
What is wrong?
Thanks,
qq
--- Paul Benedict <pa...@yahoo.com> wrote:
> Hakan,
>
> You need to store the error messages in the request
> or session after you retrieve them:
>
> ActionErrors errors = form.validate();
> if ((errors != null) && !errors.isEmpty()) {
> saveErrors(request, errors);
> return mapping.getInputForward();
> }
>
> Paul
>
>
> __________________________________________________
> Do You Yahoo!?
> Tired of spam? Yahoo! Mail has the best spam
> protection around
> http://mail.yahoo.com
>
>
---------------------------------------------------------------------
> To unsubscribe, e-mail:
> user-unsubscribe@struts.apache.org
> For additional commands, e-mail:
> user-help@struts.apache.org
>
>
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org
Re: error messages doesn't appear if validate is called programatically
Posted by Paul Benedict <pa...@yahoo.com>.
Hakan,
You need to store the error messages in the request or session after you retrieve them:
ActionErrors errors = form.validate();
if ((errors != null) && !errors.isEmpty()) {
saveErrors(request, errors);
return mapping.getInputForward();
}
Paul
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@struts.apache.org
For additional commands, e-mail: user-help@struts.apache.org