Bug 23528 - When executing ./eu-nm or ./eu-readelf -aAdehIlnrsSVcp -w, AddressSanitizer catch a double-free crash.
Summary: When executing ./eu-nm or ./eu-readelf -aAdehIlnrsSVcp -w, AddressSanitizer c...
Status: RESOLVED FIXED
Alias: None
Product: elfutils
Classification: Unclassified
Component: libelf (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-15 12:18 UTC by wcventure
Modified: 2018-09-04 07:34 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
When executing ./eu-nm or ./eu-readelf -aAdehIlnrsSVcp -w, AddressSanitizer chatch a double-free crashes. (818 bytes, application/x-object)
2018-08-15 12:18 UTC, wcventure
Details

Note You need to log in before you can comment on or make changes to this bug.
Description wcventure 2018-08-15 12:18:05 UTC
Created attachment 11185 [details]
When executing ./eu-nm or ./eu-readelf -aAdehIlnrsSVcp -w, AddressSanitizer chatch a double-free crashes.

When executing ./eu-nm @@ or ./eu-readelf -aAdehIlnrsSVcp -w @@, AddressSanitizer chatch a double-free crashes.

The AddressSanitizer's output shows as follow:
==30316==ERROR: AddressSanitizer: attempting double-free on 0x60400000de50 in thread T0:
    #0 0x7ffffe5282ca in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x982ca)
    #1 0x7ffffde26c83 in elf_end /mnt/d/Project/elfutils/libelf/elf_end.c:171
    #2 0x7ffffe1b23f6 in free_file /mnt/d/Project/elfutils/libdwfl/dwfl_module.c:57
    #3 0x7ffffe1b23f6 in __libdwfl_module_free /mnt/d/Project/elfutils/libdwfl/dwfl_module.c:113
    #4 0x7ffffe1b06bc in dwfl_end /mnt/d/Project/elfutils/libdwfl/dwfl_end.c:54
    #5 0x409883 in show_symbols /mnt/d/Project/elfutils/src/nm.c:1494
    #6 0x40cf4c in handle_elf /mnt/d/Project/elfutils/src/nm.c:1578
    #7 0x4035dc in process_file /mnt/d/Project/elfutils/src/nm.c:374
    #8 0x4035dc in main /mnt/d/Project/elfutils/src/nm.c:249
    #9 0x7ffffd4c082f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #10 0x4043c8 in _start (/mnt/d/Project/elfutils/build/bin/eu-nm+0x4043c8)

0x60400000de50 is located 0 bytes inside of 36-byte region [0x60400000de50,0x60400000de74)
freed by thread T0 here:
    #0 0x7ffffe5282ca in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x982ca)
    #1 0x7ffffdea3dcf in __libelf_reset_rawdata /mnt/d/Project/elfutils/libelf/elf_compress.c:325

previously allocated by thread T0 here:
    #0 0x7ffffe528602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0x7ffffdea31dd in __libelf_decompress /mnt/d/Project/elfutils/libelf/elf_compress.c:223
    #2 0x7ffffdeaa490  (/mnt/d/Project/elfutils/build/lib/libelf.so.1+0x9a490)

SUMMARY: AddressSanitizer: double-free ??:0 __interceptor_free
==30316==ABORTING
[Inferior 1 (process 30316) exited with code 01]
Comment 1 Mark Wielaard 2018-08-15 13:20:30 UTC
Replicated under valgrind:

$ valgrind -q eu-readelf -S Double-free-libelf

==13892== Invalid free() / delete / delete[] / realloc()
==13892==    at 0x48369EB: free (vg_replace_malloc.c:530)
==13892==    by 0x4899094: elf_end (elf_end.c:171)
==13892==    by 0x4868619: free_file (dwfl_module.c:57)
==13892==    by 0x4868787: __libdwfl_module_free (dwfl_module.c:113)
==13892==    by 0x4868370: dwfl_end (dwfl_end.c:54)
==13892==    by 0x114617: process_file (readelf.c:873)
==13892==    by 0x111C13: main (readelf.c:350)
==13892==  Address 0x51138b0 is 0 bytes inside a block of size 36 free'd
==13892==    at 0x48369EB: free (vg_replace_malloc.c:530)
==13892==    by 0x48A5C46: __libelf_reset_rawdata (elf_compress.c:325)
==13892==    by 0x48A5D80: elf_compress (elf_compress.c:514)
==13892==    by 0x4869F00: relocate_section (relocate.c:507)
==13892==    by 0x486A567: __libdwfl_relocate (relocate.c:752)
==13892==    by 0x486EB5B: dwfl_module_getelf (dwfl_module_getelf.c:52)
==13892==    by 0x11E764: process_elf_file (readelf.c:901)
==13892==    by 0x11E764: process_dwflmod (readelf.c:760)
==13892==    by 0x486C450: dwfl_getmodules (dwfl_getmodules.c:86)
==13892==    by 0x1143BF: process_file (readelf.c:868)
==13892==    by 0x111C13: main (readelf.c:350)
==13892==  Block was alloc'd at
==13892==    at 0x48357BF: malloc (vg_replace_malloc.c:299)
==13892==    by 0x48A59E7: __libelf_decompress (elf_compress.c:223)
==13892==    by 0x48A60D4: elf_compress_gnu (elf_compress_gnu.c:178)
==13892==    by 0x4869F68: relocate_section (relocate.c:504)
==13892==    by 0x486A567: __libdwfl_relocate (relocate.c:752)
==13892==    by 0x486EB5B: dwfl_module_getelf (dwfl_module_getelf.c:52)
==13892==    by 0x11E764: process_elf_file (readelf.c:901)
==13892==    by 0x11E764: process_dwflmod (readelf.c:760)
==13892==    by 0x486C450: dwfl_getmodules (dwfl_getmodules.c:86)
==13892==    by 0x1143BF: process_file (readelf.c:868)
==13892==    by 0x111C13: main (readelf.c:350)

The issue is this section:

[10] .zdebug_abbrev       PROGBITS     00000000 00011d 00002b  0 NGTC   0   0  1

Note how it claims to be both gnu style compressed (starts with .zdebug) and gabi compressed (has SHF_COMPRESSED flag C).

Then the code in relocate.c tries to decompress the section data twice:

  if (strncmp (tname, ".zdebug", strlen ("zdebug")) == 0)
    elf_compress_gnu (tscn, 0, 0);

  if ((tshdr->sh_flags & SHF_COMPRESSED) != 0)
    if (elf_compress (tscn, 0, 0) < 0)
      return DWFL_E_LIBELF;

So it tries to decompress twice. The second call sees that the data is already decompressed and assumes it was done implicitly by some call to elf_strptr, and so just uses the decompressed data to setup the decompressed section data again, throwing away the buffer that elf_compress_gnu has already setup. elf_end then tries to free this data buffer again.

The most direct fix is to just make relocate.c not decompress twice in different ways:

diff --git a/libdwfl/relocate.c b/libdwfl/relocate.c
index 9afcdebe..81750cd3 100644
--- a/libdwfl/relocate.c
+++ b/libdwfl/relocate.c
@@ -238,8 +238,7 @@ resolve_symbol (Dwfl_Module *referer, struct reloc_symtab_cache *symtab,
          /* If the section is already decompressed, that isn't an error.  */
          if (strncmp (sname, ".zdebug", strlen (".zdebug")) == 0)
            elf_compress_gnu (scn, 0, 0);
-
-         if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
+         else if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
            if (elf_compress (scn, 0, 0) < 0)
              return DWFL_E_LIBELF;
 
@@ -502,8 +501,7 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
 
   if (strncmp (tname, ".zdebug", strlen ("zdebug")) == 0)
     elf_compress_gnu (tscn, 0, 0);
-
-  if ((tshdr->sh_flags & SHF_COMPRESSED) != 0)
+  else if ((tshdr->sh_flags & SHF_COMPRESSED) != 0)
     if (elf_compress (tscn, 0, 0) < 0)
       return DWFL_E_LIBELF;
 
@@ -523,8 +521,7 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
 
   if (strncmp (sname, ".zdebug", strlen ("zdebug")) == 0)
     elf_compress_gnu (scn, 0, 0);
-
-  if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
+  else if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
     if (elf_compress (scn, 0, 0) < 0)
       return DWFL_E_LIBELF;
 

But we can probably fix it in elf_compress[_gnu] by better checks or just by refusing to elf_compress_gnu if the section already has the SHF_COMPRESSED flag set.
Comment 2 Frank Ch. Eigler 2018-08-15 15:28:10 UTC
FWIW, is there some technical reason why an .zdebug section couldn't possibly ALSO be SHT_COMPRESSED ?  i.e., compressed twice for some reason by the generator?
Comment 3 Mark Wielaard 2018-08-15 20:43:32 UTC
(In reply to Frank Ch. Eigler from comment #2)
> FWIW, is there some technical reason why an .zdebug section couldn't
> possibly ALSO be SHT_COMPRESSED ?  i.e., compressed twice for some reason by
> the generator?

In theory you could gabi compress a section that is GNU compressed. But in practice eu-elfcompress won't let you do that (it will first decompress the .zdebug_xxx section, rename it to .debug_xxx and then gabi compress it). It would also be somewhat pointless since they use the same zlib compression scheme. So unless you use ELF_CHF_FORCE it wouldn't actually work since the result would likely be bigger.

The other way around however (GNU compress an already gabi compressed section) would be problematic. There is no good way to know whether the data is corrupted or already (de)compressed since the GNU compression has no meta-data (like Elf[32|64]_Chdr or a section flag) associated only the implicit section naming, but from just the name you cannot know whether or not the section data has already be (de)compressed.

So I think the correct fix is as follows:

diff --git a/libelf/elf_compress_gnu.c b/libelf/elf_compress_gnu.c
index c35dc395..dfa7c571 100644
--- a/libelf/elf_compress_gnu.c
+++ b/libelf/elf_compress_gnu.c
@@ -80,7 +80,9 @@ elf_compress_gnu (Elf_Scn *scn, int inflate, unsigned int flags)
       sh_addralign = shdr->sh_addralign;
     }
 
-  if ((sh_flags & SHF_ALLOC) != 0)
+  /* Allocated sections, or sections that are already compressed
+     cannot (also) be GNU compressed.  */
+  if ((sh_flags & SHF_ALLOC) != 0 || (sh_flags & SHF_COMPRESSED))
     {
       __libelf_seterrno (ELF_E_INVALID_SECTION_FLAGS);
       return -1;

That way you could still (theoretically) gabi compress a gnu compressed section. But you wouldn't be able to gnu (de)compress a section that is already gabi compressed (unless you first [gabi] decompress it).
Comment 4 Mark Wielaard 2018-08-18 20:49:26 UTC
commit 56b18521fb8d46d40fc090c0de9d11a08bc982fa
Author: Mark Wielaard <mark@klomp.org>
Date:   Sat Aug 18 12:42:16 2018 +0200

    libelf: Return error if elf_compress_gnu is used on SHF_COMPRESSED section.
    
    Compressing a section that is already compressed is fine, but useless.
    But it isn't possible to gnu compress (or decompress) a SHF_COMPRESSED
    section since there is no state kept that would tell if the section was
    first GNU compressed or first gabi compressed. Calling elf_compress_gnu
    on a section and then calling elf_compress on it to decompress it twice
    could cause a crash (the other way around is fine). Just disallow it.
    
    https://sourceware.org/bugzilla/show_bug.cgi?id=23528
    
    Signed-off-by: Mark Wielaard <mark@klomp.org>
Comment 5 Mark Wielaard 2018-09-04 07:34:36 UTC
Apparently this bug got assigned CVE-2018-16402