From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42165) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bamnT-00054L-5T for qemu-devel@nongnu.org; Fri, 19 Aug 2016 12:37:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bamnP-0003I8-U8 for qemu-devel@nongnu.org; Fri, 19 Aug 2016 12:37:31 -0400 Received: from 20.mo1.mail-out.ovh.net ([188.165.45.168]:41281) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bamnP-0003Ho-NR for qemu-devel@nongnu.org; Fri, 19 Aug 2016 12:37:27 -0400 Received: from player726.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo1.mail-out.ovh.net (Postfix) with ESMTP id 75934FF9BF6 for ; Fri, 19 Aug 2016 18:37:26 +0200 (CEST) Date: Fri, 19 Aug 2016 18:37:17 +0200 From: Greg Kurz Message-ID: <20160819183717.4de8c99d@bahia.lan> In-Reply-To: References: <1470892391-4917-1-git-send-email-ppandit@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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: P J P , Qemu Developers , Prasad J Pandit , "Michael S. Tsirkin" , Felix Wilhelm , "Aneesh Kumar K.V" On Fri, 19 Aug 2016 16:14:48 +0100 Peter Maydell wrote: > On 19 August 2016 at 15:55, Peter Maydell 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