From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1062) id 52BD63857BB3; Mon, 9 Jan 2023 23:40:30 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 52BD63857BB3 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Alan Modra To: bfd-cvs@sourceware.org Subject: [binutils-gdb] peXXigen.c sanity checks X-Act-Checkin: binutils-gdb X-Git-Author: Alan Modra X-Git-Refname: refs/heads/master X-Git-Oldrev: 5a671d7a854b4e4cf31837e423419654139a482d X-Git-Newrev: 10c386190cb8dcc398292b6053d5fbf6bfd3a4ff Message-Id: <20230109234030.52BD63857BB3@sourceware.org> Date: Mon, 9 Jan 2023 23:40:30 +0000 (GMT) X-BeenThere: binutils-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Jan 2023 23:40:30 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D10c386190cb8= dcc398292b6053d5fbf6bfd3a4ff commit 10c386190cb8dcc398292b6053d5fbf6bfd3a4ff Author: Alan Modra Date: Sat Jan 7 11:50:10 2023 +1030 peXXigen.c sanity checks =20 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. =20 * 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: --- bfd/peXXigen.c | 96 +++++++++++++++++++++++++++++++-----------------------= ---- 1 file changed, 52 insertions(+), 44 deletions(-) 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_ENT= RIES] =3D N_("Reserved") }; =20 +static bool +get_contents_sanity_check (bfd *abfd, asection *section, + bfd_size_type dataoff, bfd_size_type datasize) +{ + if ((section->flags & SEC_HAS_CONTENTS) =3D=3D 0) + return false; + if (dataoff > section->size + || datasize > section->size - dataoff) + return false; + ufile_ptr filesize =3D bfd_get_file_size (abfd); + if (filesize !=3D 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 =3D first_thunk - (ft_section->vma - extra->ImageBase); ft_datasize =3D ft_section->size - ft_idx; + if (!get_contents_sanity_check (abfd, ft_section, + ft_idx, ft_datasize)) + continue; ft_data =3D (bfd_byte *) bfd_malloc (ft_datasize); if (ft_data =3D=3D NULL) continue; @@ -1582,24 +1603,9 @@ pe_print_edata (bfd * abfd, void * vfile) _("\nThere is an export table, but the section containing it could no= t 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; - } =20 dataoff =3D addr - section->vma; datasize =3D 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 se= ction\n"), - section->name); - return true; - } } =20 /* PR 17512: Handle corrupt PE binaries. */ @@ -1612,6 +1618,14 @@ pe_print_edata (bfd * abfd, void * vfile) return true; } =20 + 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) =20 if (! bfd_get_section_contents (abfd, section, data, (file_ptr) dataoff, datasize)) - return false; + { + free (data); + return false; + } =20 /* Go get Export Directory Table. */ edt.export_flags =3D bfd_get_32 (abfd, data + 0); @@ -3333,7 +3350,7 @@ rsrc_parse_entry (bfd *abfd, if (HighBitSet (val)) { entry->is_dir =3D true; - entry->value.directory =3D bfd_malloc (sizeof * entry->value.directo= ry); + entry->value.directory =3D bfd_malloc (sizeof (*entry->value.directo= ry)); if (entry->value.directory =3D=3D NULL) return dataend; =20 @@ -3344,12 +3361,12 @@ rsrc_parse_entry (bfd *abfd, } =20 entry->is_dir =3D false; - entry->value.leaf =3D bfd_malloc (sizeof * entry->value.leaf); + entry->value.leaf =3D bfd_malloc (sizeof (*entry->value.leaf)); if (entry->value.leaf =3D=3D NULL) return dataend; =20 data =3D datastart + val; - if (data < datastart || data >=3D dataend) + if (data < datastart || data + 12 > dataend) return dataend; =20 addr =3D bfd_get_32 (abfd, data); @@ -3357,6 +3374,8 @@ rsrc_parse_entry (bfd *abfd, entry->value.leaf->codepage =3D bfd_get_32 (abfd, data + 8); /* FIXME: We assume that the reserved field (data + 12) is OK. */ =20 + if (size > dataend - datastart - (addr - rva_bias)) + return dataend; entry->value.leaf->data =3D bfd_malloc (size); if (entry->value.leaf->data =3D=3D NULL) return dataend; @@ -3385,7 +3404,7 @@ rsrc_parse_entries (bfd *abfd, return highest_data; } =20 - entry =3D bfd_malloc (sizeof * entry); + entry =3D bfd_malloc (sizeof (*entry)); if (entry =3D=3D NULL) return dataend; =20 @@ -3404,7 +3423,7 @@ rsrc_parse_entries (bfd *abfd, =20 if (i) { - entry->next_entry =3D bfd_malloc (sizeof * entry); + entry->next_entry =3D bfd_malloc (sizeof (*entry)); entry =3D entry->next_entry; if (entry =3D=3D NULL) return dataend; @@ -4192,13 +4211,7 @@ rsrc_process_section (bfd * abfd, =20 rva_bias =3D sec->vma - pe->pe_opthdr.ImageBase; =20 - data =3D bfd_malloc (size); - if (data =3D=3D NULL) - return; - - datastart =3D data; - - if (! bfd_get_section_contents (abfd, sec, data, 0, size)) + if (! bfd_malloc_and_get_section (abfd, sec, &datastart)) goto end; =20 /* 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 =3D bfd_malloc (max_num_input_rsrc * sizeof * rsrc_sizes); + data =3D datastart; + rsrc_sizes =3D bfd_malloc (max_num_input_rsrc * sizeof (*rsrc_sizes)); if (rsrc_sizes =3D=3D NULL) goto end; =20 @@ -4227,7 +4241,7 @@ rsrc_process_section (bfd * abfd, { max_num_input_rsrc +=3D 10; rsrc_sizes =3D bfd_realloc (rsrc_sizes, max_num_input_rsrc - * sizeof * rsrc_sizes); + * sizeof (*rsrc_sizes)); if (rsrc_sizes =3D=3D NULL) goto end; } @@ -4280,7 +4294,7 @@ rsrc_process_section (bfd * abfd, data =3D datastart; rva_bias =3D sec->vma - pe->pe_opthdr.ImageBase; =20 - type_tables =3D bfd_malloc (num_resource_sets * sizeof * type_tables); + type_tables =3D bfd_malloc (num_resource_sets * sizeof (*type_tables)); if (type_tables =3D=3D NULL) goto end; =20 @@ -4553,21 +4567,15 @@ _bfd_XXi_final_link_postscript (bfd * abfd, struct = coff_final_link_info *pfinfo) if (sec) { bfd_size_type x =3D sec->rawsize; - bfd_byte *tmp_data =3D NULL; - - if (x) - tmp_data =3D bfd_malloc (x); + bfd_byte *tmp_data; =20 - if (tmp_data !=3D 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