Bug 22887 - null pointer dereference in aout_32_swap_std_reloc_out
Summary: null pointer dereference in aout_32_swap_std_reloc_out
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.31
: P2 normal
Target Milestone: ---
Assignee: Alan Modra
URL:
Keywords:
: 23405 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-02-24 06:24 UTC by skysider
Modified: 2020-06-04 00:44 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed: 2018-02-28 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description skysider 2018-02-24 06:24:55 UTC
The test command is objcopy with specific elf file.
Below is part of gdb debugging output.

Program received signal SIGSEGV, Segmentation fault.
0x084cf65c in aout_32_swap_std_reloc_out (natptr=0xf590528c, g=0xf4b03fe8, abfd=<optimized out>) at /work/binutils-gdb/bfd/aoutx.h:1971
1971      asection *output_section = sym->section->output_section;
(gdb) bt
#0  0x084cf65c in aout_32_swap_std_reloc_out (natptr=0xf590528c, g=0xf4b03fe8, abfd=<optimized out>) at /work/binutils-gdb/bfd/aoutx.h:1971
#1  aout_32_squirt_out_relocs (abfd=0xf5b03970, section=0xf5903d48) at /work/binutils-gdb/bfd/aoutx.h:2444
#2  0x0849ae05 in i386linux_write_object_contents (abfd=0xf5b03970) at /work/binutils-gdb/bfd/i386linux.c:77
#3  0x081a9940 in bfd_close (abfd=0xf5b03970) at /work/binutils-gdb/bfd/opncls.c:731
#4  0x08080bbe in copy_file (input_filename=input_filename@entry=0xffffd8ef "out/slave/crashes/id:000125,sig:06,src:003346+002348,op:splice,rep:8", output_filename=output_filename@entry=0xf6500b80 "out/slave/crashes/stv31c0r", input_target=<optimized out>, 
    output_target=0x87f6320 "a.out-i386-linux", input_arch=0x0) at /work/binutils-gdb/binutils/objcopy.c:3530
#5  0x0805b429 in copy_main (argv=<optimized out>, argc=<optimized out>) at /work/binutils-gdb/binutils/objcopy.c:5478
#6  main (argc=2, argv=0xffffd7c4) at /work/binutils-gdb/binutils/objcopy.c:5582
(gdb) list
1966      asymbol *sym = *(g->sym_ptr_ptr);
1967      int r_extern;
1968      unsigned int r_length;
1969      int r_pcrel;
1970      int r_baserel, r_jmptable, r_relative;
1971      asection *output_section = sym->section->output_section;
1972
1973      PUT_WORD (abfd, g->address, natptr->r_address);
1974
1975      BFD_ASSERT (g->howto != NULL);
(gdb) p sym
$1 = (asymbol *) 0x0

It seems that there is lack of check if sym is null.
The test elf file is https://github.com/skysider/FuzzVuln/blob/master/binutils_objcopy_null_pointer_dereference_aout_32_swap_std_reloc_out.elf
Comment 1 skysider 2018-02-27 14:47:51 UTC
I compile binutils in 32bit machine.
Comment 2 Alan Modra 2018-02-28 06:43:10 UTC
It would be useful to know exactly how you configured binutils.  My 32-bit build with --enable-targets=all gives
binutils/objcopy:/home/alan/src/tmp/pr22887.bin: file format is ambiguous
binutils/objcopy: Matching formats: a.out-i386 a.out-i386-bsd a.out-i386-freebsd a.out-i386-lynx a.out-m88k-mach3 a.out-mips-little a.out-pc532-mach a.out-sparc-linux a.out-vax-bsd

So, using -F
binutils/objcopy -F a.out-i386 ~/src/tmp/pr22887.bin xxx
gives
binutils/objcopy:/home/alan/src/tmp/pr22887.bin: No such file or directory
which is a nonsense error, but not a segfault.

I'll be committing a patch to fix the nonsense error soon.
Comment 3 Sourceware Commits 2018-02-28 07:06:51 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 0d329c0a83a23cebb86fbe0ebddd780dc0df2424
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Feb 28 17:14:24 2018 +1030

    Nonsense error messages on invalid aout string offset
    
    translate_symbol_table returns false on detecting an out of range name
    string offset, hooray for error checking, but doesn't set bfd_error or
    print a useful error.  bfd_error therefore contains whatever it had
    previously, in my testing, bfd_error_system_call.  So the error
    printed depended on errno.
    
    	PR 22887
    	* aoutx.h (translate_symbol_table): Print an error message and set
    	bfd_error on finding an invalid name string offset.
Comment 4 skysider 2018-02-28 07:45:34 UTC
The command I use is "objcopy --debugging -p -D --keep-file-symbols $POC /tmp/1" and it still crashes.
Comment 5 Sourceware Commits 2018-02-28 11:51: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=116acb2c268c89c89186673a7c92620d21825b25

commit 116acb2c268c89c89186673a7c92620d21825b25
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Feb 28 22:09:50 2018 +1030

    PR22887, null pointer dereference in aout_32_swap_std_reloc_out
    
    	PR 22887
    	* aoutx.h (swap_std_reloc_in): Correct r_index bound check.
Comment 6 Alan Modra 2018-02-28 12:24:23 UTC
Fixed
Comment 7 rsprudencio 2018-04-26 14:56:21 UTC
Previous versions like binutils-2.27 have similar (if not identical) code without that fix, why only 2.30 was flagged as vulnerable? Could you elaborate?
Comment 8 Alan Modra 2018-04-26 23:24:33 UTC
Bugs that can only be triggered by incorrect input are not a high priority for backporting to previous versions of binutils.
Comment 9 rsprudencio 2018-04-27 09:05:06 UTC
It was published with misleading information, giving the idea that only 2.30 was vulnerable, the matter here is clarity of information and which versions were affected, I agree it is low priority, but NVD CVE page noting only 2.30 gives a really weak feedback to community which wishes to automate processes like identifying vulnerable products.
Comment 10 Alan Modra 2018-07-13 06:48:44 UTC
*** Bug 23405 has been marked as a duplicate of this bug. ***
Comment 11 Sourceware Commits 2020-06-04 00:44:15 UTC
The master branch has been updated by Stephen Casner <slcasner@sourceware.org>:

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

commit 31af1e68af26f5cae209de3530d0455b8a944b2d
Author: Stephen Casner <casner@acm.org>
Date:   Wed Jun 3 17:43:45 2020 -0700

    Copy several years of fixes from bfd/aoutx.h to bfd/pdp11.c.
    
    * pdp11.c (some_aout_object_p): 4c1534c7a2a - Don't set EXEC_P for
    files with relocs.
    (aout_get_external_symbols): 6b8f0fd579d - Return if count is zero.
    0301ce1486b PR 22306 - Handle stringsize of zero, and error for any
    other size that doesn't qcover the header word.
    bf82069dce1 PR 23056 - Allocate an extra byte at the end of the
    string table, and zero it.
    (translate_symbol_table): 0d329c0a83a PR 22887 - Print an error
    message and set bfd_error on finding an invalid name string offset.
    (add_to_stringtab): INLINE -> inline
    (pdp11_aout_swap_reloc_in): 116acb2c268 PR 22887 - Correct r_index
    bound check.
    (squirt_out_relocs): e2996cc315d PR 20921 - Check for and report
    any relocs that could not be recognised.
    92744f05809 PR 20929 - Check for relocs without an associated symbol.
    (find_nearest_line):  808346fcfcf PR 23055 - Check that the symbol
    name exists and is long enough, before attempting to see if it is
    for a .o file.
    c3864421222 - Correct case for N_SO being the last symbol.
    50455f1ab29 PR 20891 - Handle the case where the main file name
    and the directory name are both empty.
    e82ab856bb4 PR 20892 - Handle the case where function name is empty.
    (aout_link_add_symbols): e517df3dbf7 PR 19629 - Check for out of
    range string table offsets.
    531336e3a0b PR 20909 - Fix off-by-one error in check for an
    illegal string offset.
    (aout_link_includes_newfunc): Add comment.
    (pdp11_aout_link_input_section): ad756e3f9e6 - Return with an error
    on unexpected relocation type rather than ASSERT.