[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path |
Date: |
Mon, 22 Aug 2016 17:14:28 +0300 |
On Fri, Aug 19, 2016 at 06:03:29PM +0100, Peter Maydell wrote:
> 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.
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
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, (continued)
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Greg Kurz, 2016/08/18
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Peter Maydell, 2016/08/19
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Peter Maydell, 2016/08/19
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Greg Kurz, 2016/08/19
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Peter Maydell, 2016/08/19
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Greg Kurz, 2016/08/19
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Peter Maydell, 2016/08/22
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, P J P, 2016/08/22
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Michael S. Tsirkin, 2016/08/22
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Peter Maydell, 2016/08/22
- Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path,
Michael S. Tsirkin <=
Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path, Greg Kurz, 2016/08/19