From bf31e6044082986689e17af54e2ca3cc1ac8419b Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Tue, 31 Dec 2019 22:24:31 +1030 Subject: [PATCH] asan: alpha-vms: Heap-buffer-overflow This fixes yet more errors in the alpha-vms buffer size checks. * vms-alpha.c (_bfd_vms_slurp_eisd): Don't overflow when checking offset. Don't overflow when checking rec_size, and do allow rec_size to the end of the buffer. Ensure eisd->type can be accessed, not just the first 32 bytes. Don't call _bfd_vms_save_counted_string with zero length remaining. Fail on empty string section name. (_bfd_vms_slurp_egsd): Formatting. Catch more reads past end of record size. Correct remaining length calculation. Fail on empty string section name. Consolidate error paths. --- bfd/ChangeLog | 12 +++++++ bfd/vms-alpha.c | 92 ++++++++++++++++++++++--------------------------- 2 files changed, 53 insertions(+), 51 deletions(-) diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 02e3caba59c..003f013b415 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,15 @@ +2019-12-31 Alan Modra + + * vms-alpha.c (_bfd_vms_slurp_eisd): Don't overflow when checking + offset. Don't overflow when checking rec_size, and do allow + rec_size to the end of the buffer. Ensure eisd->type can be + accessed, not just the first 32 bytes. Don't call + _bfd_vms_save_counted_string with zero length remaining. Fail + on empty string section name. + (_bfd_vms_slurp_egsd): Formatting. Catch more reads past end + of record size. Correct remaining length calculation. Fail + on empty string section name. Consolidate error paths. + 2019-12-30 Alan Modra * vms-alpha.c (alpha_vms_free_private): New function, extracted.. diff --git a/bfd/vms-alpha.c b/bfd/vms-alpha.c index e4928d7c974..2d289c3e8cf 100644 --- a/bfd/vms-alpha.c +++ b/bfd/vms-alpha.c @@ -529,12 +529,12 @@ _bfd_vms_slurp_eisd (bfd *abfd, unsigned int offset) asection *section; flagword bfd_flags; - /* 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) + /* PR 17512: file: 3d9e9fe9. */ + if (offset > PRIV (recrd.rec_size) + || (PRIV (recrd.rec_size) - offset + < offsetof (struct vms_eisd, eisdsize) + 4)) return FALSE; - eisd = (struct vms_eisd *)(PRIV (recrd.rec) + offset); + eisd = (struct vms_eisd *) (PRIV (recrd.rec) + offset); rec_size = bfd_getl32 (eisd->eisdsize); if (rec_size == 0) break; @@ -547,11 +547,10 @@ _bfd_vms_slurp_eisd (bfd *abfd, unsigned int offset) } /* 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) + if (rec_size < offsetof (struct vms_eisd, type) + 1) return FALSE; /* Make sure that the record is not too big either. */ - if (offset + rec_size >= PRIV (recrd.rec_size)) + if (rec_size > PRIV (recrd.rec_size) - offset) return FALSE; offset += rec_size; @@ -592,7 +591,7 @@ _bfd_vms_slurp_eisd (bfd *abfd, unsigned int offset) if (flags & EISD__M_GBL) { - if (rec_size < offsetof (struct vms_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 (abfd, eisd->gblnam, @@ -600,7 +599,7 @@ _bfd_vms_slurp_eisd (bfd *abfd, unsigned int offset) else name = _bfd_vms_save_counted_string (abfd, eisd->gblnam, EISD__K_GBLNAMLEN); - if (name == NULL) + if (name == NULL || name[0] == 0) return FALSE; bfd_flags |= SEC_COFF_SHARED_LIBRARY; bfd_flags &= ~(SEC_ALLOC | SEC_LOAD); @@ -1181,6 +1180,7 @@ _bfd_vms_slurp_egsd (bfd *abfd) unsigned int gsd_size; unsigned char *vms_rec; bfd_vma base_addr; + long psindx; vms_debug2 ((2, "EGSD\n")); @@ -1210,16 +1210,9 @@ _bfd_vms_slurp_egsd (bfd *abfd) /* PR 21615: Check for size overflow. */ if (PRIV (recrd.rec_size) < gsd_size) { - _bfd_error_handler (_("corrupt EGSD record: size (%#x) is larger than remaining space (%#x)"), - gsd_size, PRIV (recrd.rec_size)); - bfd_set_error (bfd_error_bad_value); - return FALSE; - } - - if (gsd_size < 4) - { - _bfd_error_handler (_("corrupt EGSD record: size (%#x) is too small"), - gsd_size); + _bfd_error_handler (_("corrupt EGSD record type %d: size (%#x) " + "is larger than remaining space (%#x)"), + gsd_type, gsd_size, PRIV (recrd.rec_size)); bfd_set_error (bfd_error_bad_value); return FALSE; } @@ -1229,10 +1222,19 @@ _bfd_vms_slurp_egsd (bfd *abfd) case EGSD__C_PSC: /* Program section definition. */ { - struct vms_egps *egps = (struct vms_egps *)vms_rec; + struct vms_egps *egps = (struct vms_egps *) vms_rec; flagword new_flags, vms_flags; asection *section; + if (offsetof (struct vms_egps, flags) + 2 > gsd_size) + { + too_small: + _bfd_error_handler (_("corrupt EGSD record type %d: size (%#x) " + "is too small"), + gsd_type, gsd_size); + bfd_set_error (bfd_error_bad_value); + return FALSE; + } vms_flags = bfd_getl16 (egps->flags); if ((vms_flags & EGPS__V_REL) == 0) @@ -1245,9 +1247,14 @@ _bfd_vms_slurp_egsd (bfd *abfd) { char *name; bfd_vma align_addr; + size_t left; - name = _bfd_vms_save_counted_string (abfd, &egps->namlng, - gsd_size - 4); + if (offsetof (struct vms_egps, namlng) >= gsd_size) + goto too_small; + left = gsd_size - offsetof (struct vms_egps, namlng); + name = _bfd_vms_save_counted_string (abfd, &egps->namlng, left); + if (name == NULL || name[0] == 0) + return FALSE; section = bfd_make_section (abfd, name); if (!section) @@ -1314,6 +1321,8 @@ _bfd_vms_slurp_egsd (bfd *abfd) struct vms_egsy *egsy = (struct vms_egsy *) vms_rec; flagword old_flags; + if (offsetof (struct vms_egsy, flags) + 2 > gsd_size) + goto too_small; old_flags = bfd_getl16 (egsy->flags); if (old_flags & EGSY__V_DEF) nameoff = ESDF__B_NAMLNG; @@ -1321,11 +1330,7 @@ _bfd_vms_slurp_egsd (bfd *abfd) nameoff = ESRF__B_NAMLNG; if (nameoff >= gsd_size) - { - _bfd_error_handler (_("ECSD__C_SYM record is too small")); - bfd_set_error (bfd_error_bad_value); - return FALSE; - } + goto too_small; entry = add_symbol (abfd, vms_rec + nameoff, gsd_size - nameoff); if (entry == NULL) return FALSE; @@ -1343,8 +1348,7 @@ _bfd_vms_slurp_egsd (bfd *abfd) if (old_flags & EGSY__V_DEF) { - struct vms_esdf *esdf = (struct vms_esdf *)vms_rec; - long psindx; + struct vms_esdf *esdf = (struct vms_esdf *) vms_rec; entry->value = bfd_getl64 (esdf->value); if (PRIV (sections) == NULL) @@ -1354,7 +1358,9 @@ _bfd_vms_slurp_egsd (bfd *abfd) /* PR 21813: Check for an out of range index. */ if (psindx < 0 || psindx >= (int) PRIV (section_count)) { - _bfd_error_handler (_("corrupt EGSD record: its psindx field is too big (%#lx)"), + bad_psindx: + _bfd_error_handler (_("corrupt EGSD record: its psindx " + "field is too big (%#lx)"), psindx); bfd_set_error (bfd_error_bad_value); return FALSE; @@ -1367,14 +1373,9 @@ _bfd_vms_slurp_egsd (bfd *abfd) entry->code_value = bfd_getl64 (esdf->code_address); psindx = bfd_getl32 (esdf->ca_psindx); - /* PR 21813: Check for an out of range index. */ + /* PR 21813: Check for an out of range index. */ if (psindx < 0 || psindx >= (int) PRIV (section_count)) - { - _bfd_error_handler (_("corrupt EGSD record: its psindx field is too big (%#lx)"), - psindx); - bfd_set_error (bfd_error_bad_value); - return FALSE; - } + goto bad_psindx; entry->code_section = PRIV (sections)[psindx]; } } @@ -1391,11 +1392,7 @@ _bfd_vms_slurp_egsd (bfd *abfd) old_flags = bfd_getl16 (egst->header.flags); if (nameoff >= gsd_size) - { - _bfd_error_handler (_("ECSD__C_SYMG record is too small")); - bfd_set_error (bfd_error_bad_value); - return FALSE; - } + goto too_small; entry = add_symbol (abfd, &egst->namlng, gsd_size - nameoff); if (entry == NULL) return FALSE; @@ -1408,19 +1405,12 @@ _bfd_vms_slurp_egsd (bfd *abfd) if (old_flags & EGSY__V_REL) { - long psindx; - if (PRIV (sections) == NULL) return FALSE; psindx = bfd_getl32 (egst->psindx); /* PR 21813: Check for an out of range index. */ if (psindx < 0 || psindx >= (int) PRIV (section_count)) - { - _bfd_error_handler (_("corrupt EGSD record: its psindx field is too big (%#lx)"), - psindx); - bfd_set_error (bfd_error_bad_value); - return FALSE; - } + goto bad_psindx; entry->section = PRIV (sections)[psindx]; } else -- 2.30.2