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: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
