]> git.ipfire.org Git - thirdparty/grub.git/commit
fs/hfsplus: Don't use uninitialized data on corrupt filesystems
authorDaniel Axtens <dja@axtens.net>
Tue, 2 Feb 2021 05:59:35 +0000 (16:59 +1100)
committerDaniel Kiper <daniel.kiper@oracle.com>
Tue, 2 Mar 2021 14:54:18 +0000 (15:54 +0100)
commit2ca0e5dbcdcb6fc93ccae39a0f39d0dba4a7ff20
tree41dd605b189e7078cd291b89b21283922af026ba
parent58ea11d5b9ca0966bd9c68d8ba5240cf7dc3ba83
fs/hfsplus: Don't use uninitialized data on corrupt filesystems

Valgrind identified the following use of uninitialized data:

  ==2782220== Conditional jump or move depends on uninitialised value(s)
  ==2782220==    at 0x42B364: grub_hfsplus_btree_search (hfsplus.c:566)
  ==2782220==    by 0x42B21D: grub_hfsplus_read_block (hfsplus.c:185)
  ==2782220==    by 0x42A693: grub_fshelp_read_file (fshelp.c:386)
  ==2782220==    by 0x42C598: grub_hfsplus_read_file (hfsplus.c:219)
  ==2782220==    by 0x42C598: grub_hfsplus_mount (hfsplus.c:330)
  ==2782220==    by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958)
  ==2782220==    by 0x4C1AE6: grub_fs_probe (fs.c:73)
  ==2782220==    by 0x407C94: grub_ls_list_files (ls.c:186)
  ==2782220==    by 0x407C94: grub_cmd_ls (ls.c:284)
  ==2782220==    by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55)
  ==2782220==    by 0x4045A6: execute_command (grub-fstest.c:59)
  ==2782220==    by 0x4045A6: fstest (grub-fstest.c:433)
  ==2782220==    by 0x4045A6: main (grub-fstest.c:772)
  ==2782220==  Uninitialised value was created by a heap allocation
  ==2782220==    at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==2782220==    by 0x4C0305: grub_malloc (mm.c:42)
  ==2782220==    by 0x42C21D: grub_hfsplus_mount (hfsplus.c:239)
  ==2782220==    by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958)
  ==2782220==    by 0x4C1AE6: grub_fs_probe (fs.c:73)
  ==2782220==    by 0x407C94: grub_ls_list_files (ls.c:186)
  ==2782220==    by 0x407C94: grub_cmd_ls (ls.c:284)
  ==2782220==    by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55)
  ==2782220==    by 0x4045A6: execute_command (grub-fstest.c:59)
  ==2782220==    by 0x4045A6: fstest (grub-fstest.c:433)
  ==2782220==    by 0x4045A6: main (grub-fstest.c:772)

This happens when the process of reading the catalog file goes sufficiently
wrong that there's an attempt to read the extent overflow file, which has
not yet been loaded. Keep track of when the extent overflow file is
fully loaded and refuse to use it before then.

The load valgrind doesn't like is btree->nodesize, and that's then used
to allocate a data structure. It looks like there are subsequently a lot
of reads based on that pointer so OOB reads are likely, and indeed crashes
(albeit difficult-to-replicate ones) have been observed in fuzzing.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
grub-core/fs/hfsplus.c
include/grub/hfsplus.h