From 7adc0a8174f1233f6d92edd0671c18c9870e64e7 Mon Sep 17 00:00:00 2001 From: Nick Clifton Date: Wed, 21 Jun 2017 15:21:11 +0100 Subject: [PATCH] Fix address violation parsing a corrupt Alpha VMS binary file. PR binutils/21639 * vms-misc.c (_bfd_vms_save_sized_string): Use unsigned int as type of the size parameter. (_bfd_vms_save_counted_string): Add second parameter - the maximum length of the counted string. * vms.h (_bfd_vms_save_sized_string): Update prototype. (_bfd_vms_save_counted_string): Likewise. * vms-alpha.c (_bfd_vms_slurp_eisd): Update calls to _bfd_vms_save_counted_string. (_bfd_vms_slurp_ehdr): Likewise. (_bfd_vms_slurp_egsd): Likewise. (Parse_module): Likewise. --- bfd/ChangeLog | 15 ++++++++++++++ bfd/vms-alpha.c | 54 +++++++++++++++++++++++++++++++++++++------------ bfd/vms-misc.c | 8 +++++--- bfd/vms.h | 4 ++-- 4 files changed, 63 insertions(+), 18 deletions(-) diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 7b2927fb3ea..35b9d97a98f 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,18 @@ +2017-06-21 Nick Clifton + + PR binutils/21639 + * vms-misc.c (_bfd_vms_save_sized_string): Use unsigned int as + type of the size parameter. + (_bfd_vms_save_counted_string): Add second parameter - the maximum + length of the counted string. + * vms.h (_bfd_vms_save_sized_string): Update prototype. + (_bfd_vms_save_counted_string): Likewise. + * vms-alpha.c (_bfd_vms_slurp_eisd): Update calls to + _bfd_vms_save_counted_string. + (_bfd_vms_slurp_ehdr): Likewise. + (_bfd_vms_slurp_egsd): Likewise. + (Parse_module): Likewise. + 2017-06-21 Alan Modra * elf64-ppc.c (ppc64_elf_size_stubs): Test for localentry:0 plt diff --git a/bfd/vms-alpha.c b/bfd/vms-alpha.c index 5f1b24aeb58..a2cfa337bfa 100644 --- a/bfd/vms-alpha.c +++ b/bfd/vms-alpha.c @@ -521,8 +521,10 @@ _bfd_vms_slurp_eisd (bfd *abfd, unsigned int offset) asection *section; flagword bfd_flags; - /* PR 17512: file: 3d9e9fe9. */ - if (offset >= PRIV (recrd.rec_size)) + /* PR 17512: file: 3d9e9fe9. + 12 is the offset of the eisdsize field from the start of the record (8) + plus the size of the eisdsize field (4). */ + if (offset >= PRIV (recrd.rec_size) - 12) return FALSE; eisd = (struct vms_eisd *)(PRIV (recrd.rec) + offset); rec_size = bfd_getl32 (eisd->eisdsize); @@ -535,8 +537,16 @@ _bfd_vms_slurp_eisd (bfd *abfd, unsigned int offset) offset = (offset + VMS_BLOCK_SIZE) & ~(VMS_BLOCK_SIZE - 1); continue; } - else - offset += rec_size; + + /* Make sure that there is enough data present in the record. */ + /* FIXME: Should we use sizeof (struct vms_eisd) rather than just 32 here ? */ + if (rec_size < 32) + return FALSE; + /* Make sure that the record is not too big either. */ + if (offset + rec_size >= PRIV (recrd.rec_size)) + return FALSE; + + offset += rec_size; size = bfd_getl32 (eisd->secsize); vaddr = bfd_getl64 (eisd->virt_addr); @@ -574,7 +584,13 @@ _bfd_vms_slurp_eisd (bfd *abfd, unsigned int offset) if (flags & EISD__M_GBL) { - name = _bfd_vms_save_counted_string (eisd->gblnam); + if (rec_size < offsetof (struct vms_eisd, gblnam)) + return FALSE; + else if (rec_size < sizeof (struct vms_eisd)) + name = _bfd_vms_save_counted_string (eisd->gblnam, + rec_size - offsetof (struct vms_eisd, gblnam)); + else + name = _bfd_vms_save_counted_string (eisd->gblnam, EISD__K_GBLNAMLEN); bfd_flags |= SEC_COFF_SHARED_LIBRARY; bfd_flags &= ~(SEC_ALLOC | SEC_LOAD); } @@ -879,7 +895,7 @@ _bfd_vms_slurp_ehdr (bfd *abfd) vms_rec = PRIV (recrd.rec); /* PR 17512: file: 62736583. */ - end = PRIV (recrd.buf) + PRIV (recrd.buf_size); + end = vms_rec + PRIV (recrd.buf_size); vms_debug2 ((2, "HDR/EMH\n")); @@ -899,11 +915,11 @@ _bfd_vms_slurp_ehdr (bfd *abfd) PRIV (hdr_data).hdr_l_recsiz = bfd_getl32 (vms_rec + 16); if ((vms_rec + 20 + vms_rec[20] + 1) >= end) goto fail; - PRIV (hdr_data).hdr_t_name = _bfd_vms_save_counted_string (vms_rec + 20); + PRIV (hdr_data).hdr_t_name = _bfd_vms_save_counted_string (vms_rec + 20, vms_rec[20]); ptr = vms_rec + 20 + vms_rec[20] + 1; if ((ptr + *ptr + 1) >= end) goto fail; - PRIV (hdr_data).hdr_t_version =_bfd_vms_save_counted_string (ptr); + PRIV (hdr_data).hdr_t_version =_bfd_vms_save_counted_string (ptr, *ptr); ptr += *ptr + 1; if (ptr + 17 >= end) goto fail; @@ -1175,6 +1191,14 @@ _bfd_vms_slurp_egsd (bfd *abfd) return FALSE; } + if (gsd_size < 4) + { + _bfd_error_handler (_("Corrupt EGSD record: size (%#x) is too small"), + gsd_size); + bfd_set_error (bfd_error_bad_value); + return FALSE; + } + switch (gsd_type) { case EGSD__C_PSC: @@ -1197,7 +1221,7 @@ _bfd_vms_slurp_egsd (bfd *abfd) char *name; unsigned long align_addr; - name = _bfd_vms_save_counted_string (&egps->namlng); + name = _bfd_vms_save_counted_string (&egps->namlng, gsd_size - 4); section = bfd_make_section (abfd, name); if (!section) @@ -4123,7 +4147,8 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr, { case DST__K_MODBEG: module->name - = _bfd_vms_save_counted_string (ptr + DST_S_B_MODBEG_NAME); + = _bfd_vms_save_counted_string (ptr + DST_S_B_MODBEG_NAME, + maxptr - (ptr + DST_S_B_MODBEG_NAME)); curr_pc = 0; prev_pc = 0; @@ -4140,7 +4165,8 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr, funcinfo = (struct funcinfo *) bfd_zalloc (abfd, sizeof (struct funcinfo)); funcinfo->name - = _bfd_vms_save_counted_string (ptr + DST_S_B_RTNBEG_NAME); + = _bfd_vms_save_counted_string (ptr + DST_S_B_RTNBEG_NAME, + maxptr - (ptr + DST_S_B_RTNBEG_NAME)); funcinfo->low = bfd_getl32 (ptr + DST_S_L_RTNBEG_ADDRESS); funcinfo->next = module->func_table; module->func_table = funcinfo; @@ -4191,8 +4217,10 @@ parse_module (bfd *abfd, struct module *module, unsigned char *ptr, unsigned int fileid = bfd_getl16 (src_ptr + DST_S_W_SRC_DF_FILEID); char *filename - = _bfd_vms_save_counted_string (src_ptr - + DST_S_B_SRC_DF_FILENAME); + = _bfd_vms_save_counted_string (src_ptr + DST_S_B_SRC_DF_FILENAME, + (ptr + rec_length) - + (src_ptr + DST_S_B_SRC_DF_FILENAME) + ); while (fileid >= module->file_table_count) { diff --git a/bfd/vms-misc.c b/bfd/vms-misc.c index 40c6cc263fa..7497f0224da 100644 --- a/bfd/vms-misc.c +++ b/bfd/vms-misc.c @@ -139,7 +139,7 @@ _bfd_hexdump (int level, unsigned char *ptr, int size, int offset) size is string size (size of record) */ char * -_bfd_vms_save_sized_string (unsigned char *str, int size) +_bfd_vms_save_sized_string (unsigned char *str, unsigned int size) { char *newstr = bfd_malloc ((bfd_size_type) size + 1); @@ -155,10 +155,12 @@ _bfd_vms_save_sized_string (unsigned char *str, int size) ptr points to size byte on entry */ char * -_bfd_vms_save_counted_string (unsigned char *ptr) +_bfd_vms_save_counted_string (unsigned char *ptr, unsigned int maxlen) { - int len = *ptr++; + unsigned int len = *ptr++; + if (len > maxlen) + return NULL; return _bfd_vms_save_sized_string (ptr, len); } diff --git a/bfd/vms.h b/bfd/vms.h index e1c5d83dfed..230ff7f26b4 100644 --- a/bfd/vms.h +++ b/bfd/vms.h @@ -118,8 +118,8 @@ extern void vms_time_t_to_vms_time (time_t ut, unsigned int *hi, unsigned int *l extern void vms_get_time (unsigned int *hi, unsigned int *lo); extern void vms_raw_get_time (unsigned char *buf); -extern char * _bfd_vms_save_sized_string (unsigned char *, int); -extern char * _bfd_vms_save_counted_string (unsigned char *); +extern char * _bfd_vms_save_sized_string (unsigned char *, unsigned); +extern char * _bfd_vms_save_counted_string (unsigned char *, unsigned); extern void _bfd_vms_output_begin (struct vms_rec_wr *, int); extern void _bfd_vms_output_alignment (struct vms_rec_wr *, int); extern void _bfd_vms_output_begin_subrec (struct vms_rec_wr *, int); -- 2.30.2