Huh!

B Horn, may I ask you to take a look at this and prepare a fix?

Andreas, please help with testing the fix.

Daniel

On Fri, Feb 28, 2025 at 10:55:46AM +0100, Andreas Klauer wrote:
> Hello,
>
> (I'm not on this list; hope this message finds you well.)
>
> it seems that this patch triggers an infinite loop when
> trying to access ntfs, so any search command that comes
> across any ntfs partition gets stuck.
>
> Basically this while-loop in find_attr()
>
>   while (at->attr_cur < mft_end && *at->attr_cur != 0xFF)
>     {
>       at->attr_nxt = next_attribute (at->attr_cur, at->end);
>       if (*at->attr_cur == GRUB_NTFS_AT_ATTRIBUTE_LIST)
> at->attr_end = at->attr_cur;
>       if ((*at->attr_cur == attr) || (attr == 0))
> return at->attr_cur;
>       at->attr_cur = at->attr_nxt;
>     }
>
> loops indefinitely (at->attr_cur=0) after next_attribute() returns NULL here:
>
>   next += u16at (curr_attribute, 4);
>   if (validate_attribute (next, end) == false)
>     return NULL;
>
> after validate_attribute() (introduced in this patch) returns false here
>
>   /* Not an error case, just reached the end of the attributes. */
>   if (attr_size == 0)
>     return false;
>
> Simply checking at->attr_cur in the while loop makes it work again:
>
>   while (at->attr_cur && at->attr_cur < mft_end && *at->attr_cur != 0xFF)
>
> but I don't understand half of what that code actually does,
> so I can't vouch for correctness (not sending it as a patch).
>
> Also filed here https://savannah.gnu.org/bugs/index.php?66855
>
> and here 
> https://gitlab.archlinux.org/archlinux/packaging/packages/grub/-/issues/12
>
> Kind regards,
> Andreas Klauer
>
> On Tue, Feb 18, 2025 at 07:00:24PM +0100, Daniel Kiper via Grub-devel wrote:
> > From: B Horn <b...@horn.uk>
> >
> > It was possible to read OOB when an attribute had a size that exceeded
> > the allocated buffer. This resolves that by making sure all attributes
> > that get read are fully in the allocated space by implementing
> > a function to validate them.
> >
> > Defining the offsets in include/grub/ntfs.h but they are only used in
> > the validation function and not across the rest of the NTFS code.
> >
> > Signed-off-by: B Horn <b...@horn.uk>
> > Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
> > ---
> >  grub-core/fs/ntfs.c | 153 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/grub/ntfs.h |  22 ++++++++
> >  2 files changed, 175 insertions(+)

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to