You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Jeanne Waldman <je...@oracle.com> on 2007/09/12 03:19:38 UTC
[TRINIDAD] comments welcome for issue 703
Hi there,
I'm about to fix issue:
https://issues.apache.org/jira/browse/TRINIDAD-703
snippet from issue:
We register our image resource loader with a fairly loose pattern:
register("(/.*\\.(css|jpg|gif|png|jpeg|svg|js))",
new CoreClassLoaderResourceLoader(parent));
In theory could someone get at an image on the class path outside of our own
images by crafting a funky URL along the lines of
"../../../../oracle/someotherpackage/foo.gif"?
Yes.
ClassLoaderResourceLoader should prevent access outside of the "rootPackage".
I mention how I am fixing it (disallowing ".." in the path), so please comment if you'd like.
Thanks,
Jeanne
Re: [TRINIDAD] comments welcome for issue 703
Posted by Simon Lessard <si...@gmail.com>.
Well, I no longer use those with Trinidad so I'm not sure about it, but with
ADF Faces the resulting CSS still contains '..'
~ Simon
On 9/12/07, Adam Winer <aw...@gmail.com> wrote:
>
> Do such URLs in skins actually result in incoming URLs containing
> "..", or does it get resolved out when we generate the .css?
>
> -- Adam
>
>
> On 9/12/07, Jeanne Waldman <je...@oracle.com> wrote:
> >
> > Hi Simon,
> >
> > Very good point. I have seen skin's have ".." in the background-image
> path.
> > I forgot about that.
> >
> > The reason for fixing this issue is that we feel it is a security issue
> if
> > the use of the ".." in the path is such that the path goes outside of
> the
> > 'root'.
> >
> > If I switch to the old code path and log a warning about the deprecated
> URL
> > usage, then the security issue will still exist.
> > It sounds like to fix the issue correctly, I'll have to make sure if
> the
> > path contains ".." that the path doesn't take us outside the root. e.g.,
> > foo/bar/../../zoo/../.. -> takes you outside root.
> >
> > A side note -- I found a bug in the DirectoryResourceLoader where it
> was
> > allowing paths outside the root directory, even though the comment
> > said that it wasn't. I can fix that easily, and I will log a separate
> issue
> > and fix that since it isn't controversial.
> >
> > Thanks again for your comments,
> > - Jeanne
> >
> >
> > Simon Lessard wrote:
> > Hello Jeanne,
> >
> > Personally you won't break anything with my projects, but it's only
> because
> > I fully converted the skin to use the new Trinidad URL system (well it's
> > more Trinidad URL system than new actually). However, preventing the
> '..'
> > will most likely make the passage between ADF Faces and Trinidad more
> > difficult as '..' was often needed with ADF Faces and background-image
> > within skins. Would it be possible to do your change but if you detect
> '..',
> > switch to the old code path and log a warning about a deprecated URL
> usage
> > within the skin? We could offer a grace period until one month or so
> after
> > JDeveloper 11g get in production maybe? I would use that date because
> the
> > amount of Trinidad user will most likely get a big boost from old ADF
> Faces
> > users when JDev 11 is officially released. Also, those new users will
> most
> > likely have to do the aforementioned conversion.
> >
> >
> > Regards,
> >
> > ~ Simon
> >
> >
> > On 9/11/07, Jeanne Waldman <je...@oracle.com> wrote:
> > > My main concern is
> > > a. should I simply reject any path with ".." in it as dangerous -or-
> > >
> > > b if the path contains ".." should figure out if it resolves to a path
> > > outside the root and only reject it in that case.
> > >
> > > b is safer, but requires more processing.
> > >
> > > Thanks,
> > >
> > > - Jeanne
> > >
> > >
> > > Jeanne Waldman wrote:
> > > > Hi there,
> > > > I'm about to fix issue:
> > > > https://issues.apache.org/jira/browse/TRINIDAD-703
> > > >
> > > > snippet from issue:
> > > >
> > > > We register our image resource loader with a fairly loose pattern:
> > > > register("(/.*\\.(css|jpg|gif|png|jpeg|svg|js))",
> > > > new CoreClassLoaderResourceLoader(parent));
> > > >
> > > > In theory could someone get at an image on the class path outside of
> > > > our own
> > > > images by crafting a funky URL along the lines of
> > > > "../../../../oracle/someotherpackage/foo.gif"? Yes.
> > > > ClassLoaderResourceLoader should prevent access outside of the
> > > > "rootPackage".
> > > >
> > > >
> > > > I mention how I am fixing it (disallowing ".." in the path), so
> please
> > > > comment if you'd like.
> > > >
> > > > Thanks,
> > > > Jeanne
> > > >
> > > >
> > >
> >
> >
>
Re: [TRINIDAD] comments welcome for issue 703
Posted by Adam Winer <aw...@gmail.com>.
Do such URLs in skins actually result in incoming URLs containing
"..", or does it get resolved out when we generate the .css?
-- Adam
On 9/12/07, Jeanne Waldman <je...@oracle.com> wrote:
>
> Hi Simon,
>
> Very good point. I have seen skin's have ".." in the background-image path.
> I forgot about that.
>
> The reason for fixing this issue is that we feel it is a security issue if
> the use of the ".." in the path is such that the path goes outside of the
> 'root'.
>
> If I switch to the old code path and log a warning about the deprecated URL
> usage, then the security issue will still exist.
> It sounds like to fix the issue correctly, I'll have to make sure if the
> path contains ".." that the path doesn't take us outside the root. e.g.,
> foo/bar/../../zoo/../.. -> takes you outside root.
>
> A side note -- I found a bug in the DirectoryResourceLoader where it was
> allowing paths outside the root directory, even though the comment
> said that it wasn't. I can fix that easily, and I will log a separate issue
> and fix that since it isn't controversial.
>
> Thanks again for your comments,
> - Jeanne
>
>
> Simon Lessard wrote:
> Hello Jeanne,
>
> Personally you won't break anything with my projects, but it's only because
> I fully converted the skin to use the new Trinidad URL system (well it's
> more Trinidad URL system than new actually). However, preventing the '..'
> will most likely make the passage between ADF Faces and Trinidad more
> difficult as '..' was often needed with ADF Faces and background-image
> within skins. Would it be possible to do your change but if you detect '..',
> switch to the old code path and log a warning about a deprecated URL usage
> within the skin? We could offer a grace period until one month or so after
> JDeveloper 11g get in production maybe? I would use that date because the
> amount of Trinidad user will most likely get a big boost from old ADF Faces
> users when JDev 11 is officially released. Also, those new users will most
> likely have to do the aforementioned conversion.
>
>
> Regards,
>
> ~ Simon
>
>
> On 9/11/07, Jeanne Waldman <je...@oracle.com> wrote:
> > My main concern is
> > a. should I simply reject any path with ".." in it as dangerous -or-
> >
> > b if the path contains ".." should figure out if it resolves to a path
> > outside the root and only reject it in that case.
> >
> > b is safer, but requires more processing.
> >
> > Thanks,
> >
> > - Jeanne
> >
> >
> > Jeanne Waldman wrote:
> > > Hi there,
> > > I'm about to fix issue:
> > > https://issues.apache.org/jira/browse/TRINIDAD-703
> > >
> > > snippet from issue:
> > >
> > > We register our image resource loader with a fairly loose pattern:
> > > register("(/.*\\.(css|jpg|gif|png|jpeg|svg|js))",
> > > new CoreClassLoaderResourceLoader(parent));
> > >
> > > In theory could someone get at an image on the class path outside of
> > > our own
> > > images by crafting a funky URL along the lines of
> > > "../../../../oracle/someotherpackage/foo.gif"? Yes.
> > > ClassLoaderResourceLoader should prevent access outside of the
> > > "rootPackage".
> > >
> > >
> > > I mention how I am fixing it (disallowing ".." in the path), so please
> > > comment if you'd like.
> > >
> > > Thanks,
> > > Jeanne
> > >
> > >
> >
>
>
Re: [TRINIDAD] comments welcome for issue 703
Posted by Simon Lessard <si...@gmail.com>.
Hello Jeanne,
Personally you won't break anything with my projects, but it's only because
I fully converted the skin to use the new Trinidad URL system (well it's
more Trinidad URL system than new actually). However, preventing the '..'
will most likely make the passage between ADF Faces and Trinidad more
difficult as '..' was often needed with ADF Faces and background-image
within skins. Would it be possible to do your change but if you detect '..',
switch to the old code path and log a warning about a deprecated URL usage
within the skin? We could offer a grace period until one month or so after
JDeveloper 11g get in production maybe? I would use that date because the
amount of Trinidad user will most likely get a big boost from old ADF Faces
users when JDev 11 is officially released. Also, those new users will most
likely have to do the aforementioned conversion.
Regards,
~ Simon
On 9/11/07, Jeanne Waldman <je...@oracle.com> wrote:
>
> My main concern is
> a. should I simply reject any path with ".." in it as dangerous -or-
>
> b if the path contains ".." should figure out if it resolves to a path
> outside the root and only reject it in that case.
>
> b is safer, but requires more processing.
>
> Thanks,
>
> - Jeanne
>
>
> Jeanne Waldman wrote:
> > Hi there,
> > I'm about to fix issue:
> > https://issues.apache.org/jira/browse/TRINIDAD-703
> >
> > snippet from issue:
> >
> > We register our image resource loader with a fairly loose pattern:
> > register("(/.*\\.(css|jpg|gif|png|jpeg|svg|js))",
> > new CoreClassLoaderResourceLoader(parent));
> >
> > In theory could someone get at an image on the class path outside of
> > our own
> > images by crafting a funky URL along the lines of
> > "../../../../oracle/someotherpackage/foo.gif"? Yes.
> > ClassLoaderResourceLoader should prevent access outside of the
> > "rootPackage".
> >
> >
> > I mention how I am fixing it (disallowing ".." in the path), so please
> > comment if you'd like.
> >
> > Thanks,
> > Jeanne
> >
> >
>
Re: [TRINIDAD] comments welcome for issue 703
Posted by Jeanne Waldman <je...@oracle.com>.
My main concern is
a. should I simply reject any path with ".." in it as dangerous -or-
b if the path contains ".." should figure out if it resolves to a path
outside the root and only reject it in that case.
b is safer, but requires more processing.
Thanks,
- Jeanne
Jeanne Waldman wrote:
> Hi there,
> I'm about to fix issue:
> https://issues.apache.org/jira/browse/TRINIDAD-703
>
> snippet from issue:
>
> We register our image resource loader with a fairly loose pattern:
> register("(/.*\\.(css|jpg|gif|png|jpeg|svg|js))",
> new CoreClassLoaderResourceLoader(parent));
>
> In theory could someone get at an image on the class path outside of
> our own
> images by crafting a funky URL along the lines of
> "../../../../oracle/someotherpackage/foo.gif"? Yes.
> ClassLoaderResourceLoader should prevent access outside of the
> "rootPackage".
>
>
> I mention how I am fixing it (disallowing ".." in the path), so please
> comment if you'd like.
>
> Thanks,
> Jeanne
>
>