You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@cocoon.apache.org by cz...@apache.org on 2003/11/14 14:02:09 UTC
cvs commit: cocoon-2.1/src/java/org/apache/cocoon/components/source/impl ContextSourceFactory.java
cziegeler 2003/11/14 05:02:09
Modified: . status.xml
src/java/org/apache/cocoon/components/source/impl
ContextSourceFactory.java
Log:
<action dev="CZ" type="fix" fixes-bug="24093">
Disable accessing files outside the context via the context protocol.
</action>
Revision Changes Path
1.194 +4 -4 cocoon-2.1/status.xml
Index: status.xml
===================================================================
RCS file: /home/cvs/cocoon-2.1/status.xml,v
retrieving revision 1.193
retrieving revision 1.194
diff -u -r1.193 -r1.194
--- status.xml 13 Nov 2003 12:32:47 -0000 1.193
+++ status.xml 14 Nov 2003 13:02:09 -0000 1.194
@@ -192,9 +192,9 @@
<changes>
<release version="@version@" date="@date@">
- <action dev="NN" type="fix">
- DUMMY
- </action>
+ <action dev="CZ" type="fix" fixes-bug="24093">
+ Disable accessing files outside the context via the context protocol.
+ </action>
</release>
<release version="2.1.3" date="November 13 2003">
<action dev="DC" type="fix" fixes-bug="24463" due-to-email="tek@leonid.de" due-to="Leo Leonid">
1.5 +12 -6 cocoon-2.1/src/java/org/apache/cocoon/components/source/impl/ContextSourceFactory.java
Index: ContextSourceFactory.java
===================================================================
RCS file: /home/cvs/cocoon-2.1/src/java/org/apache/cocoon/components/source/impl/ContextSourceFactory.java,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -r1.4 -r1.5
--- ContextSourceFactory.java 25 Oct 2003 18:06:20 -0000 1.4
+++ ContextSourceFactory.java 14 Nov 2003 13:02:09 -0000 1.5
@@ -69,9 +69,10 @@
import org.apache.cocoon.environment.Context;
/**
- * A factory for the context protocol using the context of the servlet api. It builds the
- * source by asking the environment context for the real URL
- * (see {@link org.apache.cocoon.environment.Context#getResource(String)}) and then resolving this real URL.
+ * A factory for the context protocol using the context of the servlet api.
+ * It builds the source by asking the environment context for the real URL
+ * (see {@link org.apache.cocoon.environment.Context#getResource(String)})
+ * and then resolving this real URL.
*
* @author <a href="mailto:cziegeler@apache.org">Carsten Ziegeler</a>
* @author <a href="http://www.apache.org/~sylvain">Sylvain Wallez</a>
@@ -142,8 +143,13 @@
}
// Remove the protocol and the first '/'
- int pos = location.indexOf(":/");
- String path = location.substring(pos+1);
+ final int pos = location.indexOf(":/");
+ final String path = location.substring(pos+1);
+
+ // fix for #24093, we don't give access to files outside the context:
+ if ( path.indexOf("../") != -1 ) {
+ throw new MalformedURLException("Invalid path ('../' is not allowed) : " + path);
+ }
URL u;
Re: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/components/source/impl
ContextSourceFactory.java
Posted by Upayavira <uv...@upaya.co.uk>.
Carsten Ziegeler wrote:
>Sylvain Wallez wrote:
>
>
>>> // Remove the protocol and the first '/'
>>> - int pos = location.indexOf(":/");
>>> - String path = location.substring(pos+1);
>>> + final int pos = location.indexOf(":/");
>>> + final String path = location.substring(pos+1);
>>> +
>>> + // fix for #24093, we don't give access to files
>>>
>>>
>>outside the context:
>>
>>
>>> + if ( path.indexOf("../") != -1 ) {
>>> + throw new MalformedURLException("Invalid path
>>>
>>>
>>('../' is not allowed) : " + path);
>>
>>
>>> + }
>>>
>>>
>>>
>>>
>>Isn't this way of checking too strict? We can have perfectly valid cases
>>where one concatenates a base "context://foo/bar/" base URI with a
>>"../baz" relative path.
>>
>>
>>
>Hmmm, who does such nice things?
>Ok, but you're right - don't we have a URL mangler somewhere that does
>this for us?
>
>Carsten
>
>
IIRC NetUtils.normalize() will remove any .. from a URL. It splits on /,
so it can handle a context: protocol, but how it deals with .. at the
beginning of a URL I can't work out immediately. If it doesn't, it
shouldn't be hard to patch it to work appropriately.
Don't know if this is relevent.
Regards, Upayavira
RE: cvs commit:cocoon-2.1/src/java/org/apache/cocoon/components/source/implContextSourceFactory.java
Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Bruno Dumon wrote:
> > >
> > Hmmm, who does such nice things?
>
> The one who does such nice things should use the method
> SourceResolver.resolve(String location, String base, Map parameters)
> instead of concatenating the two strings.
>
> > Ok, but you're right - don't we have a URL mangler somewhere that does
> > this for us?
>
> The SourceResolver does all that.
>
The problem is a little bit different, we don't absolutize the urls
ourselfes, this is done by the servlet context. So we pass a
"hallo/../../this.xml" to the context.
And we want to check before we pass this path to the context, if
a file outside the context is specified, like in the case above.
Can we use the absolutize() method of NetUtils therefore?
Carsten
RE: cvs commit:
cocoon-2.1/src/java/org/apache/cocoon/components/source/impl
ContextSourceFactory.java
Posted by Bruno Dumon <br...@outerthought.org>.
On Fri, 2003-11-14 at 15:11, Carsten Ziegeler wrote:
> Sylvain Wallez wrote:
> >
> > > // Remove the protocol and the first '/'
> > > - int pos = location.indexOf(":/");
> > > - String path = location.substring(pos+1);
> > > + final int pos = location.indexOf(":/");
> > > + final String path = location.substring(pos+1);
> > > +
> > > + // fix for #24093, we don't give access to files
> > outside the context:
> > > + if ( path.indexOf("../") != -1 ) {
> > > + throw new MalformedURLException("Invalid path
> > ('../' is not allowed) : " + path);
> > > + }
> > >
> > >
> >
> > Isn't this way of checking too strict? We can have perfectly valid cases
> > where one concatenates a base "context://foo/bar/" base URI with a
> > "../baz" relative path.
> >
> Hmmm, who does such nice things?
The one who does such nice things should use the method
SourceResolver.resolve(String location, String base, Map parameters)
instead of concatenating the two strings.
> Ok, but you're right - don't we have a URL mangler somewhere that does
> this for us?
The SourceResolver does all that.
--
Bruno Dumon http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
bruno@outerthought.org bruno@apache.org
RE: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/components/source/impl ContextSourceFactory.java
Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Sylvain Wallez wrote:
>
> > // Remove the protocol and the first '/'
> > - int pos = location.indexOf(":/");
> > - String path = location.substring(pos+1);
> > + final int pos = location.indexOf(":/");
> > + final String path = location.substring(pos+1);
> > +
> > + // fix for #24093, we don't give access to files
> outside the context:
> > + if ( path.indexOf("../") != -1 ) {
> > + throw new MalformedURLException("Invalid path
> ('../' is not allowed) : " + path);
> > + }
> >
> >
>
> Isn't this way of checking too strict? We can have perfectly valid cases
> where one concatenates a base "context://foo/bar/" base URI with a
> "../baz" relative path.
>
Hmmm, who does such nice things?
Ok, but you're right - don't we have a URL mangler somewhere that does
this for us?
Carsten
Re: cvs commit: cocoon-2.1/src/java/org/apache/cocoon/components/source/impl
ContextSourceFactory.java
Posted by Sylvain Wallez <sy...@apache.org>.
cziegeler@apache.org wrote:
>cziegeler 2003/11/14 05:02:09
>
> Modified: . status.xml
> src/java/org/apache/cocoon/components/source/impl
> ContextSourceFactory.java
> Log:
> <action dev="CZ" type="fix" fixes-bug="24093">
> Disable accessing files outside the context via the context protocol.
> </action>
>
>
<snip/>
> // Remove the protocol and the first '/'
> - int pos = location.indexOf(":/");
> - String path = location.substring(pos+1);
> + final int pos = location.indexOf(":/");
> + final String path = location.substring(pos+1);
> +
> + // fix for #24093, we don't give access to files outside the context:
> + if ( path.indexOf("../") != -1 ) {
> + throw new MalformedURLException("Invalid path ('../' is not allowed) : " + path);
> + }
>
>
Isn't this way of checking too strict? We can have perfectly valid cases
where one concatenates a base "context://foo/bar/" base URI with a
"../baz" relative path.
Sylvain
--
Sylvain Wallez Anyware Technologies
http://www.apache.org/~sylvain http://www.anyware-tech.com
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
Orixo, the opensource XML business alliance - http://www.orixo.com