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.

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);