struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Craig R. McClanahan" <craig...@apache.org>
Subject Re: Security issues with Struts
Date Tue, 02 Jul 2002 23:16:40 GMT


On Tue, 2 Jul 2002, Marcel Kruzel wrote:

> Date: Tue, 02 Jul 2002 10:14:05 +0200
> From: Marcel Kruzel <marcel.kruzel@vsb.cz>
> Reply-To: Struts Developers List <struts-dev@jakarta.apache.org>
> To: Struts Developers List <struts-dev@jakarta.apache.org>
> Subject: Re: Security issues with Struts
>
> >>I believe Struts have provide a basic mechanism to resolve
> >>the problems associated with the multiple submits. But when
> >>considering this in a security issue context, we might have rooms
> >>to enhance the mechanism - here is my little thoughts:
> >>
> >>1) Since the transaction token is visible by client browser, users
> >>could use the same token to cheat Struts by a client program.
> >>
> >>
> >
> > A snooper could make the initial submit for a particular form (if they
> > could get it in faster), but couldn't do much else.  However, the security
> > implications of exposing the session id are *much* more serious than
> > exposing the transaction token.  If data exposure is an issue, use an SSL
> > connection.
> >
> >
> >>2) Even though request scoped form beans plus a transaction
> >>token make Struts thread safe, but I observed developers still make
> >>mistakes by assigning attributes from the request scoped form beans
> >>to their own version of session scoped java objects, which is
> >>possibly participating in a transaction from the previous request.
> >>Any non-trivial application have session scoped objects, so this
> >>is very likely to happen in general.
> >>
> >>
> >
> > I'm not sure what a framework can do about this in any general way.  Do
> > you have some specific suggestions?  I fear that just "single thread all
> > access to the session" is going to cause more problems than it would
> > solve.
> >
> >
> >>3) The session scoped form bean plus a transaction token may not
> >>work correctly all the time: Before the first request reach the
> >>statement isTokenValid(request, true), the second request might
> >>already corrupt the session scoped form bean by populating bad
> >>data. Therefore it corrupts the transaction state of the first request,
> >>if the form bean is participating in that transaction.
> >>
>
> Thanx for so many replies!
>
> Precisely! the transactionToken does help if You
> want to detect multiple THE SAME submits. But this is not
> our issue here. If the second submit contains different values,
> the session scoped form bean will get populated
> before I am able to detect that there is a second
> submit! The handling (perform()) of the first
> submit will continue with wrong parameters!
>
> The only solution here is to:
> PREVENT from populating of the session scoped
> form bean if there is still a work going on with the SAME form bean.
> PERIOD.
> (thus a locking mechanism, anyway).
> The simplest one is utilization of the Java sync
> mechanism on session object, however,
> I understand, that there might be some problems doing so
> (if done in each form of the application, the user
> might lock himself for a while!
> (and clicking and clicking ....:-))))))
> Here commes a brilliant idea! Let's synchronize
> on the form bean itself! That should work!
> Of course, you still have to write
> your own RequestProcessor as Jing Zhou suggests, so You
> can DO something (sync or check transaction context)
> before the form gets populated.
>

You can accomplish this for yourself by adapting the request processing
lifecycle Struts supports.  In Struts 1.1, you just provide your own
RequsetProcessor subclass, and override the relevant methods.  (In Struts
1.0, these methods are embedded in ActionServlet, but are named the same
and do the same things).  It's not clear to me that a general purpose
solution can be defined that would (a) meet nearly everyone's needs; (b)
not get in the way of some people; and (c) not harm performance in the
usual case where you really don't submit more than one request at the same
time.  Patches are welcome to show me how this ought to be done.

>
> Of course, request scoped beans solve the problems,
> but it is more difficult to code,
> (all thouse hidden fields in multipage forms,
> you can't use the redirects, so the history
> in client browser doesn't contain the submits, etc.)
>

My favorite science fiction author (Robert Heinlein) had a relevant
acronym for this kind of situation:  TANSTAAFL.

  "There ain't no such thing as a free lunch."

:-)

Craig


>
>
>
> >>
> >
> > Session scope form beans are already susceptible to problems of
> > simultaneous access, even without taking transaction tokens into
> > consideration.  That is why request scope form beans are generally
> > recommended.
> >
> >
> >>Here are what I am doing in our own project and hopefully it
> >>will help Struts:
> >>
> >>We created a client transaction context (relative to EJB server)
> >>which manage both JDBC connection based transaction and JTA
> >>UserTransaction based transaction. In our own version of the
> >>RequestProcessor, we detect the transaction context before
> >>populating the form bean. If an active client transaction context
> >>found in the HttpSession, there are two choices, let the request
> >>wait or throw an exception. I prefer the second one, which has a
> >>consistent semantic with SessionBean defined by EJB 2.0 spec:
> >>Container will throw exception if there is a concurrent access
> >>to the business method of the same stateful SessionBean instance.
> >>In other words, Struts could guard the container in early stage and
> >>solve the potential problems 1-3, and many bad comments on
> >>the session scoped form beans will be gone :-)
> >>
> >>Is my thoughts valid? Any further thoughts?
> >>
> >>
> >
> > Many (maybe most?) Struts based apps don't use EJBs.  Is there a way we
> > could define the semantics of this sort of things in terms of APIs that
> > Struts could support, that don't rely on them?
>
>
> I think, that setting your own token in
> session (WORKING,NOT_WORKING) might
> have the same results. Then in RequestProcessor,
> You are going to check whether there is  a WORKING
> token in the session, if it is not then set one.
> (critical section here of course)
> You just have to make sure, that you will
> remove it (put NOT_WORKING) when finished!
>
>
>
> >
> >
> >>Jing
> >>
> >>
> >
> > Craig
> >
> >
> >
> >>----- Original Message -----
> >>From: "Craig R. McClanahan" <craigmcc@apache.org>
> >>To: "Struts Developers List" <struts-dev@jakarta.apache.org>
> >>Sent: Monday, July 01, 2002 11:54 AM
> >>Subject: Re: Security issues with Struts
> >>
> >>
> >>
> >>>There are at least a couple of issues that I can pull out of your problem
> >>>description -- here's my thoughts on them.
> >>>
> >>>REUSE OF FORM BEANS
> >>>
> >>>You only have to worry about reuse of the same physical form bean on
> >>>multiple requests if you are using session scope to save them in.  If you
> >>>are using request scope (recommended for performance anyway, because it
> >>>reduces the memory load on the server), a new bean gets created populated,
> >>>and validated for each request.  That still leaves the problem of
> >>>detecting when the user submits the "same" form twice ...
> >>>
> >>>DETECTING MULTIPLE SUBMITS
> >>>
> >>>This is a general issue for all web applications, not specific to Struts.
> >>>However, Struts provides a solution based on the concept of a "transaction
> >>>token" that can be used to easily detect when the user tries something
> >>>like this.  It works as follows:
> >>>
> >>>* In your Action that sets up the input form, call the method:
> >>>
> >>>    saveToken(request)
> >>>
> >>>  somewhere along the way, before forwarding to the actual page.
> >>>  This records a serial number in the user's session.
> >>>
> >>>* When the <html:form> tag actually renders the form, it sees the
> >>>  serial number and generates a hidden field to include it's value
> >>>  along with the rest of your fields.
> >>>
> >>>* In the Action that receives the form (after validation), you can
> >>>  call the method:
> >>>
> >>>    isTokenValid(request, true);
> >>>
> >>>  to check the token included in the request (if any), and clear the
> >>>  value saved in the session (which is normally what you want to do).
> >>>  This method will return false if there is no token at all in the
> >>>  form, or if the token doesn't match the saved value.  The fact that
> >>>  you are resetting the saved value means that it will also return
> >>>  false if the user submits a form, presses stop, and submits it again.
> >>>
> >>>I believe that your session synchronization approach isn't necessary to
> >>>deal with the particular problem you've described (although it might be
> >>>useful for other application-specific reasons).  I hesitate to add
> >>>something like this to the framework itself, though -- managing
> >>>simultaneous requests to the same session has such a wide range of
> >>>possible impacts that I don't think a single simple solution is going to
> >>>cover all of the use cases.  And using request scope for form beans covers
> >>>quite a large subset of the possible impacts all by itself (at the cost,
> >>>of course, of having to include hidden variables on your forms for
> >>>multi-page form beans).
> >>>
> >>>Craig
> >>>
> >>>
> >>>
> >>>On Mon, 1 Jul 2002, Marcel Kruzel wrote:
> >>>
> >>>
> >>>>Date: Mon, 01 Jul 2002 08:46:11 +0200
> >>>>From: Marcel Kruzel <marcel.kruzel@vsb.cz>
> >>>>Reply-To: Struts Developers List <struts-dev@jakarta.apache.org>
> >>>>To: struts-dev@jakarta.apache.org
> >>>>Subject: Security issues with Struts
> >>>>
> >>>>Hello Struts developers,
> >>>>
> >>>>We are now developing an internet banking
> >>>>application with Stuts of course.
> >>>>Of course, we are highly concerned
> >>>>in possible security holes in the framework
> >>>>(or in the application using the framework).
> >>>>I believe, there is one, that, when properly
> >>>>used, can cause some troubles for developers!
> >>>>
> >>>>Here it is:
> >>>>
> >>>>Imagine a scenario, where user submits a form,
> >>>>the Struts automatically populate the
> >>>>form bean and then
> >>>>the validation of the parameters takes place.
> >>>>After that, if OK, the perform method is called.
> >>>>Here, I already know, that the
> >>>>params were ok, so I write the
> >>>>transaction to database. The problem here
> >>>>is the perform method. When precisely at the
> >>>>moment of perform method call user decides
> >>>>to submit the form once again (with
> >>>>different values of course), the form bean
> >>>>is again populated, and possibly wrong
> >>>>(not validated) data might be writen to database
> >>>>(by the first thread, that is not aware
> >>>>of the second submit).
> >>>>I am afraid, there is not a possibility to
> >>>>synchronize acccess to the form,
> >>>>since the population of form bean is automatic.
> >>>>
> >>>>I know, that the solution to the problem here is
> >>>>in the perform method
> >>>>to make copy of the parameters and then validate
> >>>>them again. Or, I can
> >>>>make validation only in the perform method,
> >>>>but first I have to remove the
> >>>>form bean from session, then validate
> >>>>and then write to database, and then possibly
> >>>>return that form bean to the session.
> >>>>
> >>>>Sorry for so long description,
> >>>>but I tried to make myself as clear as possible.
> >>>>
> >>>>My preferred solution to the problem would be:
> >>>>In the struts-config.xml put an attribute
> >>>>to action, describing, that handling
> >>>>such an action requires session synchronization.
> >>>>Thus everything, starting from form population
> >>>>and ending with "return mapping.findForward(...)"
> >>>>would be synchronized on a session object.
> >>>>(I do this synchronization on a session
> >>>>anyway - in each perform method).
> >>>>
> >>>>So, what do You think?
> >>>>
> >>>>Thanx to all contributors
> >>>>for such a great framework,
> >>>>and enjoy the summer.
> >>>>
> >>>>Marcel Kruzel
> >>>>Czech Republic
> >>>>
> >>>>
> >>>>
> >>>>--
> >>>>To unsubscribe, e-mail:
> >>>>
> >><mailto:struts-dev-unsubscribe@jakarta.apache.org>
> >>
> >>>>For additional commands, e-mail:
> >>>>
> >><mailto:struts-dev-help@jakarta.apache.org>
> >>
> >>>>
> >>>
> >>>--
> >>>To unsubscribe, e-mail:
> >>>
> >><mailto:struts-dev-unsubscribe@jakarta.apache.org>
> >>
> >>>For additional commands, e-mail:
> >>>
> >><mailto:struts-dev-help@jakarta.apache.org>
> >>
> >>>
> >>
> >>--
> >>To unsubscribe, e-mail:   <mailto:struts-dev-unsubscribe@jakarta.apache.org>
> >>For additional commands, e-mail: <mailto:struts-dev-help@jakarta.apache.org>
> >>
> >>
> >
> >
> > --
> > To unsubscribe, e-mail:   <mailto:struts-dev-unsubscribe@jakarta.apache.org>
> > For additional commands, e-mail: <mailto:struts-dev-help@jakarta.apache.org>
> >
> >
> >
>
>
>
>
>
> --
> To unsubscribe, e-mail:   <mailto:struts-dev-unsubscribe@jakarta.apache.org>
> For additional commands, e-mail: <mailto:struts-dev-help@jakarta.apache.org>
>
>


--
To unsubscribe, e-mail:   <mailto:struts-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:struts-dev-help@jakarta.apache.org>


Mime
View raw message