[Bug libelf/29000] New: Conditional jump or move depends on uninitialised value in elf_compress_gnu

[Bug libelf/29000] New: Conditional jump or move depends on uninitialised value in elf_compress_gnu

[evvers at ya dot ru]

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

            Bug ID: 29000
           Summary: Conditional jump or move depends on uninitialised
                    value in elf_compress_gnu
           Product: elfutils
           Version: unspecified
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: libelf
          Assignee: unassigned at sourceware dot org
          Reporter: evvers at ya dot ru
                CC: elfutils-devel at sourceware dot org
  Target Milestone: ---

Created attachment 14035
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14035&action=edit
file triggering valgrind warning

It was found with MSan on OSS-Fuzz but can be reproduced with Valgrind by
applying https://sourceware.org/pipermail/elfutils-devel/2022q1/004767.html and
running the following commands:
```
autoreconf -i -f
./configure --enable-maintainer-mode
make V=1 -j$(nproc)
make -C tests fuzz-libelf V=1
LD_LIBRARY_PATH="$(pwd)/libelf;$(pwd)/libdw" DEBUGINFOD_URLS= valgrind
--track-origins=yes ./tests/fuzz-libelf
clusterfuzz-testcase-minimized-fuzz-libelf-6467719510228992
```
```
unning: ../clusterfuzz-testcase-minimized-fuzz-libelf-6467719510228992
==65519== Conditional jump or move depends on uninitialised value(s)
==65519==    at 0x4868734: elf_compress_gnu (elf_compress_gnu.c:155)
==65519==    by 0x401553: fuzz_logic_one (fuzz-libelf.c:41)
==65519==    by 0x4016D9: LLVMFuzzerTestOneInput (fuzz-libelf.c:82)
==65519==    by 0x4012B8: main (fuzz-main.c:33)
==65519==  Uninitialised value was created by a heap allocation
==65519==    at 0x484486F: malloc (vg_replace_malloc.c:381)
==65519==    by 0x48606C6: convert_data (elf_getdata.c:168)
==65519==    by 0x48606C6: __libelf_set_data_list_rdlock (elf_getdata.c:457)
==65519==    by 0x48608C7: __elf_getdata_rdlock (elf_getdata.c:564)
==65519==    by 0x486870A: elf_compress_gnu (elf_compress_gnu.c:150)
==65519==    by 0x401553: fuzz_logic_one (fuzz-libelf.c:41)
==65519==    by 0x4016D9: LLVMFuzzerTestOneInput (fuzz-libelf.c:82)
==65519==    by 0x4012B8: main (fuzz-main.c:33)
==65519==
Done:    ../clusterfuzz-testcase-minimized-fuzz-libelf-6467719510228992: (608
bytes)
==65519==
```

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

[Bug libelf/29000] Conditional jump or move depends on uninitialised value in elf_compress_gnu

[evvers at ya dot ru]

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

--- Comment #1 from Evgeny Vereshchagin <evvers at ya dot ru> ---
Created attachment 14036
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14036&action=edit
file triggering issue in fuzz-libdwfl

The same issue was found by fuzz-libdwfl.
```
make -C tests fuzz-libdwfl
LD_LIBRARY_PATH="$(pwd)/libelf;$(pwd)/libdw" DEBUGINFOD_URLS= valgrind
--track-origins=yes ./tests/fuzz-libdwfl
../clusterfuzz-testcase-minimized-fuzz-libdwfl-4725021634854912
```
```
Running: ../clusterfuzz-testcase-minimized-fuzz-libdwfl-4725021634854912
==65641== Conditional jump or move depends on uninitialised value(s)
==65641==    at 0x4868734: elf_compress_gnu (elf_compress_gnu.c:155)
==65641==    by 0x489EF0E: check_section (dwarf_begin_elf.c:210)
==65641==    by 0x489F6D2: global_read (dwarf_begin_elf.c:409)
==65641==    by 0x489F6D2: dwarf_begin_elf (dwarf_begin_elf.c:560)
==65641==    by 0x48C033C: load_dw (dwfl_module_getdwarf.c:1342)
==65641==    by 0x48C0500: find_dw (dwfl_module_getdwarf.c:1392)
==65641==    by 0x48C0500: dwfl_module_getdwarf (dwfl_module_getdwarf.c:1447)
==65641==    by 0x401512: LLVMFuzzerTestOneInput (fuzz-libdwfl.c:45)
==65641==    by 0x401248: main (fuzz-main.c:33)
==65641==  Uninitialised value was created by a heap allocation
==65641==    at 0x484486F: malloc (vg_replace_malloc.c:381)
==65641==    by 0x48606C6: convert_data (elf_getdata.c:168)
==65641==    by 0x48606C6: __libelf_set_data_list_rdlock (elf_getdata.c:457)
==65641==    by 0x48608C7: __elf_getdata_rdlock (elf_getdata.c:564)
==65641==    by 0x486870A: elf_compress_gnu (elf_compress_gnu.c:150)
==65641==    by 0x489EF0E: check_section (dwarf_begin_elf.c:210)
==65641==    by 0x489F6D2: global_read (dwarf_begin_elf.c:409)
==65641==    by 0x489F6D2: dwarf_begin_elf (dwarf_begin_elf.c:560)
==65641==    by 0x48C033C: load_dw (dwfl_module_getdwarf.c:1342)
==65641==    by 0x48C0500: find_dw (dwfl_module_getdwarf.c:1392)
==65641==    by 0x48C0500: dwfl_module_getdwarf (dwfl_module_getdwarf.c:1447)
==65641==    by 0x401512: LLVMFuzzerTestOneInput (fuzz-libdwfl.c:45)
==65641==    by 0x401248: main (fuzz-main.c:33)
==65641==
Done:    ../clusterfuzz-testcase-minimized-fuzz-libdwfl-4725021634854912: (1087
bytes)
```

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

[Bug libelf/29000] Conditional jump or move depends on uninitialised value in elf_compress_gnu

[mark at klomp dot org]

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

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                 CC|                            |mark at klomp dot org
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-03-25

--- Comment #2 from Mark Wielaard <mark at klomp dot org> ---
That is interesting, it seems to occur because the section is decompressed
twice (and fails). Apparently elf_getdata leaves some uninitialized data around
the first time, which gets picked up (and checked) the second time.

This can only happen when using elf_compress_gnu because we cannot tell whether
or not the data is already (de)compressed (it is a convention based on the
section name, instead of using a proper section flag).

It can also be triggered on the second example with valgrind eu-readelf -w

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

[Bug libelf/29000] Conditional jump or move depends on uninitialised value in elf_compress_gnu

[mark at klomp dot org]

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

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at sourceware dot org   |mark at klomp dot org

--- Comment #3 from Mark Wielaard <mark at klomp dot org> ---
> It can also be triggered on the second example with valgrind eu-readelf -w

Thanks, that was very useful. I don't think it really is a bug. But it is use
of undefined data. The issue is that if the ELF data structures need to be
converted then it only makes sense to convert full datastructures. But just
dropping the bad/partial data is not a great idea either. So this proposed
patch just copies over the bad/partial data that couldn't be converted. That
way it is at least deterministically defined.

https://code.wildebeest.org/git/user/mjw/elfutils/commit/?h=fuzz
https://sourceware.org/pipermail/elfutils-devel/2022q1/004825.html

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

[Bug libelf/29000] Conditional jump or move depends on uninitialised value in elf_compress_gnu

[evvers at ya dot ru]

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

--- Comment #4 from Evgeny Vereshchagin <evvers at ya dot ru> ---
I rebased the "fuzz" branch on top of my fork and ran all the tests in
https://github.com/evverx/elfutils/pull/73. MSan no longer complains. Thanks!

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

[Bug libelf/29000] Conditional jump or move depends on uninitialised value in elf_compress_gnu

[mark at klomp dot org]

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

Mark Wielaard <mark at klomp dot org> changed:

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

--- Comment #5 from Mark Wielaard <mark at klomp dot org> ---
(In reply to Evgeny Vereshchagin from comment #4)
> I rebased the "fuzz" branch on top of my fork and ran all the tests in
> https://github.com/evverx/elfutils/pull/73. MSan no longer complains. Thanks!

Thanks for testing. Pushed as:

commit e3e2ae06fbfcd1b2f3de6945689ef9d9c94a2123
Author: Mark Wielaard <mark@klomp.org>
Date:   Wed Mar 30 00:17:08 2022 +0200

    libelf: Also copy/convert partial datastructures in xlate functions

    The generated xlate functions can only convert full datastructures,
    dropping any trailing partial data on the floor. That means some of
    the data might be undefined. Just copy over the trailing bytes as
    is. That data isn't really usable. But at least it is defined data.

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

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

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