beehive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daryl Olander <dolan...@gmail.com>
Subject Re: Page Flow Security Risk
Date Fri, 17 Feb 2006 16:59:33 GMT
I agree we need to move to a POJO model....

I think the issue is that we expose objects like the struts config that are
developed independently of Beehive which may have setters which could open
up security holes.  It's also the case that we expose object that expose
object and underlying modification to the runtime could open up a security
hole and no one would know.  The just simply added a new feature and exposed
a setter.  This is certainly a brittle design even if we verified all of the
current paths.

I don't think opion #3 is viable because we still need to support for at
least some time the current model even if it's deprecated.

On 2/17/06, Rich Feit <richfeit@gmail.com> wrote:
>
> It's not happenstance.  When we still extended Struts Action, I had
> workarounds in there to prevent access to dangerous base class objects
> (like getServlet()).  In general I allowed public getters for
> unmodifiable objects.  If we're exposing something dangerous, then it's
> my fault -- it isn't just bad luck.
>
> > Access to the shared flow Map is luckily illegal when the expressions
> are being updated.
> I think I *did* expose a potential security hole by not returning
> Collections.unmodifiableMap() from
> FlowControllerFactory.getSharedFlowsForPath() -- this needs to be
> fixed.  Why is access to the Map illegal currently?
>
>
> I would vote for this option:
>
>     3) Verify that what's currently exposed is safe, and move to the
> POJO-pageflow model (deprecate use of the base class).
>
> Rich
>
> Daryl Olander wrote:
> > I've been looking at a possible security risk in page flows.  At the
> moment,
> > I don't think we have an actual security hole, but I think we have a
> > situation where we could create one very easy.
> >
> > The issue is that there are a number of public properties on the
> > PageFlowController class.  There are public getters that give access to
> low
> > level structures.  For example, you can get the ModuleConfig from
> Struts,
> > the ActionForm, ActionServlet, the map of shared flows, etc.  This issue
> > arises because you can submit a form that contains a hidden field that
> would
> > update these data items.
> >
> >   <netui:form action="submit">
> >     <netui:hidden dataSource="pageFlow.moduleConfig.prefix"
> > dataInput="value"/>
> >     <netui:button value="submit" />
> >   </netui:form>
> >
> > In the above code, this could modify the Struts ModuleConfig structure
> and
> > set the prefix value to "value".
> >
> > In fact, in looking around at this for a little while, I couldn't find
> > anything you can do that is destructive.  The Struts config information
> is
> > frozen, so the code above results in an IllegalStateException.  Access
> to
> > the shared flow Map is luckily illegal when the expressions are being
> > updated.
> >
> > I think that it's purely happenstance that we are not exposing a
> security
> > hole here. In fact, with a bit more playing round, we might find that we
> > really are exposing a hole.  We need to prevent page flow updates for
> these
> > base class properties.  There seems to be a number of ways we could
> solve
> > this,
> >
> > 1) We could prevent all update to PageFlow.  This is a pretty radical
> > solution because it's a backward incompatible change.
> > 2) We could create a list of properties that can't be updated.  The list
> > could be created automatically through reflection.
> >
> > Right now, I would lean toward 2, but I think we should have more
> discussion
> > of this issue.
> >
> > Thoughts?
> >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message