From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1034.google.com (mail-pj1-x1034.google.com [IPv6:2607:f8b0:4864:20::1034]) by sourceware.org (Postfix) with ESMTPS id 3EB63385782B for ; Mon, 9 Jan 2023 23:39:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3EB63385782B Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pj1-x1034.google.com with SMTP id v13-20020a17090a6b0d00b00219c3be9830so11493196pjj.4 for ; Mon, 09 Jan 2023 15:39:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-disposition:mime-version:message-id:subject:to:from:date :from:to:cc:subject:date:message-id:reply-to; bh=YYZluyOxd6OG5uZT7XBW6rxXHZpSdzBFXA/DHPtEFSk=; b=BLiYqO3lpm51oV5RFK5AyoxT11NmK4iWRWlsmyOMVM5rk4d2syaVZ8v/xFSWTDC2nc jeyzdwkxtcXCL2Qmpy0YJZ2PGMEcbu4YHlwdUGcuJH5bIdWOdxJH8qT5rTqZKo+atDXy C9WasjR7hbNVl2+Yot3UN4sknnT3bw+SZwKNFH7tOpjf7Q67fukUXKMQM4pbA7rBsSub 77jBeJYJcunwAAUFpmApeDvJhc4rf/c2YCqlk6WhcVW8l+KRge8V1AZYdlZ5vxvF9oYc ajDvLRmkZN8ONR4S9Z7KKpM8T5ldCs7rnBqG2vQZgTTLTZPRSE1v14t2Q6hD0UTN8/ui 3cPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-disposition:mime-version:message-id:subject:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=YYZluyOxd6OG5uZT7XBW6rxXHZpSdzBFXA/DHPtEFSk=; b=z3SdZED3HWOnmgG3aOBtMOPHiG+9ZFT9Cz0t7Ulyv18vUvGx1xP77But/JJDceDtGC 6BH+Ex9+GaWjJtTrSFplYy582KY3px3dQfsTfAeXbif0dMoqed/ijQWShwtIMsg8Hnxi 6nOGNCdQ2Gs/uOo0CsQA5/ga8VDPQB4zjJrF2ymJh8ub1zpzJQC0hGRGdWJyDMvRvppJ UhSUr44saLt3ektAP3piOkia2UezCW83fZhNU+drbBWlECSUWvNxDnPZdGNanijISpHL X9gtkcma+c42ECw6rTjbKDX9UyUZ8hT+A8T94LHnlEmNjcOT10vcBM3CdPLbrfpKtAL6 Rpjw== X-Gm-Message-State: AFqh2kpEvtFmSPmXv4fRkH3N1ypKcwTDn7fzLKuTOloEUn1hnjp+gxNS 2dJ7wab/E6TTsN0lsrQR8IysPsB7upw= X-Google-Smtp-Source: AMrXdXtrY1WQVnxV/sF1MAxzE+dp5+oHWFwpWG7JRl9phCrg70DilU2KLoLaV7Rz/qc732xNN+91wA== X-Received: by 2002:a17:90a:df8e:b0:221:6899:4485 with SMTP id p14-20020a17090adf8e00b0022168994485mr71685353pjv.15.1673307562045; Mon, 09 Jan 2023 15:39:22 -0800 (PST) Received: from squeak.grove.modra.org ([2406:3400:51d:8cc0:86e3:29a7:f00c:acc1]) by smtp.gmail.com with ESMTPSA id p10-20020a17090a348a00b00218d894fac3sm7938207pjb.3.2023.01.09.15.39.21 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Jan 2023 15:39:21 -0800 (PST) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id 4293611407EA; Tue, 10 Jan 2023 10:09:19 +1030 (ACDT) Date: Tue, 10 Jan 2023 10:09:19 +1030 From: Alan Modra To: binutils@sourceware.org Subject: peXXigen.c sanity checks Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3035.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Also fix a memory leak, and make some style changes. I tend to read (sizeof * x) as a multiplication of two variables, which I would not do if binutils followed the gcc coding conventions consistently (see https://gcc.gnu.org/codingconventions.html#Expressions). (sizeof *x) looks a lot better to me, or even (sizeof (*x)) which I've used here. * peXXigen.c (get_contents_sanity_check): New function. (pe_print_idata): Use it here.. (pe_print_edata): ..and here. Free data on error return. (rsrc_parse_entry): Check entry size read from file. (rsrc_parse_entries): Style fixes. (rsrc_process_section): Use bfd_malloc_and_get_section. (_bfd_XXi_final_link_postscript): Likewise. diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c index 292f7f76a0f..fa2b4296e86 100644 --- a/bfd/peXXigen.c +++ b/bfd/peXXigen.c @@ -1247,6 +1247,24 @@ static char * dir_names[IMAGE_NUMBEROF_DIRECTORY_ENTRIES] = N_("Reserved") }; +static bool +get_contents_sanity_check (bfd *abfd, asection *section, + bfd_size_type dataoff, bfd_size_type datasize) +{ + if ((section->flags & SEC_HAS_CONTENTS) == 0) + return false; + if (dataoff > section->size + || datasize > section->size - dataoff) + return false; + ufile_ptr filesize = bfd_get_file_size (abfd); + if (filesize != 0 + && ((ufile_ptr) section->filepos > filesize + || dataoff > filesize - section->filepos + || datasize > filesize - section->filepos - dataoff)) + return false; + return true; +} + static bool pe_print_idata (bfd * abfd, void * vfile) { @@ -1413,6 +1431,9 @@ pe_print_idata (bfd * abfd, void * vfile) { ft_idx = first_thunk - (ft_section->vma - extra->ImageBase); ft_datasize = ft_section->size - ft_idx; + if (!get_contents_sanity_check (abfd, ft_section, + ft_idx, ft_datasize)) + continue; ft_data = (bfd_byte *) bfd_malloc (ft_datasize); if (ft_data == NULL) continue; @@ -1582,24 +1603,9 @@ pe_print_edata (bfd * abfd, void * vfile) _("\nThere is an export table, but the section containing it could not be found\n")); return true; } - else if (!(section->flags & SEC_HAS_CONTENTS)) - { - fprintf (file, - _("\nThere is an export table in %s, but that section has no contents\n"), - section->name); - return true; - } dataoff = addr - section->vma; datasize = extra->DataDirectory[PE_EXPORT_TABLE].Size; - if (dataoff > section->size - || datasize > section->size - dataoff) - { - fprintf (file, - _("\nThere is an export table in %s, but it does not fit into that section\n"), - section->name); - return true; - } } /* PR 17512: Handle corrupt PE binaries. */ @@ -1612,6 +1618,14 @@ pe_print_edata (bfd * abfd, void * vfile) return true; } + if (!get_contents_sanity_check (abfd, section, dataoff, datasize)) + { + fprintf (file, + _("\nThere is an export table in %s, but contents cannot be read\n"), + section->name); + return true; + } + /* xgettext:c-format */ fprintf (file, _("\nThere is an export table in %s at 0x%lx\n"), section->name, (unsigned long) addr); @@ -1622,7 +1636,10 @@ pe_print_edata (bfd * abfd, void * vfile) if (! bfd_get_section_contents (abfd, section, data, (file_ptr) dataoff, datasize)) - return false; + { + free (data); + return false; + } /* Go get Export Directory Table. */ edt.export_flags = bfd_get_32 (abfd, data + 0); @@ -3333,7 +3350,7 @@ rsrc_parse_entry (bfd *abfd, if (HighBitSet (val)) { entry->is_dir = true; - entry->value.directory = bfd_malloc (sizeof * entry->value.directory); + entry->value.directory = bfd_malloc (sizeof (*entry->value.directory)); if (entry->value.directory == NULL) return dataend; @@ -3344,12 +3361,12 @@ rsrc_parse_entry (bfd *abfd, } entry->is_dir = false; - entry->value.leaf = bfd_malloc (sizeof * entry->value.leaf); + entry->value.leaf = bfd_malloc (sizeof (*entry->value.leaf)); if (entry->value.leaf == NULL) return dataend; data = datastart + val; - if (data < datastart || data >= dataend) + if (data < datastart || data + 12 > dataend) return dataend; addr = bfd_get_32 (abfd, data); @@ -3357,6 +3374,8 @@ rsrc_parse_entry (bfd *abfd, entry->value.leaf->codepage = bfd_get_32 (abfd, data + 8); /* FIXME: We assume that the reserved field (data + 12) is OK. */ + if (size > dataend - datastart - (addr - rva_bias)) + return dataend; entry->value.leaf->data = bfd_malloc (size); if (entry->value.leaf->data == NULL) return dataend; @@ -3385,7 +3404,7 @@ rsrc_parse_entries (bfd *abfd, return highest_data; } - entry = bfd_malloc (sizeof * entry); + entry = bfd_malloc (sizeof (*entry)); if (entry == NULL) return dataend; @@ -3404,7 +3423,7 @@ rsrc_parse_entries (bfd *abfd, if (i) { - entry->next_entry = bfd_malloc (sizeof * entry); + entry->next_entry = bfd_malloc (sizeof (*entry)); entry = entry->next_entry; if (entry == NULL) return dataend; @@ -4192,13 +4211,7 @@ rsrc_process_section (bfd * abfd, rva_bias = sec->vma - pe->pe_opthdr.ImageBase; - data = bfd_malloc (size); - if (data == NULL) - return; - - datastart = data; - - if (! bfd_get_section_contents (abfd, sec, data, 0, size)) + if (! bfd_malloc_and_get_section (abfd, sec, &datastart)) goto end; /* Step zero: Scan the input bfds looking for .rsrc sections and record @@ -4210,7 +4223,8 @@ rsrc_process_section (bfd * abfd, at the end of a variable amount. (It does not appear to be based upon the section alignment or the file alignment). We need to skip any padding bytes when parsing the input .rsrc sections. */ - rsrc_sizes = bfd_malloc (max_num_input_rsrc * sizeof * rsrc_sizes); + data = datastart; + rsrc_sizes = bfd_malloc (max_num_input_rsrc * sizeof (*rsrc_sizes)); if (rsrc_sizes == NULL) goto end; @@ -4227,7 +4241,7 @@ rsrc_process_section (bfd * abfd, { max_num_input_rsrc += 10; rsrc_sizes = bfd_realloc (rsrc_sizes, max_num_input_rsrc - * sizeof * rsrc_sizes); + * sizeof (*rsrc_sizes)); if (rsrc_sizes == NULL) goto end; } @@ -4280,7 +4294,7 @@ rsrc_process_section (bfd * abfd, data = datastart; rva_bias = sec->vma - pe->pe_opthdr.ImageBase; - type_tables = bfd_malloc (num_resource_sets * sizeof * type_tables); + type_tables = bfd_malloc (num_resource_sets * sizeof (*type_tables)); if (type_tables == NULL) goto end; @@ -4553,21 +4567,15 @@ _bfd_XXi_final_link_postscript (bfd * abfd, struct coff_final_link_info *pfinfo) if (sec) { bfd_size_type x = sec->rawsize; - bfd_byte *tmp_data = NULL; - - if (x) - tmp_data = bfd_malloc (x); + bfd_byte *tmp_data; - if (tmp_data != NULL) + if (bfd_malloc_and_get_section (abfd, sec, &tmp_data)) { - if (bfd_get_section_contents (abfd, sec, tmp_data, 0, x)) - { - qsort (tmp_data, - (size_t) (x / 12), - 12, sort_x64_pdata); - bfd_set_section_contents (pfinfo->output_bfd, sec, - tmp_data, 0, x); - } + qsort (tmp_data, + (size_t) (x / 12), + 12, sort_x64_pdata); + bfd_set_section_contents (pfinfo->output_bfd, sec, + tmp_data, 0, x); free (tmp_data); } else -- Alan Modra Australia Development Lab, IBM