Bug 28660 - ASan seems to complain about a "heap-buffer-overflow"
Summary: ASan seems to complain about a "heap-buffer-overflow"
Status: RESOLVED FIXED
Alias: None
Product: elfutils
Classification: Unclassified
Component: libdw (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Mark Wielaard
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-12-06 13:16 UTC by Evgeny Vereshchagin
Modified: 2021-12-16 21:36 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed: 2021-12-15 00:00:00


Attachments
File triggering a heap-buffer-overflow (131 bytes, application/octet-stream)
2021-12-06 13:16 UTC, Evgeny Vereshchagin
Details
File triggering an "invalid read" (1.44 KB, application/x-core)
2021-12-09 21:20 UTC, Evgeny Vereshchagin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Evgeny Vereshchagin 2021-12-06 13:16:03 UTC
Created attachment 13826 [details]
File triggering a heap-buffer-overflow

One of systemd fuzz targets triggered a "heap-buffer-overflow". It can be reproduced by building the master branch of the elfutils repository with ASan and passing the attachment to `./src/stack`:
```
$ git describe
elfutils-0.186-7-g99782bd2

autoreconf -i -f
./configure --enable-maintainer-mode CFLAGS='-g -O1 -fno-omit-frame-pointer -fsanitize=address' CXXFLAGS='-g -O1 -fno-omit-frame-pointer -fsanitize=address'
ASAN_OPTIONS=detect_leaks=0 make -j$(nproc) V=1
LD_PRELOAD="/lib64/libasan.so.6 ./libelf/libelf.so ./libdw/libdw.so" UBSAN_OPTIONS=print_stacktrace=1:print_summary=1  ./src/stack --core ../oss-fuzz-41566
=================================================================
==85112==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000001b70 at pc 0x7f96b3109a31 bp 0x7ffebaf7f470 sp 0x7ffebaf7f468
READ of size 4 at 0x602000001b70 thread T0
    #0 0x7f96b3109a30 in dwfl_link_map_report /home/vagrant/elfutils/libdwfl/link_map.c:904
    #1 0x7f96b310e877 in _new.dwfl_core_file_report /home/vagrant/elfutils/libdwfl/core-file.c:548
    #2 0x4037b8 in parse_opt /home/vagrant/elfutils/src/stack.c:595
    #3 0x7f96b2d05f81 in __argp_parse (/lib64/libc.so.6+0x10cf81)
    #4 0x404b7d in main /home/vagrant/elfutils/src/stack.c:695
    #5 0x7f96b2c20b74 in __libc_start_main (/lib64/libc.so.6+0x27b74)
    #6 0x40256d in _start (/home/vagrant/elfutils/src/stack+0x40256d)

0x602000001b71 is located 0 bytes to the right of 1-byte region [0x602000001b70,0x602000001b71)
allocated by thread T0 here:
    #0 0x7f96b32b693f in __interceptor_malloc (/lib64/libasan.so.6+0xae93f)
    #1 0x7f96b31096c4 in dwfl_link_map_report /home/vagrant/elfutils/libdwfl/link_map.c:879
    #2 0x7f96b310e877 in _new.dwfl_core_file_report /home/vagrant/elfutils/libdwfl/core-file.c:548
    #3 0x4037b8 in parse_opt /home/vagrant/elfutils/src/stack.c:595
    #4 0x7f96b2d05f81 in __argp_parse (/lib64/libc.so.6+0x10cf81)
    #5 0x404b7d in main /home/vagrant/elfutils/src/stack.c:695
    #6 0x7f96b2c20b74 in __libc_start_main (/lib64/libc.so.6+0x27b74)

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/vagrant/elfutils/libdwfl/link_map.c:904 in dwfl_link_map_report
Shadow bytes around the buggy address:
  0x0c047fff8310: fa fa 00 03 fa fa 00 04 fa fa fd fa fa fa fd fa
  0x0c047fff8320: fa fa 00 03 fa fa 00 04 fa fa fd fa fa fa fd fa
  0x0c047fff8330: fa fa 00 03 fa fa 00 04 fa fa fd fa fa fa fd fa
  0x0c047fff8340: fa fa 00 03 fa fa 00 04 fa fa fd fa fa fa fd fa
  0x0c047fff8350: fa fa 00 03 fa fa 00 04 fa fa fd fa fa fa fd fa
=>0x0c047fff8360: fa fa 00 03 fa fa 00 04 fa fa 00 04 fa fa[01]fa
  0x0c047fff8370: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8390: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff83a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff83b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==85112==ABORTING
```

It was also reported in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=41566
Comment 1 Mark Wielaard 2021-12-08 20:11:07 UTC
Valgrind also complains about this.
But this seems to be resolved by the proposed patch for PR28657.
https://sourceware.org/bugzilla/show_bug.cgi?id=28657
Which makes sense since we aren't trying to deal with a zero phdr size.
Comment 2 Evgeny Vereshchagin 2021-12-08 20:25:01 UTC
As far as I can see both issues are gone with that patch applied. Thanks!
Comment 3 Evgeny Vereshchagin 2021-12-09 21:09:52 UTC
Looks like it keeps popping up with all the patches applied
```
0a2c8345 libdwfl: Don't try to convert too many dyns in dwfl_link_map_report
ea8ce550 libdwfl: Don't install an Elf handle in a Dwfl_Module twice
906e0ca5 libdwfl: Don't trust e_shentsize in dwfl_segment_report_module
a5dc98be libdwfl: Make sure we know the phdr entry size before searching phdrs.
8ae296dc libdwfl: Add overflow check while iterating in dwfl_segment_report_module
c0dd1c35 libdwfl: Don't try to convert too many bytes in dwfl_link_map_report
5ba884a5 configure: Add --enable-sanitize-address
```
I'll attach a file triggering it once the fuzz target runs into it again
Comment 4 Evgeny Vereshchagin 2021-12-09 21:20:21 UTC
Created attachment 13842 [details]
File triggering an "invalid read"

I've just attached a file triggering the issue:

```
autoreconf -i -f
./configure --enable-maintainer-mode
make -j$(nproc) V=1
DEBUGINFOD_URLS= LD_PRELOAD="./libelf/libelf.so ./libdw/libdw.so" valgrind --leak-check=full ./src/stack --core ../crash-e8e47de6a28b1be30e3a7e2f92b7c9e4f4fffa9d
==87229== Memcheck, a memory error detector
==87229== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==87229== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==87229== Command: ./src/stack --core ../crash-e8e47de6a28b1be30e3a7e2f92b7c9e4f4fffa9d
==87229==
==87229== Invalid read of size 4
==87229==    at 0x48C783F: dwfl_link_map_report (link_map.c:917)
==87229==    by 0x48C8DC5: dwfl_core_file_report@@ELFUTILS_0.158 (core-file.c:548)
==87229==    by 0x402EC6: parse_opt (stack.c:595)
==87229==    by 0x4C4D591: argp_parse (in /usr/lib64/libc.so.6)
==87229==    by 0x4024EA: main (stack.c:695)
==87229==  Address 0x5029ae0 is 0 bytes after a block of size 4,096 alloc'd
==87229==    at 0x484186F: malloc (vg_replace_malloc.c:381)
==87229==    by 0x48C7D6B: dwfl_link_map_report (link_map.c:891)
==87229==    by 0x48C8DC5: dwfl_core_file_report@@ELFUTILS_0.158 (core-file.c:548)
==87229==    by 0x402EC6: parse_opt (stack.c:595)
==87229==    by 0x4C4D591: argp_parse (in /usr/lib64/libc.so.6)
==87229==    by 0x4024EA: main (stack.c:695)
```
Comment 5 Mark Wielaard 2021-12-15 23:44:21 UTC
Proposed some more sanity checks:
https://sourceware.org/pipermail/elfutils-devel/2021q4/004523.html
Comment 6 Evgeny Vereshchagin 2021-12-16 00:53:28 UTC
Thanks! I can confirm that the issue is gone.

I tested it in https://github.com/evverx/elfutils/pull/53 by adding that file to the testsuite in https://github.com/evverx/elfutils/pull/53/commits/38c241bf6269ab5a1dd93b606e11001dc3b6c1f4. I also ran the fuzzer for about 10 minutes.

Interestingly, something started to trigger unreproducible MSan crashes but I'm inclined to say it was probably a fluke.
Comment 7 Evgeny Vereshchagin 2021-12-16 01:48:03 UTC
> Interestingly, something started to trigger unreproducible MSan crashes but
> I'm inclined to say it was probably a fluke.

It wasn't a glitch. The file I added to the test suite was also automatically added to the seed corpus, which in turn triggered new code paths and led to large allocations I reported elsewhere. So it doesn't have anything to do with this issue and the patch.
Comment 8 Mark Wielaard 2021-12-16 21:36:34 UTC
Thanks for testing. Pushed as:

commit 3c9b69161b842708b4ef2f4e0f0b3ad1812798c2
Author: Mark Wielaard <mark@klomp.org>
Date:   Thu Dec 16 00:29:22 2021 +0100

    libdwfl: Make sure phent is sane and there is at least one phdr
    
    dwfl_link_map_report can only handle program headers that are the
    correct (32 or 64 bit) size. The buffer read in needs to contain room
    for at least one Phdr.
    
    https://sourceware.org/bugzilla/show_bug.cgi?id=28660
    
    Signed-off-by: Mark Wielaard <mark@klomp.org>