Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 5 Aug 2022 09:38:29 +0200
From:   Lukas Czerner <lczerner@redhat.com>
To:     Daniel Ng <danielng@google.com>
Cc:     linux-ext4@vger.kernel.org,
        Sarthak Kukreti <sarthakkukreti@google.com>
Subject: Re: [BUG] fsck unable to resolve filenames that include '='
Message-ID: <20220805073829.dcwhe3dizlk5hz5n@fedora>
References: <CANFuW3eGgyeWba-2GjDtdhYvX2fV7-dcrHn-4O8QAeHDERAbqw@mail.gmail.com>
 <20220803143113.frmayykhlhvcqkxg@fedora>
 <CANFuW3d5e=0qs+8mQmy+Gd5zJK9NcnJETZ1PAgTgS8E33qG5ng@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CANFuW3d5e=0qs+8mQmy+Gd5zJK9NcnJETZ1PAgTgS8E33qG5ng@mail.gmail.com>
Precedence: bulk

Hi Daniel,

On Fri, Aug 05, 2022 at 03:32:14PM +1000, Daniel Ng wrote:
> Hey Lukas,
> 
> that aligns with our observations; I also ran into this when running
> tune2fs on the file too.
> 
> I'm not very familiar with blkid_get_devname(), but that sounds sensible to me.
> 
> Alternatively, my colleague (sarthak@) was suggesting that we could
> add a stat() check for the file in the conditional that checks if
> there is a '=' character in the
> token - which would work for us at least. I suppose it just depends on
> what we should prioritize finding. It might make more sense to
> prioritize finding
> a regular file, before trying to parse the expression. In your
> example, the 'LABEL=volume-label' file couldn't ever be selected if
> there is also a
> device with 'volume-label' (it also seems more likely that in that
> situation, the file should be interacted with, over interpreting the
> token as an expression).
> 
> What are your thoughts behind the ordering (and alluded exploits)?

I think this is a bad approach. It means that a file in a working
directory from where the e2fsc/tune2fs is invoked can effectively
prevent user/admin to work with the file system with LABEL= or UUID=
without user/admin ever having any clue what it is going on.

You can always acces the file with the name LABEL=volume-label just by
specifying ./LABEL=volume-label or a full path, not a big deal and it
makes it very clear what the intentional target is.

I've got a patch that I plan to send out soon, in which I just check for
the collision and prefer the device found by blkid_get_devname(). Will
send it out soon, so let's move the discussion there.

Thanks!
-Lukas


> 
> Kind regards,
> Daniel
> 
> On Thu, Aug 4, 2022 at 12:31 AM Lukas Czerner <lczerner@redhat.com> wrote:
> >
> > On Tue, Aug 02, 2022 at 06:21:56PM +1000, Daniel Ng wrote:
> > > Hi,
> > >
> > > I've run into an issue when trying to use fsck with an ext4 image when
> > > it has '=' in its name.
> > >
> > > Repro steps:
> > > 1. fallocate -l 1G test=.img
> > > 2. mkfs.ext4 test=.img
> > > 3. fsck test=.img
> > >
> > > Response:
> > > 'fsck.ext4: Unable to resolve '<path>/test=.img'
> > >
> > > Expected:
> > > fsck to do it's thing.
> > >
> > > Observations:
> > > Originally I wasn't sure what the source was, I thought that maybe
> > > mkfs wasn't creating the image appropriately.
> > > However, I've tried:
> > > - renaming the image
> > > - creating a hard-link to the image
> > >
> > > Running fsck on either the renamed image, or the hard-link, works as expected.
> > >
> > > Kernel version: Linux version 4.19.251-13516-ga0bcf8d80077
> > > Environment: Running on a Chromebook
> > >
> > > Kind regards,
> > > Daniel
> >
> > Hi Daniel,
> >
> > yeah, that's a good catch. The problem is that various e2fsprogs tools
> > (at least tune2fs and e2fsck) are using blkid_get_devname() to get the
> > device name without ever checking if we already got the actual existing
> > device name.
> >
> > The reason to call blkid_get_devname() at all is to get device in the
> > form of NAME=value (like for example UUID=uuid, or LABEL=volume-label).
> > However if we blindly pass in the device (or in this case regular file)
> > name with an equal sign in it, the blkid_get_devname just returns whatever
> > it can find by that tag. Which is likely nothing.
> >
> > Unless of course, you're trying to use e2fsck, or tune2fs on a file with
> > an actual filename LABEL=volume-label and you have actual file system
> > with 'volume-label' LABEL ;) That's a problematic behavior and depending
> > on how we go about fixing it it could be potentialy exploitable...
> >
> > Maybe something like this:
> >
> >         1. look for the actual block device first
> >         2. if none is found call blkid_get_devname()
> >         3. if that didn't return anything maybe see if have a regular
> >            file and work with that
> >         4. if we still get nothing, then we're "Unable to resolve..."
> >
> > Thoughts?
> >
> > -Lukas
> >
>