Fix address violation when attempting to read a corrupt field in a COFF archive header structure.

	PR 21786
	* coff-rs6000.c (_bfd_strntol): New function.
	(_bfd_strntoll): New function.
	(GET_VALUE_IN_FIELD): New macro.
	(EQ_VALUE_IN_FIELD): new macro.
	(_bfd_xcoff_slurp_armap): Use new macros.
	(_bfd_xcoff_archive_p): Likewise.
	(_bfd_xcoff_read_ar_hdr): Likewise.
	(_bfd_xcoff_openr_next_archived_file): Likewise.
	(_bfd_xcoff_stat_arch_elt): Likewise.

1 files changed, 76 insertions, 50 deletions

diff --git a/bfd/coff-rs6000.c b/bfd/coff-rs6000.c
index 025c424..c72d0db 100644
--- a/bfd/coff-rs6000.c
+++ b/bfd/coff-rs6000.c
@@ -203,7 +203,8 @@ bfd_boolean (*xcoff_complain_overflow[XCOFF_MAX_COMPLAIN_OVERFLOW])
 };
 
 /* Information about one member of an archive.  */
-struct member_layout {
+struct member_layout
+{
   /* The archive member that this structure describes.  */
   bfd *member;
 
@@ -237,7 +238,8 @@ struct member_layout {
 };
 
 /* A structure used for iterating over the members of an archive.  */
-struct archive_iterator {
+struct archive_iterator
+{
   /* The archive itself.  */
   bfd *archive;
 
@@ -654,8 +656,6 @@ _bfd_xcoff_swap_aux_out (bfd *abfd, void * inp, int type, int in_class,
 end:
   return bfd_coff_auxesz (abfd);
 }
-
-
 
 /* The XCOFF reloc table.  Actually, XCOFF relocations specify the
    bitsize and whether they are signed or not, along with a
@@ -663,7 +663,6 @@ end:
    different algorithms for putting in the reloc.  Many of these
    relocs need special_function entries, which I have not written.  */
 
-
 reloc_howto_type xcoff_howto_table[] =
 {
   /* 0x00: Standard 32 bit relocation.  */
@@ -1185,6 +1184,51 @@ bfd_xcoff_ar_archive_set_magic (bfd *abfd ATTRIBUTE_UNUSED,
  /* bfd_xcoff_archive_set_magic (abfd, magic); */
 }
 
+/* PR 21786:  The PE/COFF standard does not require NUL termination for any of
+   the ASCII fields in the archive headers.  So in order to be able to extract
+   numerical values we provide our own versions of strtol and strtoll which
+   take a maximum length as an additional parameter.  Also - just to save space,
+   we omit the endptr return parameter, since we know that it is never used.  */
+
+static long
+_bfd_strntol (const char * nptr, int base, unsigned int maxlen)
+{
+  char buf[24]; /* Should be enough.  */
+
+  BFD_ASSERT (maxlen < (sizeof (buf) - 1));
+
+  memcpy (buf, nptr, maxlen);
+  buf[maxlen] = 0;
+  return strtol (buf, NULL, base);
+}
+
+static long long
+_bfd_strntoll (const char * nptr, int base, unsigned int maxlen)
+{
+  char buf[32]; /* Should be enough.  */
+
+  BFD_ASSERT (maxlen < (sizeof (buf) - 1));
+
+  memcpy (buf, nptr, maxlen);
+  buf[maxlen] = 0;
+  return strtoll (buf, NULL, base);
+}
+
+/* Macro to read an ASCII value stored in an archive header field.  */
+#define GET_VALUE_IN_FIELD(VAR, FIELD)		  \
+  do						  \
+    {						  \
+      (VAR) = sizeof (VAR) > sizeof (long)	  \
+        ? _bfd_strntoll (FIELD, 10, sizeof FIELD) \
+	: _bfd_strntol (FIELD, 10, sizeof FIELD); \
+    }						  \
+  while (0)
+
+#define EQ_VALUE_IN_FIELD(VAR, FIELD)			\
+  (sizeof (VAR) > sizeof (long)				\
+   ? (VAR) ==_bfd_strntoll (FIELD, 10, sizeof FIELD)	\
+   : (VAR) == _bfd_strntol (FIELD, 10, sizeof FIELD))
+
 /* Read in the armap of an XCOFF archive.  */
 
 bfd_boolean
@@ -1209,7 +1253,7 @@ _bfd_xcoff_slurp_armap (bfd *abfd)
       /* This is for the old format.  */
       struct xcoff_ar_hdr hdr;
 
-      off = strtol (xcoff_ardata (abfd)->symoff, (char **) NULL, 10);
+      GET_VALUE_IN_FIELD (off, xcoff_ardata (abfd)->symoff);
       if (off == 0)
 	{
 	  bfd_has_map (abfd) = FALSE;
@@ -1225,12 +1269,12 @@ _bfd_xcoff_slurp_armap (bfd *abfd)
 	return FALSE;
 
       /* Skip the name (normally empty).  */
-      namlen = strtol (hdr.namlen, (char **) NULL, 10);
+      GET_VALUE_IN_FIELD (namlen, hdr.namlen);
       off = ((namlen + 1) & ~ (size_t) 1) + SXCOFFARFMAG;
       if (bfd_seek (abfd, off, SEEK_CUR) != 0)
 	return FALSE;
 
-      sz = strtol (hdr.size, (char **) NULL, 10);
+      GET_VALUE_IN_FIELD (sz, hdr.size);
 
       /* Read in the entire symbol table.  */
       contents = (bfd_byte *) bfd_alloc (abfd, sz);
@@ -1264,7 +1308,7 @@ _bfd_xcoff_slurp_armap (bfd *abfd)
       /* This is for the new format.  */
       struct xcoff_ar_hdr_big hdr;
 
-      off = strtol (xcoff_ardata_big (abfd)->symoff, (char **) NULL, 10);
+      GET_VALUE_IN_FIELD (off, xcoff_ardata_big (abfd)->symoff);
       if (off == 0)
 	{
 	  bfd_has_map (abfd) = FALSE;
@@ -1280,15 +1324,12 @@ _bfd_xcoff_slurp_armap (bfd *abfd)
 	return FALSE;
 
       /* Skip the name (normally empty).  */
-      namlen = strtol (hdr.namlen, (char **) NULL, 10);
+      GET_VALUE_IN_FIELD (namlen, hdr.namlen);
       off = ((namlen + 1) & ~ (size_t) 1) + SXCOFFARFMAG;
       if (bfd_seek (abfd, off, SEEK_CUR) != 0)
 	return FALSE;
 
-      /* XXX This actually has to be a call to strtoll (at least on 32-bit
-	 machines) since the field width is 20 and there numbers with more
-	 than 32 bits can be represented.  */
-      sz = strtol (hdr.size, (char **) NULL, 10);
+      GET_VALUE_IN_FIELD (sz, hdr.size);
 
       /* Read in the entire symbol table.  */
       contents = (bfd_byte *) bfd_alloc (abfd, sz);
@@ -1393,8 +1434,8 @@ _bfd_xcoff_archive_p (bfd *abfd)
 	  goto error_ret;
 	}
 
-      bfd_ardata (abfd)->first_file_filepos = strtol (hdr.firstmemoff,
-						      (char **) NULL, 10);
+      GET_VALUE_IN_FIELD (bfd_ardata (abfd)->first_file_filepos,
+			  hdr.firstmemoff);
 
       amt = SIZEOF_AR_FILE_HDR;
       bfd_ardata (abfd)->tdata = bfd_zalloc (abfd, amt);
@@ -1469,7 +1510,7 @@ _bfd_xcoff_read_ar_hdr (bfd *abfd)
 	  return NULL;
 	}
 
-      namlen = strtol (hdr.namlen, (char **) NULL, 10);
+      GET_VALUE_IN_FIELD (namlen, hdr.namlen);
       amt = SIZEOF_AR_HDR + namlen + 1;
       hdrp = (struct xcoff_ar_hdr *) bfd_alloc (abfd, amt);
       if (hdrp == NULL)
@@ -1486,7 +1527,7 @@ _bfd_xcoff_read_ar_hdr (bfd *abfd)
       ((char *) hdrp)[SIZEOF_AR_HDR + namlen] = '\0';
 
       ret->arch_header = (char *) hdrp;
-      ret->parsed_size = strtol (hdr.size, (char **) NULL, 10);
+      GET_VALUE_IN_FIELD (ret->parsed_size, hdr.size);
       ret->filename = (char *) hdrp + SIZEOF_AR_HDR;
     }
   else
@@ -1501,7 +1542,7 @@ _bfd_xcoff_read_ar_hdr (bfd *abfd)
 	  return NULL;
 	}
 
-      namlen = strtol (hdr.namlen, (char **) NULL, 10);
+      GET_VALUE_IN_FIELD (namlen, hdr.namlen);
       amt = SIZEOF_AR_HDR_BIG + namlen + 1;
       hdrp = (struct xcoff_ar_hdr_big *) bfd_alloc (abfd, amt);
       if (hdrp == NULL)
@@ -1518,10 +1559,7 @@ _bfd_xcoff_read_ar_hdr (bfd *abfd)
       ((char *) hdrp)[SIZEOF_AR_HDR_BIG + namlen] = '\0';
 
       ret->arch_header = (char *) hdrp;
-      /* XXX This actually has to be a call to strtoll (at least on 32-bit
-	 machines) since the field width is 20 and there numbers with more
-	 than 32 bits can be represented.  */
-      ret->parsed_size = strtol (hdr.size, (char **) NULL, 10);
+      GET_VALUE_IN_FIELD (ret->parsed_size, hdr.size);
       ret->filename = (char *) hdrp + SIZEOF_AR_HDR_BIG;
     }
 
@@ -1550,14 +1588,11 @@ _bfd_xcoff_openr_next_archived_file (bfd *archive, bfd *last_file)
       if (last_file == NULL)
 	filestart = bfd_ardata (archive)->first_file_filepos;
       else
-	filestart = strtol (arch_xhdr (last_file)->nextoff, (char **) NULL,
-			    10);
+	GET_VALUE_IN_FIELD (filestart, arch_xhdr (last_file)->nextoff);
 
       if (filestart == 0
-	  || filestart == strtol (xcoff_ardata (archive)->memoff,
-				  (char **) NULL, 10)
-	  || filestart == strtol (xcoff_ardata (archive)->symoff,
-				  (char **) NULL, 10))
+	  || EQ_VALUE_IN_FIELD (filestart, xcoff_ardata (archive)->memoff)
+	  || EQ_VALUE_IN_FIELD (filestart, xcoff_ardata (archive)->symoff))
 	{
 	  bfd_set_error (bfd_error_no_more_archived_files);
 	  return NULL;
@@ -1568,20 +1603,11 @@ _bfd_xcoff_openr_next_archived_file (bfd *archive, bfd *last_file)
       if (last_file == NULL)
 	filestart = bfd_ardata (archive)->first_file_filepos;
       else
-	/* XXX These actually have to be a calls to strtoll (at least
-	   on 32-bit machines) since the fields's width is 20 and
-	   there numbers with more than 32 bits can be represented.  */
-	filestart = strtol (arch_xhdr_big (last_file)->nextoff, (char **) NULL,
-			    10);
-
-      /* XXX These actually have to be calls to strtoll (at least on 32-bit
-	 machines) since the fields's width is 20 and there numbers with more
-	 than 32 bits can be represented.  */
+	GET_VALUE_IN_FIELD (filestart, arch_xhdr_big (last_file)->nextoff);
+
       if (filestart == 0
-	  || filestart == strtol (xcoff_ardata_big (archive)->memoff,
-				  (char **) NULL, 10)
-	  || filestart == strtol (xcoff_ardata_big (archive)->symoff,
-				  (char **) NULL, 10))
+	  || EQ_VALUE_IN_FIELD (filestart, xcoff_ardata_big (archive)->memoff)
+	  || EQ_VALUE_IN_FIELD (filestart, xcoff_ardata_big (archive)->symoff))
 	{
 	  bfd_set_error (bfd_error_no_more_archived_files);
 	  return NULL;
@@ -1606,20 +1632,20 @@ _bfd_xcoff_stat_arch_elt (bfd *abfd, struct stat *s)
     {
       struct xcoff_ar_hdr *hdrp = arch_xhdr (abfd);
 
-      s->st_mtime = strtol (hdrp->date, (char **) NULL, 10);
-      s->st_uid = strtol (hdrp->uid, (char **) NULL, 10);
-      s->st_gid = strtol (hdrp->gid, (char **) NULL, 10);
-      s->st_mode = strtol (hdrp->mode, (char **) NULL, 8);
+      GET_VALUE_IN_FIELD (s->st_mtime, hdrp->date);
+      GET_VALUE_IN_FIELD (s->st_uid, hdrp->uid);
+      GET_VALUE_IN_FIELD (s->st_gid, hdrp->gid);
+      GET_VALUE_IN_FIELD (s->st_mode, hdrp->mode);
       s->st_size = arch_eltdata (abfd)->parsed_size;
     }
   else
     {
       struct xcoff_ar_hdr_big *hdrp = arch_xhdr_big (abfd);
 
-      s->st_mtime = strtol (hdrp->date, (char **) NULL, 10);
-      s->st_uid = strtol (hdrp->uid, (char **) NULL, 10);
-      s->st_gid = strtol (hdrp->gid, (char **) NULL, 10);
-      s->st_mode = strtol (hdrp->mode, (char **) NULL, 8);
+      GET_VALUE_IN_FIELD (s->st_mtime, hdrp->date);
+      GET_VALUE_IN_FIELD (s->st_uid, hdrp->uid);
+      GET_VALUE_IN_FIELD (s->st_gid, hdrp->gid);
+      GET_VALUE_IN_FIELD (s->st_mode, hdrp->mode);
       s->st_size = arch_eltdata (abfd)->parsed_size;
     }