Bug 21813 - Additional undefined behavior and crashes
Summary: Additional undefined behavior and crashes
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.30
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-21 17:32 UTC by Ned Williamson
Modified: 2021-10-24 11:36 UTC (History)
2 users (show)

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


Attachments
5 testcases with asan output (2.81 KB, application/x-xz)
2017-07-21 17:32 UTC, Ned Williamson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ned Williamson 2017-07-21 17:32:47 UTC
Created attachment 10281 [details]
5 testcases with asan output

I've found 5 more bugs in objdump master branch.

These bugs repro for me when building for all targets with a recent version of clang and AddressSanitizer and running them with `./objdump -xg testcase` (one crash requires debug output).

If any don't reproduce, let me know and I can try to identify another testcase or help identify the root cause. I've included `asan_output` with my stacktrace for each test.

I have investigated alpha-heap-overflow, and there the fix is to check that `PRIV (recrd.rec_size) > test_len` before reading the remaining record.

Cheers,
Ned
Comment 1 Sourceware Commits 2017-07-24 12:50:54 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

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

commit ca4cf9b9c622a5695e01f7f5815a7382a31fcf51
Author: Nick Clifton <nickc@redhat.com>
Date:   Mon Jul 24 13:49:22 2017 +0100

    Fix address violation errors parsing corrupt binary files.
    
    	PR 21813
    binutils* rddbg.c (read_symbol_stabs_debugging_info): Check for an empty
    	string whilst concatenating symbol names.
    
    bfd	* mach-o.c (bfd_mach_o_canonicalize_relocs): Pass the base address
    	of the relocs to the canonicalize_one_reloc routine.
    	* mach-o.h (struct bfd_mach_o_backend_data): Update the prototype
    	for the _bfd_mach_o_canonicalize_one_reloc field.
    	* mach-o-arm.c (bfd_mach_o_arm_canonicalize_one_reloc): Add
    	res_base parameter.  Use to check for corrupt pair relocs.
    	* mach-o-aarch64.c (bfd_mach_o_arm64_canonicalize_one_reloc):
    	Likewise.
    	* mach-o-i386.c (bfd_mach_o_i386_canonicalize_one_reloc):
    	Likewise.
    	* mach-o-x86-64.c (bfd_mach_o_x86_64_canonicalize_one_reloc):
    	Likewise.
    
    	* vms-alpha.c (_bfd_vms_slurp_eihd): Make sure that there is
    	enough data in the record before attempting to parse it.
    	(_bfd_vms_slurp_eeom): Likewise.
    
    	(_bfd_vms_slurp_egsd): Check for an invalid section index.
    	(image_set_ptr): Likewise.
    	(alpha_vms_slurp_relocs): Likewise.
Comment 2 Sourceware Commits 2017-07-24 13:05:21 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

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

commit 8a2df5e2df374289e00ecd8f099eb46d76ef982e
Author: Nick Clifton <nickc@redhat.com>
Date:   Mon Jul 24 14:04:04 2017 +0100

    Fix another memory access error triggered by attempting to parse a corrupt binary.
    
    	PR 21813
    	(alpha_vms_object_p): Check for a truncated record.
Comment 3 Nick Clifton 2017-07-24 13:07:31 UTC
Hi Ned,

  Thanks for the bug report.  I have checked in a couple of patches to 
  address the issues you reported.  (I had trouble reproducing the alpha-
  heap-overflow issue, which is why the patch for that test was checked
  in separately.  Your analysis of the bug was correct, although the
  correct thing to do was to jump to the error_ret label if the input 
  buffer was too small.

Cheers
  Nick
Comment 4 Sourceware Commits 2017-09-04 15:33:25 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=deeb3d27c254ad8bf8c3877fa6b61817f56191f5

commit deeb3d27c254ad8bf8c3877fa6b61817f56191f5
Author: Nick Clifton <nickc@redhat.com>
Date:   Mon Sep 4 16:31:12 2017 +0100

    Import patch from mainline to fix address violation errors when parsing corrupt VMS and MACHO binaries.
    
    	PR 21813
    	* mach-o.c (bfd_mach_o_canonicalize_relocs): Pass the base address
    	of the relocs to the canonicalize_one_reloc routine.
    	* mach-o.h (struct bfd_mach_o_backend_data): Update the prototype
    	for the _bfd_mach_o_canonicalize_one_reloc field.
    	* mach-o-arm.c (bfd_mach_o_arm_canonicalize_one_reloc): Add
    	res_base parameter.  Use to check for corrupt pair relocs.
    	* mach-o-aarch64.c (bfd_mach_o_arm64_canonicalize_one_reloc):
    	Likewise.
    	* mach-o-i386.c (bfd_mach_o_i386_canonicalize_one_reloc):
    	Likewise.
    	* mach-o-x86-64.c (bfd_mach_o_x86_64_canonicalize_one_reloc):
    	Likewise.
    	* vms-alpha.c (_bfd_vms_slurp_eihd): Make sure that there is
    	enough data in the record before attempting to parse it.
    	(_bfd_vms_slurp_eeom): Likewise.
    	(_bfd_vms_slurp_egsd): Check for an invalid section index.
    	(image_set_ptr): Likewise.
    	(alpha_vms_slurp_relocs): Likewise.
    	(alpha_vms_object_p): Check for a truncated record.
Comment 5 Sourceware Commits 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.
Comment 6 yzhan219 2021-07-02 15:27:54 UTC
(In reply to Ned Williamson from comment #0)
> Created attachment 10281 [details]
> 5 testcases with asan output
> 
> I've found 5 more bugs in objdump master branch.
> 
> These bugs repro for me when building for all targets with a recent version
> of clang and AddressSanitizer and running them with `./objdump -xg testcase`
> (one crash requires debug output).
> 
> If any don't reproduce, let me know and I can try to identify another
> testcase or help identify the root cause. I've included `asan_output` with
> my stacktrace for each test.
> 
> I have investigated alpha-heap-overflow, and there the fix is to check that
> `PRIV (recrd.rec_size) > test_len` before reading the remaining record.
> 
> Cheers,
> Ned

Hi Ned,

I tried to reproduce the crashed by using the test cases you provide. But none of them works, objdump says "objdump: debug_oob_read: File format not recognized". Could you please check if you have other test cases? Thank you so much.

Dennis
Comment 7 Sourceware Commits 2021-10-24 11:36:27 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit e02812494254b70fec6fa432f7f668956711133b
Author: Alan Modra <amodra@gmail.com>
Date:   Sun Oct 24 18:36:03 2021 +1030

    asan: arm-darwin: buffer overflow
    
            PR 21813
            * mach-o-arm.c (bfd_mach_o_arm_canonicalize_one_reloc): Sanity
            check PAIR reloc in other branch of condition as was done for
            PR21813.  Formatting.  Delete debug printf.