Re: [PATCH v2] Fix XFS directory extent parsing

[Jon DeVree]

I'm about to head out camping for a few days so I don't have time to
look into this but I think I found another bug in the original fuzzer
patch. See below.

On Fri, Sep 15, 2023 at 17:51:21 -0400, Jon DeVree wrote:
> @@ -917,7 +935,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
>                 }
>  
>               filename = (char *)(direntry + 1);
> -             if (filename + direntry->len - 1 > (char *) tail)
> +             if (filename + direntry->len - 1 > (char *) end)
>                 return grub_error (GRUB_ERR_BAD_FS, "invalid XFS directory 
> entry");
>  
>               /* The byte after the filename is for the filetype, padding, or

The -1 in that if statement is probably supposed to be a +1. The -1
might have been enough to make the fuzzer happy, but a few lines later
outside of the diff context is:

    filename[direntry->len] = '\0';

So I'm pretty sure the bounds check should be "filename + len + 1"
rather than "filename + len - 1". The incorrect bounds check is harmless
as long as the filesystem is valid though. I don't have ready access to
any tools that would let me create a corrupted filesystem to test this.



-- 
Jon
Doge Wrangler
X(7): A program for managing terminal windows. See also screen(1) and tmux(1).