Unless someone else is already working on this, I started working on a patch for this as well as fixes to correct the NTFS Grub test failures - I’ll hopefully submit something for review in a few days.
Once it is available - any additional testing that can be done would be appreciated. Thanks, Andrew On Mon, Mar 17, 2025 at 7:47 AM Andrew Hamilton <adham...@gmail.com> wrote: > For some fuzzers I’m working on, l happened to run into the same issue > this patch is addressing and used a fix similar to what Vladimir suggested > (keeping mft_end check and adding attr_cur != NULL check). This fixes the > issue for me in my fuzzer so far. > > Thanks, > Andrew > > On Fri, Feb 28, 2025 at 7:12 PM B Horn <b...@horn.uk> wrote: > >> >> On 28/02/2025 22:17, Vladimir 'phcoder' Serbinenko wrote: >> > Why do you remove boundary check? Maybe it's better to add the second >> > boundary check from below >> >> The issue stems from my original patches being rebased over `fs/ntfs: >> Fix out-of-bounds read`, with both my patches and that commit fixing the >> same fuzzed NTFS issue in the while-loop. I felt it was better to get it >> to known good state as I verified my original patches against quite a >> few NTFS volumes including several that caused the original OOB reads. >> >> Keeping the mft_end checks doesn't hurt after adding the attr_cur check >> back in, just not much point as at->end should always be the same value. >> >> > >> > Le ven. 28 févr. 2025, 16:11, B Horn <b...@horn.uk <mailto:b...@horn.uk>> a >> > écrit : >> > >> > On some NTFS volumes GRUB would enter an infinite loop when >> > next_attribute() returned NULL, which can happen in normal cases >> when >> > the end of the attribute list is reached. >> > This would trigger a NULL deref, but as the null page is mapped on >> the >> > majority of firmware, an infinite loop would occur as the while loop >> > didn't make any progress. >> > >> > Fixing this by verifying the value of at->attr_cur on the next >> iteration >> > of the loop, after it has been set to the result of >> next_attribute(). >> > Also removing the redundant check against mft_end as the >> > next_attribute() should handle that now. >> > A pointer to the end of the buffer is stored in at->end, which is >> > initialized the same way as mft_end was. >> > >> > Fixes: https://savannah.gnu.org/bugs/index.php?66855 <https:// >> > savannah.gnu.org/bugs/index.php?66855> >> > >> > Reported-by: Andreas Klauer <andreas.kla...@metamorpher.de >> > <mailto:andreas.kla...@metamorpher.de>> >> > Signed-off-by: B Horn <b...@horn.uk <mailto:b...@horn.uk>> >> > --- >> > grub-core/fs/ntfs.c | 5 +---- >> > 1 file changed, 1 insertion(+), 4 deletions(-) >> > >> > diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c >> > index 960833a34..77531e627 100644 >> > --- a/grub-core/fs/ntfs.c >> > +++ b/grub-core/fs/ntfs.c >> > @@ -311,8 +311,6 @@ free_attr (struct grub_ntfs_attr *at) >> > static grub_uint8_t * >> > find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr) >> > { >> > - grub_uint8_t *mft_end; >> > - >> > if (at->flags & GRUB_NTFS_AF_ALST) >> > { >> > retry: >> > @@ -386,8 +384,7 @@ find_attr (struct grub_ntfs_attr *at, >> > grub_uint8_t attr) >> > return NULL; >> > } >> > at->attr_cur = at->attr_nxt; >> > - mft_end = at->mft->buf + (at->mft->data->mft_size << >> > GRUB_NTFS_BLK_SHR); >> > - while (at->attr_cur < mft_end && *at->attr_cur != 0xFF) >> > + while (at->attr_cur && *at->attr_cur != 0xFF) >> > { >> > at->attr_nxt = next_attribute (at->attr_cur, at->end); >> > if (*at->attr_cur == GRUB_NTFS_AT_ATTRIBUTE_LIST) >> > -- >> > 2.48.1 >> > >> > >> > _______________________________________________ >> > Grub-devel mailing list >> > Grub-devel@gnu.org <mailto:Grub-devel@gnu.org> >> > https://lists.gnu.org/mailman/listinfo/grub-devel <https:// >> > lists.gnu.org/mailman/listinfo/grub-devel> >> > >> > >> > _______________________________________________ >> > Grub-devel mailing list >> > Grub-devel@gnu.org >> > https://lists.gnu.org/mailman/listinfo/grub-devel >> >> >> _______________________________________________ >> Grub-devel mailing list >> Grub-devel@gnu.org >> https://lists.gnu.org/mailman/listinfo/grub-devel >> >
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
