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

[Peter Maydell]

On 19 August 2016 at 17:37, Greg Kurz <address@hidden> wrote:
> Peter Maydell <address@hidden> 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.
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