|
It's
fine to nest a JavaBean in an ActionForm if that bean is just a transport object
that is passed to EJBs but it probably isn't a good idea to nest your model
objects directly in the form.
If you
are going to nest a javabean in a form you don't need the getName()/setName()
methods, you can just have a getUser()/setUser() and reference the properties of
the nested bean by labeling the field in your jsp as
"user.name".
Hal
Ah, this maybe a problem in the way I've adapted
Struts. I reflect all UserForm method calls directly into the contained
User object owned by the UserForm. So for instance, I have
public class UserForm extends
ActionsForm
{
protected User
user;
...
public String
getName()
{
return
user.getName();
}
public void setName(String
name)
{
user.setName(name);
}
...
}
Now can you begin to see my original
concern? Maybe I need to separate the model from the form a little more
than what I have.
- jeff
----- Original Message -----
Sent: Monday, May 07, 2001 4:38
PM
Subject: Re: Potential Security Flaw in
Struts MVC
Either you are misunderstanding Struts, or I am
misunderstanding you.
Struts will populate your UserForm for you,
prior to your UserAction being called. However, it is your responsibility
to, within UserAction, copy the values from UserForm to
User.
Bryan
Jeff Trent wrote:
00bd01c0d728$40864960$6401a8c0@PROVIDENCE">
Bryan,
This is good advice. However, I thought
the beans are populated off of the request outside of the control of my
Action class derivation. Therefore, copyProperties() doesn't
pertain.
- jeff
-----
Original Message -----
Sent:
Monday, May 07, 2001 1:14 PM
Subject:
Re: Potential Security Flaw in Struts MVC
There is a security risk here as you describe, if (and
only if) you are using a generic introspection-based function (like
Struts' PropertyUtils.copyBean) to copy the values from the UserForm
object to the User object. There are several ways to avoid this
--
1. Don't put an admin flag "setter" method in your User
class.
2. In UserAction, don't use the generic bean copy utility --
instead, manually copy the values, excluding the admin flag.
3. As a
smarter alternative to #2, don't use a generic bean copy utility --
instead, write an intelligent copy function in the User class, which
"knows" that it's copying FROM a UserForm, TO a User, and therefore,
skip the copying of the Admin flag.
Bryan
Jeff
Trent wrote:
002501c0d70b$9df009a0$6401a8c0@PROVIDENCE">
I may be wrong about this (only been
working w/ Struts for a week now). But I do see a potential
security flaw in struts that I would like to hear from others
regarding.
Consider a simple set of struts classes
that represent a user in a system. You would probably have classes
that look something like this:
User
(the model
representing the user)
UserForm (an enrollment form for
a new user)
UserAction (Saves the UserForm
information to db, etc)
The User class would have accessors and
modifiers like getFirstName(), setFirstName(),
getAdministrativeUserFlag(), setAdministrativeUserFlag(), etc.
The basic implementation of the UserForm is to take the UI form data,
introspect the beans, and call the correct modifier of the UserForm
bean based on the fields contained within the UI
submission/form. A developer of course would not expose the
"Administrative User Flag" option on the UI for enrollment (that would
be found possibly in some other administrative-level module).
However, if someone is familiar with the db schema and the naming
convention the developer used, that user could subvert the application
by writing his own version of the UI which contains an "Administrative
User Flag" field (or any other field for that matter) and the basic
form processing in Struts will kindly honor the request and set the
"Administrative Flag" on the user. Unless, of course, the
developer makes special provisions to prevent this behavior.
However, its not entirely obvious to the struts user (in my opinion)
that this is even a concern. Am I making sense
here?
- jeff
|
