[Bug libdw/28659] New: UBSan seems to complain about an "integer overflow" in dwfl_segment_report_module

[Bug libdw/28659] New: UBSan seems to complain about an "integer overflow" in dwfl_segment_report_module

[evvers at ya dot ru]

https://sourceware.org/bugzilla/show_bug.cgi?id=28659

            Bug ID: 28659
           Summary: UBSan seems to complain about an "integer overflow" in
                    dwfl_segment_report_module
           Product: elfutils
           Version: unspecified
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: libdw
          Assignee: unassigned at sourceware dot org
          Reporter: evvers at ya dot ru
                CC: elfutils-devel at sourceware dot org
  Target Milestone: ---

Created attachment 13825
  --> https://sourceware.org/bugzilla/attachment.cgi?id=13825&action=edit
File triggering an integer overflow

One of systemd fuzz targets run under UBSan discovered an "integer-overflow".
It can be reproduced by building the master branch of the elfutils repository
with UBSan 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'
--enable-sanitize-undefined
make -j$(nproc) V=1
LD_PRELOAD="./libelf/libelf.so ./libdw/libdw.so"
UBSAN_OPTIONS=print_stacktrace=1:print_summary=1  ./src/stack --core
../oss-fuzz-41570
dwfl_segment_report_module.c:400:55: runtime error: signed integer overflow:
65535 * 65312 cannot be represented in type 'int'
    #0 0x7f48c29c057e in dwfl_segment_report_module
/home/vagrant/elfutils/libdwfl/dwfl_segment_report_module.c:400
    #1 0x7f48c29c7656 in _new.dwfl_core_file_report
/home/vagrant/elfutils/libdwfl/core-file.c:559
    #2 0x402b0f in parse_opt /home/vagrant/elfutils/src/stack.c:595
    #3 0x7f48c1c42f81 in __argp_parse (/lib64/libc.so.6+0x10cf81)
    #4 0x403d98 in main /home/vagrant/elfutils/src/stack.c:695
    #5 0x7f48c1b5db74 in __libc_start_main (/lib64/libc.so.6+0x27b74)
    #6 0x4024cd in _start (/home/vagrant/elfutils/src/stack+0x4024cd)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
dwfl_segment_report_module.c:400:55 in
```

It was also reported in
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=41570
```
        Running:
/mnt/scratch0/clusterfuzz/bot/inputs/fuzzer-testcases/crash-4613ca6be3014e2d8019781ba5061e2ebaad886f
dwfl_segment_report_module.c:400:55: runtime error: signed integer overflow:
65535 * 65312 cannot be represented in type 'int'
    #0 0x51915d in dwfl_segment_report_module
/src/elfutils/libdwfl/dwfl_segment_report_module.c:400:55
    #1 0x4b86e7 in dwfl_core_file_report
/src/elfutils/libdwfl/core-file.c:559:17
    #2 0x4b363e in LLVMFuzzerTestOneInput /src/fuzz-dwfl-core.c:52:6
    #3 0x43f1b3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*,
unsigned long) cxa_noexception.cpp:0
    #4 0x42aac2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned
long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
    #5 0x43058a in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char
const*, unsigned long)) cxa_noexception.cpp:0
    #6 0x4594b2 in main
/src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #7 0x7f0cfd13e0b2 in __libc_start_main
/build/glibc-eX1tMB/glibc-2.31/csu/libc-start.c:308:16
    #8 0x407d4d in _start
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
dwfl_segment_report_module.c:400:55 in
```

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libdw/28659] UBSan seems to complain about an "integer overflow" in dwfl_segment_report_module

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28659

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at sourceware dot org   |mark at klomp dot org
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-12-08
                 CC|                            |mark at klomp dot org

--- Comment #1 from Mark Wielaard <mark at klomp dot org> ---
Proposed patch:
https://sourceware.org/pipermail/elfutils-devel/2021q4/004475.html

Note that the overflow is actually harmless, the shdrs_end is sanity checked
later in the code and we don't really need the shdrs (like the comment just
before this code already explains).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libdw/28659] UBSan seems to complain about an "integer overflow" in dwfl_segment_report_module

[evvers at ya dot ru]

https://sourceware.org/bugzilla/show_bug.cgi?id=28659

--- Comment #2 from Evgeny Vereshchagin <evvers at ya dot ru> ---
> Note that the overflow is actually harmless

It is but since the fuzz target ran into it almost as soon as it started it
prevented the fuzz target from discovering new issues that can be less harmless
though.

Looks like the issue is gone. Thanks!

FWIW judging by
https://github.com/evverx/elfutils/pull/40#issuecomment-989275575, it fixed one
LGTM alert as well. I'm not sure if I mentioned this anywhere but LGTM builds
those reports on a daily basis and those reports can be found at
https://lgtm.com/projects/g/evverx/elfutils/alerts/?mode=tree .

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libdw/28659] UBSan seems to complain about an "integer overflow" in dwfl_segment_report_module

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28659

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #3 from Mark Wielaard <mark at klomp dot org> ---
Thanks for testing, pushed as:

commit b9ed67836b6f4e580927b4e8e1c8517e70a086be
Author: Mark Wielaard <mark@klomp.org>
Date:   Wed Dec 8 22:20:17 2021 +0100

    libdwfl: Don't trust e_shentsize in dwfl_segment_report_module

    When calulating the possible section header table end us the actual size
    of the section headers (sizeof (Elf32_Shdr) or sizeof (Elf64_Shdr)),
    not the ELF header e_shentsize value, which can be corrupted. This
    prevents a posssible overflow, but we check the shdrs_end is sane
    later anyway.

    https://sourceware.org/bugzilla/show_bug.cgi?id=28659

    Signed-off-by: Mark Wielaard <mark@klomp.org>

> it fixed one LGTM alert as well. I'm not sure if I mentioned this anywhere but
> LGTM builds those reports on a daily basis and those reports can be found at
> https://lgtm.com/projects/g/evverx/elfutils/alerts/?mode=tree .

Hmmm. At first I thought this was pretty useful to add to our own buildbot CI
setup. But it comes with a horribly proprietary license :{ "CodeQL can’t be
used for automated analysis, continuous integration or continuous delivery"
Sigh.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libdw/28659] UBSan seems to complain about an "integer overflow" in dwfl_segment_report_module

[evvers at ya dot ru]

https://sourceware.org/bugzilla/show_bug.cgi?id=28659

--- Comment #4 from Evgeny Vereshchagin <evvers at ya dot ru> ---
> But it comes with a horribly proprietary license

Unfortunately LGTM (like many other CI services) is tightly coupled with GitHub
(where it can be used for automated analysis of open source projects) so in
theory it should be possible to keep a read-only fork of the elfutils
repository updated by a cron script run by buildbot. I'm not sure whether it's
worth it though.

-- 
You are receiving this mail because:
You are on the CC list for the bug.