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 --git a/binutils/ChangeLog b/binutils/ChangeLog
index 53ec0725c5bc2e9d5d54996269dd65ea4bc79dcd..924f35c84c7cdbc84de8535eed1185fead95dd8d 100644 (file)
--- 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 78ac57eee95f123d3f8e52b9c32c5ecf1898650a..0cf6c2c1f2a662fe74fe8b63380ad30a0b9918e2 100644 (file)
--- 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 0004c3aa3500c85af952be11889d16138eb767e8..3e63e49901d3c96c1e661a26de6e00eda5f1b7bf 100644 (file)
--- 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 4e0b24c90fc1de51134fc8d5f9836048aa2dc779..f0cacd16b1a7117db1af35125969f08ca54aacdc 100644 (file)
--- 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);