This is the mail archive of the
binutils-cvs@sourceware.org
mailing list for the binutils project.
[binutils-gdb] Fixes for invalid memory accesses triggered by running windres on corrupt binaries.
- From: Nick Clifton <nickc at sourceware dot org>
- To: bfd-cvs at sourceware dot org
- Date: 27 Jan 2015 17:34:30 -0000
- Subject: [binutils-gdb] Fixes for invalid memory accesses triggered by running windres on corrupt binaries.
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0897ec15810bca3420ea7b8a91e491ed45780202
commit 0897ec15810bca3420ea7b8a91e491ed45780202
Author: Nick Clifton <nickc@redhat.com>
Date: Tue Jan 27 17:32:23 2015 +0000
Fixes for invalid memory accesses triggered by running windres on corrupt binaries.
PR binutils/17512
* rcparse.y: Add checks to avoid integer divide by zero.
* rescoff.c (read_coff_rsrc): Add check on the size of the
resource section.
(read_coff_res_dir): Add check on the nesting level.
Check for resource names overrunning the buffer.
* resrc.c (write_rc_messagetable): Update formatting.
Add check of 'elen' being zero.
Diff:
---
binutils/ChangeLog | 8 +++++
binutils/rcparse.y | 9 ++---
binutils/rescoff.c | 22 ++++++++++--
binutils/resrc.c | 102 +++++++++++++++++++++++++++++------------------------
4 files changed, 89 insertions(+), 52 deletions(-)
diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 53ec072..924f35c 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -7,6 +7,14 @@
* addr2line.c (slurp_symtab): If the symcount is zero, free the
symbol table pointer.
+ * rcparse.y: Add checks to avoid integer divide by zero.
+ * rescoff.c (read_coff_rsrc): Add check on the size of the
+ resource section.
+ (read_coff_res_dir): Add check on the nesting level.
+ Check for resource names overrunning the buffer.
+ * resrc.c (write_rc_messagetable): Update formatting.
+ Add check of 'elen' being zero.
+
2015-01-23 Nick Clifton <nickc@redhat.com>
* nlmconv.c (powerpc_mangle_relocs): Fix build errors introduced
diff --git a/binutils/rcparse.y b/binutils/rcparse.y
index 78ac57e..0cf6c2c 100644
--- a/binutils/rcparse.y
+++ b/binutils/rcparse.y
@@ -1887,12 +1887,12 @@ sizednumexpr:
}
| sizednumexpr '/' sizednumexpr
{
- $$.val = $1.val / $3.val;
+ $$.val = $1.val / ($3.val ? $3.val : 1);
$$.dword = $1.dword || $3.dword;
}
| sizednumexpr '%' sizednumexpr
{
- $$.val = $1.val % $3.val;
+ $$.val = $1.val % ($3.val ? $3.val : 1);
$$.dword = $1.dword || $3.dword;
}
| sizednumexpr '+' sizednumexpr
@@ -1966,12 +1966,13 @@ sizedposnumexpr:
}
| sizedposnumexpr '/' sizednumexpr
{
- $$.val = $1.val / $3.val;
+ $$.val = $1.val / ($3.val ? $3.val : 1);
$$.dword = $1.dword || $3.dword;
}
| sizedposnumexpr '%' sizednumexpr
{
- $$.val = $1.val % $3.val;
+ /* PR 17512: file: 89105a25. */
+ $$.val = $1.val % ($3.val ? $3.val : 1);
$$.dword = $1.dword || $3.dword;
}
| sizedposnumexpr '+' sizednumexpr
diff --git a/binutils/rescoff.c b/binutils/rescoff.c
index 0004c3a..3e63e49 100644
--- a/binutils/rescoff.c
+++ b/binutils/rescoff.c
@@ -142,8 +142,14 @@ read_coff_rsrc (const char *filename, const char *target)
set_windres_bfd (&wrbfd, abfd, sec, WR_KIND_BFD);
size = bfd_section_size (abfd, sec);
- data = (bfd_byte *) res_alloc (size);
+ /* PR 17512: file: 1b25ba5d
+ The call to get_file_size here may be expensive
+ but there is no other way to determine if the section size
+ is reasonable. */
+ if (size > (bfd_size_type) get_file_size (filename))
+ fatal (_("%s: .rsrc section is bigger than the file!"), filename);
+ data = (bfd_byte *) res_alloc (size);
get_windres_bfd_content (&wrbfd, data, 0, size);
flaginfo.filename = filename;
@@ -185,6 +191,13 @@ read_coff_res_dir (windres_bfd *wrbfd, const bfd_byte *data,
rc_res_entry **pp;
const struct extern_res_entry *ere;
+ /* PR 17512: file: 09d80f53.
+ Whilst in theory resources can nest to any level, in practice
+ Microsoft only defines 3 levels. Corrupt files however might
+ claim to use more. */
+ if (level > 4)
+ overrun (flaginfo, _("Resources nest too deep"));
+
if ((size_t) (flaginfo->data_end - data) < sizeof (struct extern_res_directory))
overrun (flaginfo, _("directory"));
@@ -234,7 +247,12 @@ read_coff_res_dir (windres_bfd *wrbfd, const bfd_byte *data,
re->id.u.n.length = length;
re->id.u.n.name = (unichar *) res_alloc (length * sizeof (unichar));
for (j = 0; j < length; j++)
- re->id.u.n.name[j] = windres_get_16 (wrbfd, ers + j * 2 + 2, 2);
+ {
+ /* PR 17512: file: 05dc4a16. */
+ if (length < 0 || ers >= (bfd_byte *) ere || ers + j * 2 + 4 >= (bfd_byte *) ere)
+ overrun (flaginfo, _("resource name"));
+ re->id.u.n.name[j] = windres_get_16 (wrbfd, ers + j * 2 + 2, 2);
+ }
if (level == 0)
type = &re->id;
diff --git a/binutils/resrc.c b/binutils/resrc.c
index 4e0b24c..f0cacd1 100644
--- a/binutils/resrc.c
+++ b/binutils/resrc.c
@@ -2932,53 +2932,63 @@ write_rc_messagetable (FILE *e, rc_uint_type length, const bfd_byte *data)
if (length < BIN_MESSAGETABLE_SIZE)
has_error = 1;
else
- do {
- rc_uint_type m, i;
- mt = (const struct bin_messagetable *) data;
- m = windres_get_32 (&wrtarget, mt->cblocks, length);
- if (length < (BIN_MESSAGETABLE_SIZE + m * BIN_MESSAGETABLE_BLOCK_SIZE))
- {
- has_error = 1;
- break;
- }
- for (i = 0; i < m; i++)
- {
- rc_uint_type low, high, offset;
- const struct bin_messagetable_item *mti;
+ do
+ {
+ rc_uint_type m, i;
+
+ mt = (const struct bin_messagetable *) data;
+ m = windres_get_32 (&wrtarget, mt->cblocks, length);
+
+ if (length < (BIN_MESSAGETABLE_SIZE + m * BIN_MESSAGETABLE_BLOCK_SIZE))
+ {
+ has_error = 1;
+ break;
+ }
+ for (i = 0; i < m; i++)
+ {
+ rc_uint_type low, high, offset;
+ const struct bin_messagetable_item *mti;
+
+ low = windres_get_32 (&wrtarget, mt->items[i].lowid, 4);
+ high = windres_get_32 (&wrtarget, mt->items[i].highid, 4);
+ offset = windres_get_32 (&wrtarget, mt->items[i].offset, 4);
+ while (low <= high)
+ {
+ rc_uint_type elen, flags;
+ if ((offset + BIN_MESSAGETABLE_ITEM_SIZE) > length)
+ {
+ has_error = 1;
+ break;
+ }
+ mti = (const struct bin_messagetable_item *) &data[offset];
+ elen = windres_get_16 (&wrtarget, mti->length, 2);
+ flags = windres_get_16 (&wrtarget, mti->flags, 2);
+ if ((offset + elen) > length)
+ {
+ has_error = 1;
+ break;
+ }
+ wr_printcomment (e, "MessageId = 0x%x", low);
+ wr_printcomment (e, "");
+
+ /* PR 17512: file: 5c3232dc. */
+ if (elen)
+ {
+ if ((flags & MESSAGE_RESOURCE_UNICODE) == MESSAGE_RESOURCE_UNICODE)
+ unicode_print (e, (const unichar *) mti->data,
+ (elen - BIN_MESSAGETABLE_ITEM_SIZE) / 2);
+ else
+ ascii_print (e, (const char *) mti->data,
+ (elen - BIN_MESSAGETABLE_ITEM_SIZE));
+ }
+ wr_printcomment (e,"");
+ ++low;
+ offset += elen;
+ }
+ }
+ }
+ while (0);
- low = windres_get_32 (&wrtarget, mt->items[i].lowid, 4);
- high = windres_get_32 (&wrtarget, mt->items[i].highid, 4);
- offset = windres_get_32 (&wrtarget, mt->items[i].offset, 4);
- while (low <= high)
- {
- rc_uint_type elen, flags;
- if ((offset + BIN_MESSAGETABLE_ITEM_SIZE) > length)
- {
- has_error = 1;
- break;
- }
- mti = (const struct bin_messagetable_item *) &data[offset];
- elen = windres_get_16 (&wrtarget, mti->length, 2);
- flags = windres_get_16 (&wrtarget, mti->flags, 2);
- if ((offset + elen) > length)
- {
- has_error = 1;
- break;
- }
- wr_printcomment (e, "MessageId = 0x%x", low);
- wr_printcomment (e, "");
- if ((flags & MESSAGE_RESOURCE_UNICODE) == MESSAGE_RESOURCE_UNICODE)
- unicode_print (e, (const unichar *) mti->data,
- (elen - BIN_MESSAGETABLE_ITEM_SIZE) / 2);
- else
- ascii_print (e, (const char *) mti->data,
- (elen - BIN_MESSAGETABLE_ITEM_SIZE));
- wr_printcomment (e,"");
- ++low;
- offset += elen;
- }
- }
- } while (0);
if (has_error)
wr_printcomment (e, "Illegal data");
wr_print_flush (e);