Bug 21990

Summary: Integer overflow in process_version_sections (readelf.c)
Product: binutils Reporter: Mạnh <Imdb95>
Component: binutilsAssignee: Alan Modra <amodra>
Status: RESOLVED FIXED    
Severity: normal CC: amodra
Priority: P2    
Version: 2.29   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed: 2017-08-23 00:00:00
Attachments: Crafted elf file used to trigger the bug

Description Mạnh 2017-08-22 14:27:20 UTC
Created attachment 10358 [details]
Crafted elf file used to trigger the bug

Hello,
I found this bug when fuzzing readelf with afl-fuzz.
==========Reproduce==========
manh@manh-VirtualBox:~/Fuzzing/afl/binutils/binutils-2.29$ sudo ./configure --prefix=`pwd`/../build-binutils-2.29-ggdb CC="gcc" CXX="g++" CFLAGS="-ldl -Wno-error -ggdb -O0" CXXFLAGS="-ldl -Wno-error -ggdb -O0" && sudo make && sudo make install
Trigger the bug:
manh@manh-VirtualBox:~/Fuzzing/afl/binutils$ ./build-binutils-2.29-ggdb/bin/readelf -a readefl_hang.elf 
==========Actual Result==========
The program readelf hangs for a very long time, printing repeated outputs.
manh@manh-VirtualBox:~/Fuzzing/afl/binutils$ ./build-binutils-2.29-ggdb/bin/readelf -a readefl_hang.elf 
ELF Header:
  Magic:   7f 45 4c 46 00 02 00 00 00 00 00 00 00 00 00 40 
  Class:                             none
......................
  0x0080:   Name index: 0  Flags: none  Version: 0
readelf: Warning: Invalid vna_next field of ffffff80
  0x0040: Version: 0  File: 0  Cnt: 0
  0x0080: Version: 0  File: 0  Cnt: 0
  000000: Version: 32581  File: 20000  Cnt: 19526
  000000:   Name index: 0  Flags: WEAK  Version: 0
  0x0040:   Name index: 0  Flags: none  Version: 0
  0x0080:   Name index: 0  Flags: none  Version: 0
readelf: Warning: Invalid vna_next field of ffffff80
......................
==========Build Date & Hardware==========
Version: binutils 2.29 (https://ftp.gnu.org/gnu/binutils/binutils-2.29.tar.gz)
Compilation on Ubuntu 16.04:
manh@manh-VirtualBox:~/Fuzzing/afl/binutils/binutils-2.29$ uname -a
Linux manh-VirtualBox 4.4.0-91-generic #114-Ubuntu SMP Tue Aug 8 11:56:56 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
==========Additional Information==========
Detailed analysis of the bug:
At readelf.c:10388, idx += ent.vn_next. This triggers integer overflow, with suitable value of ent.vn_next. With the crafted readelf_hang.elf above, the for loop at readelf.c:10304 iterates as following (set breakpoint at readelf.c:10327 and examine idx, ent.vn_next,...):
   + Loop 0: idx = 0; ent.vn_next = 64
   + Loop 1: idx = 64; ent.vn_next = 64
   + Loop 2: idx = 128; ent.vn_next = 4294967168
   + Loop 3: idx = 0; ent.vn_next = 64
   + Loop 4: idx = 64; ent.vn_next = 64
   + Loop 5: idx = 128; ent.vn_next = 4294967168
   + Loop 6: idx = 0; ent.vn_next = 64
   + Loop 7: idx = 64; ent.vn_next = 64
   + Loop 8: idx = 128; ent.vn_next = 4294967168
......................
When idx = 128, ent.vn_next = 4294967168, the expression idx + ent.vn_next gets 0 => idx += ent.vn_next gets overflow. So the loop would not break at line readelf.c:10312
		if (idx > (size_t) (endbuf - (char *) eneed))
		  break;
and it would iterate until cnt gets equals to section->sh_info. With readelf_hang.elf, section->sh_info = 1441792, so it iterates for 1441792 times.
==========Suggestion for Patching==========
Add the following line before line readelf.c:10388 
     if (idx + ent.vn_next < idx) break;

Cheers,
  Manh
Comment 1 cvs-commit@gcc.gnu.org 2017-08-23 10:19:53 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=452bf675ea772002aa86fb1d28f3474da70ee1de

commit 452bf675ea772002aa86fb1d28f3474da70ee1de
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Aug 23 15:42:12 2017 +0930

    PR21990, Integer overflow in process_version_sections
    
    This tidies some of the overflow checking when processing verneed
    and verdef sections.
    
    	PR 21990
    	* readelf.c (process_version_sections <SHT_GNU_verneed>): Check
    	for invalid vn_next field before adding to idx.  Use unsigned
    	long for index vars.  Move index checks.
    	<SHT_GNU_verdef>: Likewise for vd_next.
Comment 2 Alan Modra 2017-08-23 10:22:48 UTC
Fixed
Comment 3 cvs-commit@gcc.gnu.org 2017-09-05 14:33:26 UTC
The binutils-2_29-branch branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=64aa1246572306b72dc479b46d13ff749b0c3236

commit 64aa1246572306b72dc479b46d13ff749b0c3236
Author: Nick Clifton <nickc@redhat.com>
Date:   Tue Sep 5 15:32:04 2017 +0100

    Import patches from mainline to fix minor binutils bugs:
    
    	PR 21861
    	* winduni.c (codepages): Use cp1252 for codepage 0.
    
    	PR 21813
    	* rddbg.c (read_symbol_stabs_debugging_info): Check for an empty
    	string whilst concatenating symbol names.
    
    	PR 21909
    	* prdbg.c (pr_int_type): Increase size of local string buffer.
    	(pr_float_type): Likewise.
    	(pr_bool_type): Likewise.
    
    	PR 21820
    	* readelf.c (dump_section_as_strings): Do not fail if the section
    	was empty.
    	(dump_section_as_bytes): Likewise.
    
    	PR 21990
    	* readelf.c (process_version_sections <SHT_GNU_verneed>): Check
    	for invalid vn_next field before adding to idx.  Use unsigned
    	long for index vars.  Move index checks.
    	<SHT_GNU_verdef>: Likewise for vd_next.
    
    	PR 21994
    	* readelf.c (process_version_sections <SHT_GNU_verdef>): Check
    	vd_aux and vda_next for sanity.  Delete "end".  Correct overflow
    	checks.
    	(process_version_sections <SHT_GNU_verneed>): Correct overflow
    	check.  Don't report invalid vna_next on overflow.  Do report
    	invalid vna_next on size less than aux info.