Bug 22895 - integer overflow in read_attribute_value
Summary: integer overflow in read_attribute_value
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.31
: P2 normal
Target Milestone: ---
Assignee: Nick Clifton
Depends on:
Reported: 2018-02-26 03:43 UTC by skysider
Modified: 2018-03-31 12:34 UTC (History)
2 users (show)

See Also:
Last reconfirmed: 2018-02-27 00:00:00

Proposed patch (581 bytes, patch)
2018-02-27 15:01 UTC, Nick Clifton
Details | Diff
Proposed patch (638 bytes, patch)
2018-02-28 10:27 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description skysider 2018-02-26 03:43:25 UTC
The command I test is "nm-new -A -a -l -S -s --special-syms --synthetic --with-symbol-versions -D $POC".

In function read_attribute_value in dwarf2.c:1175:

case DW_FORM_block:
      amt = sizeof (struct dwarf_block);
      blk = (struct dwarf_block *) bfd_alloc (abfd, amt);
      if (blk == NULL)
	return NULL;
      blk->size = _bfd_safe_read_leb128 (abfd, info_ptr, &bytes_read,
					 FALSE, info_ptr_end);
      info_ptr += bytes_read;
      blk->data = read_n_bytes (abfd, info_ptr, info_ptr_end, blk->size);
      info_ptr += blk->size;
      attr->u.blk = blk;

I find a case where blk->size is large enough to lead to integer overflow of info_ptr.
The POC file is https://github.com/skysider/FuzzVuln/blob/master/binutils_nm_integer_overflow_read_attribute_value.elf
Comment 1 skysider 2018-02-27 14:52:20 UTC
Compile binutils in 32 bit machine.
Comment 2 Nick Clifton 2018-02-27 15:01:07 UTC
Created attachment 10857 [details]
Proposed patch

Hi Skysider,

  Please could you try out the uploaded patch and let me know if it
  resolves the problem for you.

Comment 3 skysider 2018-02-27 15:04:35 UTC
Sure, I will test it a few hours later (it's night in my region)
Comment 4 skysider 2018-02-28 03:01:06 UTC
The patch doesn't work for the crash corpus. Part of my gdb debugging:

(gdb) list
1186          if (blk == NULL)
1187            return NULL;
1188          blk->size = _bfd_safe_read_leb128 (abfd, info_ptr, &bytes_read,
1189                                             FALSE, info_ptr_end);
1190          info_ptr += bytes_read;
1191          info_ptr = read_n_bytes (abfd, info_ptr, info_ptr_end, blk);
1192          attr->u.blk = blk;
1193          break;
1194        case DW_FORM_block1:
1195          amt = sizeof (struct dwarf_block);
(gdb) p/x *blk
$20 = {size = 0xf7e7efd6, data = 0x0}
(gdb) p/x info_ptr
$21 = 0x81a073c
(gdb) p/x info_ptr_end
$23 = 0x81a101a

So I think check if(size > end) is necessary.
Comment 5 Nick Clifton 2018-02-28 10:27:39 UTC
Created attachment 10859 [details]
Proposed patch

Hi Skysider,

  Ah - you are right, I had not considered underflow.  Please could you
  try out this revised patch instead ?


PS.  I have discovered that I am unable to build 32-bit toolchains with
sanitization enabled as there is a kernel/asan bug which stops sanitization
from working. :-(
Comment 6 skysider 2018-02-28 11:35:04 UTC
The patch works. And the command I compile binutils is

configure --disable-shared LIBS=-ldl LDFLAGS=-ldl --disable-64-bit-bfd CFLAGS="-g -O0 -fsanitize=address" CXXFLAGS="-g -O0 -fsanitize=address"
make all-binutils

operating system : ubuntu 16.04 LTS  32 bit
Comment 7 cvs-commit@gcc.gnu.org 2018-02-28 11:54:07 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:


commit 12c963421d045a127c413a0722062b9932c50aa9
Author: Nick Clifton <nickc@redhat.com>
Date:   Wed Feb 28 11:50:49 2018 +0000

    Catch integer overflows/underflows when parsing corrupt DWARF FORM blocks.
    	PR 22895
    	PR 22893
    	* dwarf2.c (read_n_bytes): Replace size parameter with dwarf_block
    	pointer.  Drop unused abfd parameter.  Check the size of the block
    	before initialising the data field.  Return the end pointer if the
    	size is invalid.
    	(read_attribute_value): Adjust invocations of read_n_bytes.
Comment 8 Nick Clifton 2018-02-28 11:54:48 UTC
Patch applied.