jsr369-experts@servlet-spec.java.net

[jsr369-experts] Re: [servlet-spec users] Re: Part write clarification?

From: Mark Thomas <markt_at_apache.org>
Date: Wed, 15 Mar 2017 12:48:21 +0000

On 15/03/17 04:24, Greg Wilkins wrote:
>
> Stuart,
>
> So do you accept an absolute path that is any where in the file system,
> or does it have to be within the tree specified by the multipart config?
>
> For example, if the configuration has the location as C:\\foo\bar ,
> then do you accept a call to write with a value of something like
> C:\\somewhere\else ? or only something like C:\\foo\bar\more\specific.txt ?
>
> IMHO If absolute filenames can write anywhere on the file system, then
> that's a bit of a security problem and it makes it a little pointless to
> check relative paths for .. etc.

If this was a user provided path I'd agree. But it isn't. The
application is free to write to any part of the file system directly via
the Java API so why not here?

If the path does originate with the user then it is an application
responsibility to sanitise it, not a container responsibility.

If the application is not trusted to write to anywhere in the file
system, that is what a security manager and/or OS file permissions are for.

Finally, it think it is worth repeating that the primary reason Tomcat
implemented it this way was for ease of migration from Commons
FileUpload to the new Servlet API. The equivalent method in FileUpload
allowed absolute paths so Tomcat did to.

Mark

> On 9 March 2017 at 07:20, Stuart Douglas <sdouglas_at_redhat.com
> <mailto:sdouglas_at_redhat.com>> wrote:
>
> For what it is worth we also implement the Tomcat behavior.
>
> If we do make this behavior official we should probably put a warning
> on the javadoc about it being up to the application to make sure the
> path that is passed in is safe.
>
> For relative paths I think we should probably throw an exception if
> the resulting path is outside the base path (e.g. files like
> ../../../somefile.txt).
>
> Stuart
>
> On Wed, Mar 8, 2017 at 6:09 PM, Greg Wilkins <gregw_at_webtide.com
> <mailto:gregw_at_webtide.com>> wrote:
> >
> > All,
> >
> > Janb has asked me to seek clarification on the Part.write(String)
> method.
> >
> > The spec for @MultipartConfiguration.location says:
> >
> >> The location attribute of the
> javax.servlet.annotation.MultipartConfig and
> >> the <location> element of the <multipart-config> is interpreted as an
> >> absolute path and defaults to the value of the
> >> javax.servlet.context.tempdir. If a relative path is specified,
> it will be
> >> relative to the tempdir location. The test for absolute path vs
> relative
> >> path MUST be done via java.io.File.isAbsolute.
> >
> >
> > The javadoc for Part.write(String fileName) says:
> >
> >> fileName - the name of the file to which the stream will be
> written. The
> >> file is created relative to the location as specified in the
> MultipartConfig
> >
> >
> > It is not clear whether fileName should be interpreted as a Path
> or not, nor
> > what should be the result if it specifies an absolute location, or
> contains
> > special path elements such as "." or "..".
> >
> > Should an absolute filename throw an IAE or should it just be
> interpreted
> > relative to the config location? What if the filename is
> something like
> > "C:\\foo\bar"?
> >
> >
> > Tomcat have changed the javadoc on their version of the API to say:
> >
> >> @param fileName The location into which the uploaded part should be
> >> stored. Relative locations are relative to {_at_link
> >> javax.servlet.MultipartConfigElement#getLocation()}
> >
> >
> > For which we have raised an issue:
> > https://bz.apache.org/bugzilla/show_bug.cgi?id=60802
> <https://bz.apache.org/bugzilla/show_bug.cgi?id=60802>
> > and Spring Framework at least is expecting the tomcat behaviour as
> reported
> > in this Jetty bug:
> https://github.com/eclipse/jetty.project/issues/1337
> <https://github.com/eclipse/jetty.project/issues/1337>
> >
> >
> > I don't mind the Tomcat version (modulo some security concerns about
> > allowing the container to try to write to any old path), but would
> prefer if
> > the official version could clarify how absolute paths should be
> handled.
> >
> > cheers
> >
> >
> >
> >
> > --
> > Greg Wilkins <gregw_at_webtide.com <mailto:gregw_at_webtide.com>> CTO
> http://webtide.com
>
>
>
>
> --
> Greg Wilkins <gregw_at_webtide.com <mailto:gregw_at_webtide.com>> CTO
> http://webtide.com