Bug 24100 - An unsigned integer overflow which may cause huge amount of heap allocation in readelf
Summary: An unsigned integer overflow which may cause huge amount of heap allocation i...
Status: RESOLVED INVALID
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.31
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-17 02:47 UTC by poppeter1982
Modified: 2019-01-18 03:43 UTC (History)
1 user (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description poppeter1982 2019-01-17 02:47:29 UTC
Hi There

Peng Li and Shengjian Guo at Baidu X-Lab discovered a suspicious unsigned integer overflow which may lead to a huge amount of memory allocation in heap. 

In get_dynamic_data of readelf.c of version 2.31.51.20190117

static bfd_vma *
get_dynamic_data (Filedata * filedata, bfd_size_type number, unsigned int ent_size)
{       
    ...
   
    if (ent_size * number > filedata->file_size)
    { 
      error (_("Invalid number of dynamic entries: %s\n"),
             bfd_vmatoa ("u", number));
      return NULL;
    }
  
    e_data = (unsigned char *) cmalloc ((size_t) number, ent_size);

    ...
}

If you compile readelf with -fsanitize=unsigned-integer-overflow and run ./readelf -a input, it is reported that readelf.c:11251:16: runtime error: unsigned integer overflow: 8 * 5765762010251921410 cannot be represented in type 'unsigned long'. With regards to this input, the wrap around result of ent_size * number is still greater than file_size (2413), so the variable "number" that is 5765762010251921410 does not flow into cmalloc operation. 

However, let's think about the case where number is still a huge number and 8*huge_number is less than file_size, then consequently a huge amount of memory will be allocated in heap. One possible solution to resolve the multiplication overflow is to use division instead, for example, the conditional is changed to if (ent_size > filedata->file_size/number). 

If you have any questions about this issue and input in the attachment, please let me know.

Thanks
Peng
Comment 1 Alan Modra 2019-01-18 03:43:26 UTC
Not a bug.  Any overflow here will be caught inside cmalloc.