global out of bounds reads in file_stat() / check_dir()

[hannob]

This input file will cause out of bounds reads in fsck.fat:
https://crashes.fuzzing-project.org/dosfstools-date_dos2unix-oob-read-heap

This can be seen with address sanitizer. Now the situation seems a bit tricky, because different combinations of compilers (gcc or clang) and cflags will show different bugs.

This is with CC set to clang (3.7) and CFLAGS="-fsanitize=address -O3":

==20294==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00000050dc7c at pc 0x0000004e700e bp 0x7ffdef038790 sp 0x7ffdef038788
READ of size 4 at 0x00000050dc7c thread T0
    #0 0x4e700d in check_dir (/mnt/ram/dosfstools/src/fsck.fat+0x4e700d)
    #1 0x4e41a5 in scan_root (/mnt/ram/dosfstools/src/fsck.fat+0x4e41a5)
    #2 0x4f68c9 in main (/mnt/ram/dosfstools/src/fsck.fat+0x4f68c9)
    #3 0x7f6d37dae7af in __libc_start_main /var/tmp/portage/sys-libs/glibc-2.21-r1/work/glibc-2.21/csu/libc-start.c:289
    #4 0x418058 in _start (/mnt/ram/dosfstools/src/fsck.fat+0x418058)

0x00000050dc7c is located 4 bytes to the left of global variable 'day_n' defined in 'check.c:227:12' (0x50dc80) of size 64
0x00000050dc7c is located 35 bytes to the right of global variable '<string literal>' defined in 'check.c:259:19' (0x50dc40) of size 25
  '<string literal>' is ascii string '  Size %u bytes, date %s'

This is with CC set to gcc (5.2.0) and CFLAGS="-fsanitize=address -O3" (with older gcc 4.9 bug doesn't show up):

==20954==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00000041c19c at pc 0x0000004058de bp 0x7ffd2dbed990 sp 0x7ffd2dbed980
READ of size 4 at 0x00000041c19c thread T0
    #0 0x4058dd in file_stat.isra.0 (/mnt/ram/dosfstools/src/fsck.fat+0x4058dd)
    #1 0x40b7df in check_dir (/mnt/ram/dosfstools/src/fsck.fat+0x40b7df)
    #2 0x40e3a6 in scan_root (/mnt/ram/dosfstools/src/fsck.fat+0x40e3a6)
    #3 0x402179 in main (/mnt/ram/dosfstools/src/fsck.fat+0x402179)
    #4 0x7f1bf7ca37af in __libc_start_main (/lib64/libc.so.6+0x207af)
    #5 0x402498 in _start (/mnt/ram/dosfstools/src/fsck.fat+0x402498)

0x00000041c19c is located 36 bytes to the right of global variable '*.LC94' defined in 'check.c' (0x41c160) of size 24
  '*.LC94' is ascii string 'Root directory is full.'
0x00000041c19c is located 4 bytes to the left of global variable 'day_n' defined in 'check.c:227:12' (0x41c1a0) of size 64

This is with clang (3.7) and CFLAGS="-fsanitize=address -g -O3":

==27976==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00000050de3c at pc 0x0000004e700e bp 0x7ffd42a7a790 sp 0x7ffd42a7a788
READ of size 4 at 0x00000050de3c thread T0
    #0 0x4e700d in date_dos2unix /mnt/ram/dosfstools/src/check.c:242:29
    #1 0x4e700d in file_stat /mnt/ram/dosfstools/src/check.c:256
    #2 0x4e700d in check_dir /mnt/ram/dosfstools/src/check.c:771
    #3 0x4e41a5 in scan_root /mnt/ram/dosfstools/src/check.c:1081:11
    #4 0x4f6a79 in main /mnt/ram/dosfstools/src/fsck.fat.c:187:27
    #5 0x7fc8c5cf57af in __libc_start_main /var/tmp/portage/sys-libs/glibc-2.21-r1/work/glibc-2.21/csu/libc-start.c:289
    #6 0x418058 in _start (/mnt/ram/dosfstools/src/fsck.fat+0x418058)

0x00000050de3c is located 4 bytes to the left of global variable 'day_n' defined in 'check.c:227:12' (0x50de40) of size 64
0x00000050de3c is located 35 bytes to the right of global variable '<string literal>' defined in 'check.c:259:19' (0x50de00) of size 25
  '<string literal>' is ascii string '  Size %u bytes, date %s'

So it seems depending on optimizations different bugs are triggered.

[andreasbombe]

Looks like the same issue everytime actually – it accesses day_n[-1]. That happens when date_dos2unix() is called with a zero date parameter, or at least one where the month bits are zero.

Haven't looked yet where to put the proper fix (as in, should the zero month field be prevented earlier or just handled here).

[andreasbombe]

Looking at it that function is only called to display information to the user when asking about duplicate file names. Checking date and time fields should also happen but not here. So I just added code to handle the zero month case in date_dos2unix().

[petterreinholdtsen]

For the record, this issue is said to be fixed by commit 2aad1c8 .