On Fri, 19 Aug 2016 16:14:48 +0100 Peter Maydell <[email protected]> wrote:
> On 19 August 2016 at 15:55, Peter Maydell <[email protected]> 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
