From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41242) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bbpzp-0002Px-8F for qemu-devel@nongnu.org; Mon, 22 Aug 2016 10:14:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bbpzk-0000Fq-62 for qemu-devel@nongnu.org; Mon, 22 Aug 2016 10:14:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46184) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bbpzk-0000FK-0Y for qemu-devel@nongnu.org; Mon, 22 Aug 2016 10:14:32 -0400 Date: Mon, 22 Aug 2016 17:14:28 +0300 From: "Michael S. Tsirkin" Message-ID: <20160822170700-mutt-send-email-mst@kernel.org> References: <1470892391-4917-1-git-send-email-ppandit@redhat.com> <20160819183717.4de8c99d@bahia.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH] 9pfs: add check for relative path List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Greg Kurz , P J P , Qemu Developers , Prasad J Pandit , Felix Wilhelm , "Aneesh Kumar K.V" On Fri, Aug 19, 2016 at 06:03:29PM +0100, Peter Maydell wrote: > On 19 August 2016 at 17:37, Greg Kurz wrote: > > Peter Maydell 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