From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17705 invoked by alias); 24 Feb 2020 02:07:00 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 17677 invoked by uid 89); 24 Feb 2020 02:07:00 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=heap, 42927, vms X-HELO: mail-pg1-f179.google.com Received: from mail-pg1-f179.google.com (HELO mail-pg1-f179.google.com) (209.85.215.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 24 Feb 2020 02:06:57 +0000 Received: by mail-pg1-f179.google.com with SMTP id d9so4315700pgu.3 for ; Sun, 23 Feb 2020 18:06:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:subject:message-id:mime-version:content-disposition :user-agent; bh=BFznrHgirLqvFjDwrx9GZmKfAL1W58axfXMkKCJze4Q=; b=OAQryu87g1hPN8syWq2Q/RQYESqiGDQHK3L9zSg+r7mAOCraXKaYhtQ5p9zanQyG4X LiV3vmrKMbQVxWsO3GOr1ZHEmGLv4ImeGAPYt1e97sfstJfojRodB+dfIS+H4ZJPD1CU NRb25wlhjrwcTtLzHz4hACSKR/Drc4vMbgOecrHw51zwySSQ9OuvdkV1detsnH9nHYsU ZRqGCauCL7HoTaVHyita6ZijBN9JrnLbDsgLox3+Ue+zqflowgaSxOTL5Dqr/XWKAGw5 79mL/WH6k3K0Vbrglh/13b2qG4+jrIHOkJfEBtoWs9Sx74Xl3E2gAA/CXjgKGMda0Kfc xKqg== Return-Path: Received: from bubble.grove.modra.org ([2406:3400:51d:8cc0:d1c5:8035:aef6:3630]) by smtp.gmail.com with ESMTPSA id d64sm10047796pga.17.2020.02.23.18.06.54 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 23 Feb 2020 18:06:54 -0800 (PST) Received: by bubble.grove.modra.org (Postfix, from userid 1000) id 125F789FC8; Mon, 24 Feb 2020 12:36:51 +1030 (ACDT) Date: Mon, 24 Feb 2020 02:07:00 -0000 From: Alan Modra To: binutils@sourceware.org Subject: vms buffer overflows and large memory allocation Message-ID: <20200224020650.GD5570@bubble.grove.modra.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.9.4 (2018-02-28) X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg00523.txt.bz2 * vms-lib.c (struct carsym_mem): Add limit. (vms_add_index): Heed limit. (vms_traverse_index): Catch buffer overflows. Remove outdated fixme. (vms_lib_read_index): Set up limit. Catch 32-bit overflow. Always return actual number read. (_bfd_vms_lib_archive_p): Catch buffer overflows. Replace assertion with error exit. diff --git a/bfd/vms-lib.c b/bfd/vms-lib.c index 6ae1a7bafb..3b42857aa9 100644 --- a/bfd/vms-lib.c +++ b/bfd/vms-lib.c @@ -120,6 +120,9 @@ struct carsym_mem /* Maximum number of entries. */ unsigned int max; + /* Do not allocate more that this number of entries. */ + unsigned int limit; + /* If true, the table was reallocated on the heap. If false, it is still in the BFD's objalloc. */ bfd_boolean realloced; @@ -136,12 +139,14 @@ vms_add_index (struct carsym_mem *cs, char *name, struct carsym *n; size_t amt; - if (cs->max > -33u / 2) + if (cs->max > -33u / 2 || cs->max >= cs->limit) { bfd_set_error (bfd_error_file_too_big); return FALSE; } cs->max = 2 * cs->max + 32; + if (cs->max > cs->limit) + cs->max = cs->limit; if (_bfd_mul_overflow (cs->max, sizeof (struct carsym), &amt)) { bfd_set_error (bfd_error_file_too_big); @@ -243,6 +248,7 @@ vms_traverse_index (bfd *abfd, unsigned int vbn, struct carsym_mem *cs) file_ptr off; unsigned char *p; unsigned char *endp; + unsigned int n; /* Read the index block. */ BFD_ASSERT (sizeof (indexdef) == VMS_BLOCK_SIZE); @@ -251,7 +257,10 @@ vms_traverse_index (bfd *abfd, unsigned int vbn, struct carsym_mem *cs) /* Traverse it. */ p = &indexdef.keys[0]; - endp = p + bfd_getl16 (indexdef.used); + n = bfd_getl16 (indexdef.used); + if (n > sizeof (indexdef.keys)) + return FALSE; + endp = p + n; while (p < endp) { unsigned int idx_vbn; @@ -292,6 +301,8 @@ vms_traverse_index (bfd *abfd, unsigned int vbn, struct carsym_mem *cs) /* Point to the next index entry. */ p = keyname + keylen; + if (p > endp) + return FALSE; if (idx_off == RFADEF__C_INDEX) { @@ -333,11 +344,17 @@ vms_traverse_index (bfd *abfd, unsigned int vbn, struct carsym_mem *cs) if (!vms_read_block (abfd, kvbn, kblk)) return FALSE; + if (koff > sizeof (kblk) - sizeof (struct vms_kbn)) + return FALSE; kbn = (struct vms_kbn *)(kblk + koff); klen = bfd_getl16 (kbn->keylen); + if (klen > sizeof (kblk) - koff) + return FALSE; kvbn = bfd_getl32 (kbn->rfa.vbn); koff = bfd_getl16 (kbn->rfa.offset); + if (noff + klen > keylen) + return FALSE; memcpy (name + noff, kbn + 1, klen); noff += klen; } @@ -368,7 +385,7 @@ vms_traverse_index (bfd *abfd, unsigned int vbn, struct carsym_mem *cs) || bfd_bread (&lhs, sizeof (lhs), abfd) != sizeof (lhs)) return FALSE; - /* FIXME: this adds extra entries that were not accounted. */ + /* These extra entries may cause reallocation of CS. */ if (!vms_add_indexes_from_list (abfd, cs, name, &lhs.ng_g_rfa)) return FALSE; if (!vms_add_indexes_from_list (abfd, cs, name, &lhs.ng_wk_rfa)) @@ -397,7 +414,8 @@ vms_lib_read_index (bfd *abfd, int idx, unsigned int *nbrel) struct vms_idd idd; unsigned int flags; unsigned int vbn; - struct carsym *csbuf; + ufile_ptr filesize; + size_t amt; struct carsym_mem csm; /* Read index desription. */ @@ -411,14 +429,27 @@ vms_lib_read_index (bfd *abfd, int idx, unsigned int *nbrel) || !(flags & IDD__FLAGS_VARLENIDX)) return NULL; - csbuf = bfd_alloc (abfd, *nbrel * sizeof (struct carsym)); - if (csbuf == NULL) - return NULL; - - csm.max = *nbrel; + filesize = bfd_get_file_size (abfd); csm.nbr = 0; + csm.max = *nbrel; + csm.limit = -1u; csm.realloced = FALSE; - csm.idx = csbuf; + if (filesize != 0) + { + /* Put an upper bound based on a file full of single char keys. + This is to prevent fuzzed binary silliness. It is easily + possible to set up loops over file blocks that add syms + without end. */ + if (filesize / (sizeof (struct vms_rfa) + 2) <= -1u) + csm.limit = filesize / (sizeof (struct vms_rfa) + 2); + } + if (csm.max > csm.limit) + csm.max = csm.limit; + if (_bfd_mul_overflow (csm.max, sizeof (struct carsym), &amt)) + return NULL; + csm.idx = bfd_alloc (abfd, amt); + if (csm.idx == NULL) + return NULL; /* Note: if the index is empty, there is no block to traverse. */ vbn = bfd_getl32 (idd.vbn); @@ -429,7 +460,7 @@ vms_lib_read_index (bfd *abfd, int idx, unsigned int *nbrel) /* Note: in case of error, we can free what was allocated on the BFD's objalloc. */ - bfd_release (abfd, csbuf); + bfd_release (abfd, csm.idx); return NULL; } @@ -437,14 +468,16 @@ vms_lib_read_index (bfd *abfd, int idx, unsigned int *nbrel) { /* There are more entries than the first estimate. Allocate on the BFD's objalloc. */ + struct carsym *csbuf; csbuf = bfd_alloc (abfd, csm.nbr * sizeof (struct carsym)); if (csbuf == NULL) return NULL; memcpy (csbuf, csm.idx, csm.nbr * sizeof (struct carsym)); free (csm.idx); - *nbrel = csm.nbr; + csm.idx = csbuf; } - return csbuf; + *nbrel = csm.nbr; + return csm.idx; } /* Standard function. */ @@ -568,6 +601,8 @@ _bfd_vms_lib_archive_p (bfd *abfd, enum vms_lib_kind kind) != sizeof (buf_reclen)) goto err; reclen = bfd_getl32 (buf_reclen); + if (reclen < sizeof (struct vms_dcxmap)) + goto err; buf = _bfd_malloc_and_read (abfd, reclen, reclen); if (buf == NULL) goto err; @@ -578,39 +613,51 @@ _bfd_vms_lib_archive_p (bfd *abfd, enum vms_lib_kind kind) (abfd, tdata->nbr_dcxsbm * sizeof (struct dcxsbm_desc)); for (i = 0; i < tdata->nbr_dcxsbm; i++) { - struct vms_dcxsbm *sbm = (struct vms_dcxsbm *) (buf + sbm_off); + struct vms_dcxsbm *sbm; struct dcxsbm_desc *sbmdesc = &tdata->dcxsbm[i]; unsigned int sbm_len; unsigned int sbm_sz; unsigned int off; - unsigned char *data = (unsigned char *)sbm; unsigned char *buf1; unsigned int l, j; + if (sbm_off > reclen + || reclen - sbm_off < sizeof (struct vms_dcxsbm)) + goto err; + sbm = (struct vms_dcxsbm *) (buf + sbm_off); sbm_sz = bfd_getl16 (sbm->size); sbm_off += sbm_sz; - BFD_ASSERT (sbm_off <= reclen); sbmdesc->min_char = sbm->min_char; BFD_ASSERT (sbmdesc->min_char == 0); sbmdesc->max_char = sbm->max_char; sbm_len = sbmdesc->max_char - sbmdesc->min_char + 1; l = (2 * sbm_len + 7) / 8; - BFD_ASSERT - (sbm_sz >= sizeof (struct vms_dcxsbm) + l + 3 * sbm_len - || (tdata->nbr_dcxsbm == 1 - && sbm_sz >= sizeof (struct vms_dcxsbm) + l + sbm_len)); + if (sbm_sz < sizeof (struct vms_dcxsbm) + l + sbm_len + || (tdata->nbr_dcxsbm > 1 + && sbm_sz < sizeof (struct vms_dcxsbm) + l + 3 * sbm_len)) + goto err; sbmdesc->flags = (unsigned char *)bfd_alloc (abfd, l); - memcpy (sbmdesc->flags, data + bfd_getl16 (sbm->flags), l); + off = bfd_getl16 (sbm->flags); + if (off > reclen - sbm_off + || reclen - sbm_off - off < l) + goto err; + memcpy (sbmdesc->flags, (bfd_byte *) sbm + off, l); sbmdesc->nodes = (unsigned char *)bfd_alloc (abfd, 2 * sbm_len); - memcpy (sbmdesc->nodes, data + bfd_getl16 (sbm->nodes), 2 * sbm_len); + off = bfd_getl16 (sbm->nodes); + if (off > reclen - sbm_off + || reclen - sbm_off - off < 2 * sbm_len) + goto err; + memcpy (sbmdesc->nodes, (bfd_byte *) sbm + off, 2 * sbm_len); off = bfd_getl16 (sbm->next); if (off != 0) { + if (off > reclen - sbm_off + || reclen - sbm_off - off < 2 * sbm_len) + goto err; /* Read the 'next' array. */ - sbmdesc->next = (unsigned short *)bfd_alloc - (abfd, sbm_len * sizeof (unsigned short)); - buf1 = data + off; + sbmdesc->next = (unsigned short *) bfd_alloc (abfd, 2 * sbm_len); + buf1 = (bfd_byte *) sbm + off; for (j = 0; j < sbm_len; j++) sbmdesc->next[j] = bfd_getl16 (buf1 + j * 2); } -- Alan Modra Australia Development Lab, IBM