asan: alpha-vms: Heap-buffer-overflow
Alan Modra
amodra@gmail.com
Tue Dec 31 12:56:00 GMT 2019
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.
diff --git a/bfd/vms-alpha.c b/bfd/vms-alpha.c
index e4928d7c97..2d289c3e8c 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
--
Alan Modra
Australia Development Lab, IBM
More information about the Binutils
mailing list