Bug 24084

Summary: Negative-size-param when when calling memcpy function in elf_cvt_note function in libelf
Product: elfutils Reporter: wcventure <wcventure>
Component: backendsAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: elfutils-devel, mark
Priority: P2    
Version: unspecified   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:
Attachments: POC

Description wcventure 2019-01-11 06:08:30 UTC
Created attachment 11530 [details]
POC

Hi there,

Negative-size-param when calling memcpy function in elf_cvt_note function in libelf the latest elfutils-0.174 code base, this inputs will cause the segment faults and I have confirmed them with address sanitizer too. 

Please use the ".//eu-elflint -d $POC"to reproduce the bug. If you have any questions, please let me know.

git log

> commit 1dabad36ee28aa76b8cf14b6426b379cabee6def
> Author: Jim Wilson <jimw@sifive.com>
> Date:   Thu Dec 27 15:25:49 2018 -0800
> 
>     RISC-V: Improve riscv64 core file support.
> 
>     This fixes two problems.  The offset for x1 is changed from 1 to 8 because
>     this is a byte offset not a register skip count.  Support for reading the
>     PC value is added.  This requires changing the testsuite to match the new
>     readelf output for coredumps.
> 
>     Signed-off-by: Jim Wilson <jimw@sifive.com>

The ASAN dumps the stack trace as follows:

> =================================================================
> ==24780==ERROR: AddressSanitizer: negative-size-param: (size=-4)
>     #0 0x7f23f4234853  (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x79853)
>     #1 0x7f23f3edaa2c in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
>     #2 0x7f23f3edaa2c in elf_cvt_note /elfutils/libelf/note_xlate.h:63
>     #3 0x7f23f3edaa2c in elf_cvt_note4 /elfutils/libelf/note_xlate.h:79
>     #4 0x7f23f3f2ed30 in convert_data /elfutils/libelf/elf_getdata.c:204
>     #5 0x7f23f3f2ed30 in __libelf_set_data_list_rdlock /elfutils/libelf/elf_getdata.c:447
>     #6 0x7f23f3f301bf in __elf_getdata_rdlock /elfutils/libelf/elf_getdata.c:554
>     #7 0x469a22 in check_note_section /elfutils/src/elflint.c:4428
>     #8 0x469a22 in check_sections /elfutils/src/elflint.c:4182
>     #9 0x47a222 in process_elf_file /elfutils/src/elflint.c:4774
>     #10 0x47a222 in process_file /elfutils/src/elflint.c:242
>     #11 0x4030d5 in main /elfutils/src/elflint.c:175
>     #12 0x7f23f38d182f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
>     #13 0x404718 in _start (/elfutils/build/bin/eu-elflint+0x404718)
> 
> Address 0x7f23f52b3b30 is a wild pointer.
> SUMMARY: AddressSanitizer: negative-size-param (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x79853)
> ==24780==ABORTING
Comment 1 Mark Wielaard 2019-01-16 11:35:38 UTC
(In reply to wcventure from comment #0)
> Negative-size-param when calling memcpy function in elf_cvt_note function in
> libelf the latest elfutils-0.174 code base, this inputs will cause the
> segment faults and I have confirmed them with address sanitizer too. 

Nice find. Replicated under valgrind with the reproducer.
The root cause is a wrong overflow check.
The code wanted to make sure we had at least enough room for the header,
but got the size of the header wrong. It had hardcoded the size as 8 bytes,
but it should have been 12 bytes.

Fixed as follows:

commit e65d91d21cb09d83b001fef9435e576ba447db32
Author: Mark Wielaard <mark@klomp.org>
Date:   Wed Jan 16 12:25:57 2019 +0100

    libelf: Correct overflow check in note_xlate.
    
    We want to make sure the note_len doesn't overflow and becomes shorter
    than the note header. But the namesz and descsz checks got the note header
    size wrong). Replace the wrong constant (8) with a sizeof cvt_Nhdr (12).
    
    https://sourceware.org/bugzilla/show_bug.cgi?id=24084
    
    Signed-off-by: Mark Wielaard <mark@klomp.org>

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 5923c85..5783f0c 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2019-01-16  Mark Wielaard  <mark@klomp.org>
+
+       * note_xlate.h (elf_cvt_note): Check n_namesz and n_descsz don't
+       overflow note_len into note header.
+
 2018-11-17  Mark Wielaard  <mark@klomp.org>
 
        * elf32_updatefile.c (updatemmap): Make sure to call convert
diff --git a/libelf/note_xlate.h b/libelf/note_xlate.h
index 9bdc3e2..bc9950f 100644
--- a/libelf/note_xlate.h
+++ b/libelf/note_xlate.h
@@ -46,13 +46,13 @@ elf_cvt_note (void *dest, const void *src, size_t len, int encode,
       /* desc needs to be aligned.  */
       note_len += n->n_namesz;
       note_len = nhdr8 ? NOTE_ALIGN8 (note_len) : NOTE_ALIGN4 (note_len);
-      if (note_len > len || note_len < 8)
+      if (note_len > len || note_len < sizeof *n)
        break;
 
       /* data as a whole needs to be aligned.  */
       note_len += n->n_descsz;
       note_len = nhdr8 ? NOTE_ALIGN8 (note_len) : NOTE_ALIGN4 (note_len);
-      if (note_len > len || note_len < 8)
+      if (note_len > len || note_len < sizeof *n)
        break;
 
       /* Copy or skip the note data.  */
Comment 2 Mark Wielaard 2019-02-10 12:10:05 UTC
CVE-2019-7664