commit 29866fa186ee3ebda5242221607dba360b2e541e Author: Nick Clifton Date: Wed Jul 19 11:07:43 2017 +0100 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. commit 6c4e7b6bfbc4679f695106de2817ecf02b27c8be Author: Nick Clifton Date: Wed Jul 19 16:14:02 2017 +0100 Extend previous fix to coff-rs6000.c to coff64-rs6000.c PR 21786 * coff64-rs6000.c (_bfd_strntol): New function. (_bfd_strntoll): New function. (GET_VALUE_IN_FIELD): New macro. (xcoff64_slurp_armap): Use new macros. Upstream-Status: backport CVE: CVE-2017-12451 Signed-off-by: Thiruvadi Rajaraman Index: git/bfd/ChangeLog =================================================================== --- git.orig/bfd/ChangeLog 2017-08-31 16:07:20.966269193 +0530 +++ git/bfd/ChangeLog 2017-08-31 16:25:04.423155789 +0530 @@ -13,6 +13,19 @@ 2017-07-19 Nick Clifton + 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. + +2017-07-19 Nick Clifton + PR 21787 * archive.c (bfd_generic_archive_p): If the bfd does not have the correct magic bytes at the start, set the error to wrong format Index: git/bfd/coff-rs6000.c =================================================================== --- git.orig/bfd/coff-rs6000.c 2017-08-31 16:07:14.278208353 +0530 +++ git/bfd/coff-rs6000.c 2017-08-31 16:24:05.414696722 +0530 @@ -203,7 +203,8 @@ }; /* 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 @@ }; /* 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 @@ 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 @@ 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_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 @@ /* 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 @@ 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 @@ /* 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 @@ 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 @@ 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 @@ 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 @@ ((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 @@ 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 @@ ((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 @@ 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 @@ 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 @@ { 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; } Index: git/bfd/coff64-rs6000.c =================================================================== --- git.orig/bfd/coff64-rs6000.c 2017-08-31 16:07:14.282208390 +0530 +++ git/bfd/coff64-rs6000.c 2017-08-31 16:28:43.228864485 +0530 @@ -1852,6 +1852,46 @@ return NULL; } +/* 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) + /* Read in the armap of an XCOFF archive. */ static bfd_boolean @@ -1892,7 +1932,7 @@ return FALSE; /* Skip the name (normally empty). */ - namlen = strtol (hdr.namlen, (char **) NULL, 10); + GET_VALUE_IN_FIELD (namlen, hdr.namlen); pos = ((namlen + 1) & ~(size_t) 1) + SXCOFFARFMAG; if (bfd_seek (abfd, pos, SEEK_CUR) != 0) return FALSE;