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@xxxxxxxxxx> 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 > > >
