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

[Greg Kurz <groug@kaod.org>]

On Fri, 19 Aug 2016 16:14:48 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 19 August 2016 at 15:55, Peter Maydell <peter.maydell@linaro.org> wrote:
> > Also, strstr(name, "../") is the wrong check. There are I think
> > two possibilities here:
> >
> > (1) the "name" parameter may only validly be a single pathname
> > component. In this case we should be enforcing this by treating
> > any string with a "/" in it as an error (and checking for "../"
> > is not catching all the cases that should be errors).
> >
> > (2) the "name" parameter may be a multiple-pathname-component value.
> > In this case "../" catches too many cases, because "foo../bar" is
> > a valid string which is not relative. You would need to check for
> > (contains "/../" OR starts with "../" OR ends with "/.." OR is "..").
> >
> >
> > On IRC Greg and I discussed this and Greg suggested that
> > case (1) is what we have. We should check this though.  
> 
> 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.

Aneesh suggested an alternative to doing checks on the names would be
to forbid access outside the export path but I can't think of a simple
way to do that.

> thanks
> -- PMM

Cheers.

--
Greg