Bug 22113 - Heap out of bounds read in bfd_getl16()
Summary: Heap out of bounds read in bfd_getl16()
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.29
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-08 08:35 UTC by Kamil Frankowicz
Modified: 2018-03-31 12:30 UTC (History)
2 users (show)

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


Attachments
POC to trigger heap out of bounds read (objdump) (113 bytes, application/x-ms-dos-executable)
2017-09-08 08:35 UTC, Kamil Frankowicz
Details
New crashing test case - 2.30 (122 bytes, application/x-ms-dos-executable)
2018-03-13 09:23 UTC, Kamil Frankowicz
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kamil Frankowicz 2017-09-08 08:35:20 UTC
Created attachment 10404 [details]
POC to trigger heap out of bounds read (objdump)

After some fuzz testing I found a crashing test case.

Version: 2.29

Command: objdump -x -Wl -R -SD objdump_hoobr_bfd_getl16

ASAN:

==6445==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6270000188ff at pc 0x0000006626b9 bp 0x7fff0381c340 sp 0x7fff0381c338
READ of size 1 at 0x6270000188ff thread T0
    #0 0x6626b8 in bfd_getl16 XYZ/binutils-2.29/bfd/libbfd.c:505:27
    #1 0x857399 in pe_print_idata XYZ/binutils-2.29/bfd/peigen.c:1562:15
    #2 0x857399 in _bfd_pe_print_private_bfd_data_common XYZ/binutils-2.29/bfd/peigen.c:2904
    #3 0x83ea08 in pe_print_private_bfd_data XYZ/binutils-2.29/bfd/./peicode.h:336:8
    #4 0x4e9b57 in dump_bfd_private_header XYZ/binutils-2.29/binutils/./objdump.c:2920:3
    #5 0x4e9b57 in dump_bfd XYZ/binutils-2.29/binutils/./objdump.c:3504
    #6 0x4e8be1 in display_object_bfd XYZ/binutils-2.29/binutils/./objdump.c:3603:7
    #7 0x4e8be1 in display_any_bfd XYZ/binutils-2.29/binutils/./objdump.c:3692
    #8 0x4e7d5a in display_file XYZ/binutils-2.29/binutils/./objdump.c:3713:3
    #9 0x4e7d5a in main XYZ/binutils-2.29/binutils/./objdump.c:4015
    #10 0x7f3a3d3c182f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #11 0x419d98 in _start (XYZ/binutils-2.29/binutils/objdump+0x419d98)

0x6270000188ff is located 1 bytes to the left of 12336-byte region [0x627000018900,0x62700001b930)
allocated by thread T0 here:
    #0 0x4b85ac in malloc /home/llvm/clang-3.9/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
    #1 0x6618b3 in bfd_malloc XYZ/binutils-2.29/bfd/libbfd.c:193:9
    #2 0x856c9e in pe_print_idata XYZ/binutils-2.29/bfd/peigen.c:1388:8
    #3 0x856c9e in _bfd_pe_print_private_bfd_data_common XYZ/binutils-2.29/bfd/peigen.c:2904
    #4 0x83ea08 in pe_print_private_bfd_data XYZ/binutils-2.29/bfd/./peicode.h:336:8
    #5 0x4e8be1 in display_object_bfd XYZ/binutils-2.29/binutils/./objdump.c:3603:7
    #6 0x4e8be1 in display_any_bfd XYZ/binutils-2.29/binutils/./objdump.c:3692
    #7 0x4e7d5a in display_file XYZ/binutils-2.29/binutils/./objdump.c:3713:3
    #8 0x4e7d5a in main XYZ/binutils-2.29/binutils/./objdump.c:4015
    #9 0x7f3a3d3c182f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: heap-buffer-overflow XYZ/binutils-2.29/bfd/libbfd.c:505:27 in bfd_getl16
Shadow bytes around the buggy address:
  0x0c4e7fffb0c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4e7fffb0d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4e7fffb0e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4e7fffb0f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4e7fffb100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c4e7fffb110: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]
  0x0c4e7fffb120: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4e7fffb130: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4e7fffb140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4e7fffb150: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4e7fffb160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==6445==ABORTING
Comment 1 cvs-commit@gcc.gnu.org 2017-09-14 10:17:13 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

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

commit 4d465c689a8fb27212ef358d0aee89d60dee69a6
Author: Nick Clifton <nickc@redhat.com>
Date:   Thu Sep 14 11:15:55 2017 +0100

    Fix address violation when parsing a corrupt PE binary.
    
    	PR binutils/22113
    	* peXXigen.c (pe_print_idata): Extend check for HintName vector
    	entries.
Comment 2 cvs-commit@gcc.gnu.org 2017-09-14 10:28:52 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=dcaaca89e8618eba35193c27afcb1cfa54f74582

commit dcaaca89e8618eba35193c27afcb1cfa54f74582
Author: Nick Clifton <nickc@redhat.com>
Date:   Thu Sep 14 11:27:40 2017 +0100

    Import patch from mainline to fix an address violation when parsing a corrupt PE binary.
    
    	PR binutils/22113
    	* peXXigen.c (pe_print_idata): Extend check for HintName vector
    	entries.
Comment 3 Nick Clifton 2017-09-14 10:31:13 UTC
Hi Kamil,

  Thanks for reporting this problem.  There already was a test in the code
  that was meant to catch this kind of thing, but the test was not quite
  paranoid enough.  So I have created a patch to extend the check, and 
  committed it to the mainline and 2.29 branch sources.

Cheers
  Nick
Comment 4 Kamil Frankowicz 2018-03-13 09:07:54 UTC
Hi,

Problem still exists in 2.30:

==3183==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60f00000ec41 at pc 0x000000602aa5 bp 0x7ffe6cea6600 sp 0x7ffe6cea65f0
READ of size 1 at 0x60f00000ec41 thread T0
    #0 0x602aa4 in bfd_getl16 XYZ/binutils-2.30.0/bfd/libbfd.c:505
    #1 0x9777c8 in pex64_xdata_print_uwd_codes XYZ/binutils-2.30.0/bfd/pei-x86_64.c:241
    #2 0x9777c8 in pex64_dump_xdata XYZ/binutils-2.30.0/bfd/pei-x86_64.c:403
    #3 0x9777c8 in pex64_bfd_print_pdata_section XYZ/binutils-2.30.0/bfd/pei-x86_64.c:720
    #4 0x97887d in pex64_print_all_pdata_sections XYZ/binutils-2.30.0/bfd/pei-x86_64.c:745
    #5 0x61c56b in bfd_map_over_sections XYZ/binutils-2.30.0/bfd/section.c:1397
    #6 0x9787e9 in pex64_bfd_print_pdata XYZ/binutils-2.30.0/bfd/pei-x86_64.c:759
    #7 0x99abcd in _bfd_pex64_print_private_bfd_data_common XYZ/binutils-2.30.0/bfd/pex64igen.c:2908
    #8 0x963640 in pe_print_private_bfd_data XYZ/binutils-2.30.0/bfd/peicode.h:336
    #9 0x42009c in dump_bfd_private_header objdump.c:2966
    #10 0x42009c in dump_bfd objdump.c:3559
    #11 0x421a77 in display_object_bfd objdump.c:3658
    #12 0x421a77 in display_any_bfd objdump.c:3747
    #13 0x40ea81 in display_file objdump.c:3768
    #14 0x40ea81 in main objdump.c:4070
    #15 0x7fe1da1f782f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #16 0x411ca8 in _start (/usr/local/bin/objdump+0x411ca8)

0x60f00000ec41 is located 1 bytes to the right of 176-byte region [0x60f00000eb90,0x60f00000ec40)
allocated by thread T0 here:
    #0 0x7fe1da83d602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0x601d1a in bfd_malloc XYZ/binutils-2.30.0/bfd/libbfd.c:193
    #2 0x61200000bfb7  (<unknown module>)

SUMMARY: AddressSanitizer: heap-buffer-overflow XYZ/binutils-2.30.0/bfd/libbfd.c:505 bfd_getl16
Shadow bytes around the buggy address:
  0x0c1e7fff9d30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e7fff9d40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e7fff9d50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e7fff9d60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e7fff9d70: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c1e7fff9d80: 00 00 00 00 00 00 00 00[fa]fa fa fa fa fa fa fa
  0x0c1e7fff9d90: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1e7fff9da0: fd fd fd fd fd fd fa fa fa fa fa fa fa fa 00 00
  0x0c1e7fff9db0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1e7fff9dc0: 00 00 00 00 fa fa fa fa fa fa fa fa fd fd fd fd
  0x0c1e7fff9dd0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
Comment 5 Kamil Frankowicz 2018-03-13 09:23:02 UTC
Created attachment 10888 [details]
New crashing test case - 2.30
Comment 6 cvs-commit@gcc.gnu.org 2018-03-13 14:04:37 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

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

commit 3e33b239450771394fa6c83b67b9de80169f35e8
Author: Nick Clifton <nickc@redhat.com>
Date:   Tue Mar 13 14:02:52 2018 +0000

    Prevent memory access violations when attempting to parse an x86_64 PE binary containing corrupt unwind information.
    
    	PR 22113
    incldue	* coff/pe.h (struct pex64_unwind_info): Add a rawUnwindCodesEnd
    	field.
    
    bfd	* pei-x86_64.c (pex64_get_unwind_info): Change to a boolean
    	function.  Add an end address parameter.  Check access of the data
    	pointer to make sure that they do not extend beyond the end
    	address.  Return FALSE if any check fails.  Add the end address
    	pointer to the ui structure.
    	(pex64_xdata_print_uwd_codes): Check accesses of the raw unwind
    	codes to make sure that they do not extend beyond the end address
    	pointer.  Print an error message and return immediately if any
    	check fails.
Comment 7 Nick Clifton 2018-03-13 14:15:33 UTC
Hi Kamil,

  Thanks for reporting this problem.  It is actually a differnt bug, 
  albeit one in a similar area of code.

  I have checked in a patch that adds the necessary bounds checking,
  so this problem should now be resolved.

Cheers
  Nick