Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Aug 19, 2016 at 06:03:29PM +0100, Peter Maydell wrote:
> On 19 August 2016 at 17:37, Greg Kurz <groug@kaod.org> wrote:
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >> If (1) is true and "only single path component" is a protocol
> >> requirement then probably we should be enforcing this at a
> >> higher layer than in 9p-local.c, ie in hw/9pfs/cofs.c.
> 
> > As we discussed on IRC, the / character isn't invalid per-se. It raises
> > issues with the local backend on a linux host but does not do harm with
> > other backends.
> >
> > The proxy backend also accesses the linux filesystem but since it
> > chroots to the export path, it does not hit the path traversal issue.
> 
> The proxy backend is not actually going to do the right thing with
> a component name containing a '/' though (which would be to really
> treat it as a filename or whatever with a '/', not to mis-interpret
> it as a combined directory-and-filename. For instance opening "foo/bar"
> ought to open a file named "foo/bar", not a file bar in directory foo,
> if we're going to accept it.) It might not be a security hole, but
> it still doesn't actually support '/' in filenames.
> 
> The handle backend also assumes '/' isn't in filenames.
> 
> 'synth' might be able to handle '/' I guess, but I'd want to
> audit the code before I put any weight on that assertion.
> 
> I don't really see the point in allowing a theoretical
> /-in-names-aware backend to interact with an equally theoretical
> /-in-names-aware frontend: nobody in practice is going to
> use this. The downside of support in the middle-layer code for
> this theoretical case is that we make it harder to write correct
> backends and easy to accidentally allow security holes.

FWIW I agree.

> I'd prefer it if we made the check in the middle layer and
> explicitly said "all QEMU 9p servers insist that '/' is not a
> valid character in filenames, and backend code can assume that
> the middle layer has validated this".
> 
> thanks
> -- PMM