[Bug binutils/25306] New: Null Pointer Dereference in bfd/pef.c:bfd_pef_parse_symbols()

[v.manhnd at vincss dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=25306

Bug ID: 25306
   Summary: Null Pointer Dereference in
bfd/pef.c:bfd_pef_parse_symbols()
   Product: binutils
   Version: unspecified
Status: UNCONFIRMED
  Severity: normal
  Priority: P2
 Component: binutils
  Assignee: unassigned at sourceware dot org
  Reporter: v.manhnd at vincss dot net
  Target Milestone: ---

Created attachment 12139
  --> https://sourceware.org/bugzilla/attachment.cgi?id=12139&action=edit
The crash PoC

Hello,

There is a null pointer dereference in bfd/pef.c:bfd_pef_parse_symbols().

## Analysis
Look at the following code in bfd/pef.c:bfd_pef_parse_symbols():
--Code
  if (codesec != NULL)
{
  codelen = codesec->size;
  codebuf = bfd_malloc (codelen);
  if (bfd_seek (abfd, codesec->filepos, SEEK_SET) < 0)
goto end;
  if (bfd_bread ((void *) codebuf, codelen, abfd) != codelen)
goto end;
}
---
In the code above, codebuf is allocated without checking if the return pointer
is null, which makes the writing to codebuf by bfd_bread invalid.

## Reproduce
The attachment makes objdump crashes provided objdump is built in 32-bit.
--Log--
root@manh-ubuntu16:~/fuzz/fuzz_binutils# binutils-gdb-gcc-32/binutils/objdump
-x crash-objdump 

crash-objdump: file format pef
crash-objdump
architecture: powerpc:common64, flags 0x01ff:
HAS_RELOC, EXEC_P, HAS_LINENO, HAS_DEBUG, HAS_SYMS, HAS_LOCALS, DYNAMIC,
WP_TEXT, D_PAGED
start address 0x0630

Segmentation fault (core dumped)
---
Tested with version 39aa149769fd05fb6fade43bd41c1d7b6d63d06b of
github.com/bminor/binutils-gdb

--
Thanks & Regards,
Nguyen Duc Manh
VinCSS (a member of Vingroup)
[M] (+84) 346136886
[E] v.man...@vincss.net
[W]  www.vincss.net

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug binutils/25307] New: Heap-buffer-overflow in bfd/pef.c:bfd_pef_parse_function_stubs()

[v.manhnd at vincss dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=25307

Bug ID: 25307
   Summary: Heap-buffer-overflow in
bfd/pef.c:bfd_pef_parse_function_stubs()
   Product: binutils
   Version: unspecified
Status: UNCONFIRMED
  Severity: normal
  Priority: P2
 Component: binutils
  Assignee: unassigned at sourceware dot org
  Reporter: v.manhnd at vincss dot net
  Target Milestone: ---

Created attachment 12140
  --> https://sourceware.org/bugzilla/attachment.cgi?id=12140&action=edit
PoC

Hello,
There is a heap-buffer-overflow in bfd/pef.c:bfd_pef_parse_function_stubs()

## Analysis
Look at the following code:
--Code---
802:if ((codepos + 4) > codelen)
803:   break;
804:
805:ret = bfd_pef_parse_function_stub (abfd, codebuf + codepos, 24,
&sym_index);
--
At line 802, it checks if codepos + 4 < codelen. But at line 805,
bfd_pef_parse_function_stub reads (codebuf + codepos) with size 24. This could
pass the end of codebuf.

## Reproduction
The attachment makes objdump crash provided objdump is built with address
sanitizer.
--Version--
Tested with version 39aa149769fd05fb6fade43bd41c1d7b6d63d06b of
github.com/bminor/binutils-gdb
--Compilation--
root@manh-ubuntu16:~/fuzz/fuzz_binutils# ./configure --disable-gdb
--enable-targets=all CC=gcc CXX=g++ CFLAGS='-fsanitize=address -O0 -ggdb3'
CXXFLAGS='-fsanitize=address -O0 -ggdb3' && make -j4
Log Crash--
root@manh-ubuntu16:~/fuzz/fuzz_binutils#
./binutils-gdb-asan-64-O0/binutils/objdump -x crash-objdump3

crash-objdump3: file format pef
crash-objdump3
architecture: powerpc:common64, flags 0x01ff:
HAS_RELOC, EXEC_P, HAS_LINENO, HAS_DEBUG, HAS_SYMS, HAS_LOCALS, DYNAMIC,
WP_TEXT, D_PAGED
start address 0x01c0

=
==32000==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x6250e9e4 at pc 0x00837265 bp 0x7ffea4e6cf30 sp 0x7ffea4e6cf20
READ of size 1 at 0x6250e9e4 thread T0
#0 0x837264 in bfd_getb32
/root/fuzz/fuzz_binutils/binutils-gdb-asan-64-O0/bfd/libbfd.c:682
#1 0x12a204b in bfd_pef_parse_function_stub
/root/fuzz/fuzz_binutils/binutils-gdb-asan-64-O0/bfd/pef.c:709
#2 0x12a256a in bfd_pef_parse_function_stubs
/root/fuzz/fuzz_binutils/binutils-gdb-asan-64-O0/bfd/pef.c:805
#3 0x12a2f9a in bfd_pef_parse_symbols
/root/fuzz/fuzz_binutils/binutils-gdb-asan-64-O0/bfd/pef.c:929
#4 0x12a30ba in bfd_pef_count_symbols
/root/fuzz/fuzz_binutils/binutils-gdb-asan-64-O0/bfd/pef.c:951
#5 0x12a30d4 in bfd_pef_get_symtab_upper_bound
/root/fuzz/fuzz_binutils/binutils-gdb-asan-64-O0/bfd/pef.c:957
#6 0x4058a9 in slurp_symtab objdump.c:705
#7 0x414e52 in dump_bfd objdump.c:4037
#8 0x41557a in display_object_bfd objdump.c:4165
#9 0x41587a in display_any_bfd objdump.c:4255
#10 0x4158ef in display_file objdump.c:4276
#11 0x416a93 in main objdump.c:4603
#12 0x7f6f3f94382f in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#13 0x403838 in _start
(/root/fuzz/fuzz_binutils/binutils-gdb-asan-64-O0/binutils/objdump+0x403838)

0x6250e9e4 is located 0 bytes to the right of 8420-byte region
[0x6250c900,0x6250e9e4)
allocated by thread T0 here:
#0 0x7f6f3ff89602 in malloc
(/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
#1 0x8366e0 in bfd_malloc
/root/fuzz/fuzz_binutils/binutils-gdb-asan-64-O0/bfd/libbfd.c:275
#2 0x12a2d32 in bfd_pef_parse_symbols
/root/fuzz/fuzz_binutils/binutils-gdb-asan-64-O0/bfd/pef.c:899
#3 0x12a30ba in bfd_pef_count_symbols
/root/fuzz/fuzz_binutils/binutils-gdb-asan-64-O0/bfd/pef.c:951
#4 0x12a30d4 in bfd_pef_get_symtab_upper_bound
/root/fuzz/fuzz_binutils/binutils-gdb-asan-64-O0/bfd/pef.c:957
#5 0x4058a9 in slurp_symtab objdump.c:705
#6 0x414e52 in dump_bfd objdump.c:4037
#7 0x41557a in display_object_bfd objdump.c:4165
#8 0x41587a in display_any_bfd objdump.c:4255
#9 0x4158ef in display_file objdump.c:4276
#10 0x416a93 in main objdump.c:4603
#11 0x7f6f3f94382f in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: heap-buffer-overflow
/root/fuzz/fuzz_binutils/binutils-gdb-asan-64-O0/bfd/libbfd.c:682 bfd_getb32
Shadow bytes around the buggy address:
  0x0c4a7fff9ce0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a7fff9cf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a7fff9d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a7fff9d10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a7fff9d20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c4a7fff9d30: 00 00 00 00 00 00 00 00 00 00 00 00[04]fa fa fa
  0x0c4a7fff9d40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4a7fff9d50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4a7fff9d60: fa fa fa fa fa fa fa fa fa fa fa fa fa

[Bug binutils/25308] New: Multiple null pointer dereferencs in bfd module due to not checking return value of bfd_malloc

[v.manhnd at vincss dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=25308

Bug ID: 25308
   Summary: Multiple null pointer dereferencs in bfd module due to
not checking return value of bfd_malloc
   Product: binutils
   Version: unspecified
Status: UNCONFIRMED
  Severity: normal
  Priority: P2
 Component: binutils
  Assignee: unassigned at sourceware dot org
  Reporter: v.manhnd at vincss dot net
  Target Milestone: ---

Created attachment 12141
  --> https://sourceware.org/bugzilla/attachment.cgi?id=12141&action=edit
Crashes due to not checking return value of bfd_malloc

There are multiple null pointer dereferences in bfd module due to calling
bfd_malloc without checking if the return pointer is null.

## Analysis
Here is the list of vulnerable issues:
### Issue 1
In bfd/pef.c:bfd_pef_scan_start_address(), loaderbuf is malloced without
checking if the return pointer is null. Later, loaderbuf is feeded to bfd_read
=> Null pointer derefence.
--
  loaderbuf = bfd_malloc (loaderlen);
  if (bfd_seek (abfd, loadersec->filepos, SEEK_SET) < 0)
goto error;
  if (bfd_bread ((void *) loaderbuf, loaderlen, abfd) != loaderlen)
goto error;
--
### Issue 2, 3
In bfd/pef.c:bfd_pef_parse_function_stubs(), libraries and imports are malloced
without checking the return values. Later, they are passed to
bfd_pef_parse_imported_library and bfd_pef_parse_imported_symbol:
--
  libraries = bfd_malloc
(header.imported_library_count * sizeof (bfd_pef_imported_library));
  imports = bfd_malloc
(header.total_imported_symbol_count * sizeof (bfd_pef_imported_symbol));
...
  ret = bfd_pef_parse_imported_library
(abfd, loaderbuf + 56 + (i * 24), 24, &libraries[i]);
...
  ret = (bfd_pef_parse_imported_symbol
 (abfd,
  loaderbuf + 56 + (header.imported_library_count * 24) + (i * 4),
  4, &imports[i]));
--
### Issue 4, 5
In bfd, pef.c:bfd_pef_parse_symbols(), codebuf and loadersec are malloced
without checking the return values. Later, they are passed to bfd_read:
--
  codesec = bfd_get_section_by_name (abfd, "code");
  if (codesec != NULL)
{
  codelen = codesec->size;
  codebuf = bfd_malloc (codelen);
  if (bfd_seek (abfd, codesec->filepos, SEEK_SET) < 0)
goto end;
  if (bfd_bread ((void *) codebuf, codelen, abfd) != codelen)
goto end;
}

  loadersec = bfd_get_section_by_name (abfd, "loader");
  if (loadersec != NULL)
{
  loaderlen = loadersec->size;
  loaderbuf = bfd_malloc (loaderlen);
  if (bfd_seek (abfd, loadersec->filepos, SEEK_SET) < 0)
goto end;
  if (bfd_bread ((void *) loaderbuf, loaderlen, abfd) != loaderlen)
goto end;
}
---
### Issue 6
In bfd/pef.c:bfd_pef_print_loader_section(), loaderbuf is malloced without
checking the return value. Later, loaderbuf is passed to bfd_bread:
---
  loaderbuf = bfd_malloc (loaderlen);

  if (bfd_seek (abfd, loadersec->filepos, SEEK_SET) < 0
  || bfd_bread ((void *) loaderbuf, loaderlen, abfd) != loaderlen
  || loaderlen < 56
  || bfd_pef_parse_loader_header (abfd, loaderbuf, 56, &header) < 0)
{
  free (loaderbuf);
  return -1;
}
---
### Issue 7
In bfd/mach-o.c:bfd_mach_o_core_fetch_environment(), dsym_filename is passed to
sprintf:
---
  dsym_filename = (char *)bfd_malloc (strlen (base_bfd->filename)
   + strlen (dsym_subdir) + 1
   + strlen (base_basename) + 1);
  sprintf (dsym_filename, "%s%s/%s",
   base_bfd->filename, dsym_subdir, base_basename);

---
### Issue 8
In bfd/elf-properties.c/_bfd_elf_convert_gnu_properties(), contents is malloced
without checking the return value. Later, contents is passed to
elf_write_gnu_properties:
---
  if (size > bfd_section_size (isec))
{
  contents = (bfd_byte *) bfd_malloc (size);
  free (*ptr);
  *ptr = contents;
}
  else
contents = *ptr;

  *ptr_size = size;

  /* Generate the output .note.gnu.property section.  */
  elf_write_gnu_properties (ibfd, contents, list, size, 1 << align_shift);
---
### Issue 9
In bfd/elf32-arm.c:bfd_elf32_arm_vfp11_fix_veneer_locations(), tmp_name is
malloced without checking the return value. Later, tmp_name is passed to
sprintf:
---
  tmp_name = (char *) bfd_malloc ((bfd_size_type) strlen
  (VFP11_ERRATUM_VENEER_ENTRY_NAME) + 10);
...
  sprintf (tmp_name, VFP11