From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1062) id 846AD383DB93; Fri, 7 Oct 2022 03:10:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 846AD383DB93 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] PR29653, objcopy/strip: fuzzed small input file induces large output file X-Act-Checkin: binutils-gdb X-Git-Author: Alan Modra X-Git-Refname: refs/heads/master X-Git-Oldrev: fea044ba7b5ed9be93ce49b36188edbba7fcebb3 X-Git-Newrev: ea4e4a19b7f6c192c307b5a37c67d141f3aea074 Message-Id: <20221007031014.846AD383DB93@sourceware.org> Date: Fri, 7 Oct 2022 03:10:14 +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: Fri, 07 Oct 2022 03:10:14 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Dea4e4a19b7f6= c192c307b5a37c67d141f3aea074 commit ea4e4a19b7f6c192c307b5a37c67d141f3aea074 Author: Alan Modra Date: Fri Oct 7 10:23:05 2022 +1030 PR29653, objcopy/strip: fuzzed small input file induces large output fi= le =20 _bfd_check_format functions should not print errors or warnings if they return NULL. A NULL return means the particular target under test does not match, so there isn't any reason to make a complaint about the target. In fact there isn't a good reason to warn even if the target matches, except via the _bfd_per_xvec_warn mechanism; Some other target might be a better match. =20 This patch tidies pe_bfd_object_p with the above in mind, and restricts the PE optional header SectionAlignment and FileAlignment fields somewhat. I chose to warn on nonsense values rather than refusing to match. Refusing to match would be OK too. =20 PR 29653 * peXXigen.c (_bfd_XXi_swap_aouthdr_in): Don't emit error about invalid NumberOfRvaAndSizes here. Limit loop copying data directory to IMAGE_NUMBEROF_DIRECTORY_ENTRIES. * peicode.h (pe_bfd_object_p): Don't clear and test bfd_error around bfd_coff_swap_aouthdr_in. Warn on invalid SectionAlignm= ent, FileAlignment and NumberOfRvaAndSizes. Don't return NULL on invalid NumberOfRvaAndSizes. Diff: --- bfd/peXXigen.c | 55 ++++++++++++++++++------------------------------------- bfd/peicode.h | 34 ++++++++++++++++++++++++++++------ 2 files changed, 46 insertions(+), 43 deletions(-) diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c index a7b85713023..e74ed3968a2 100644 --- a/bfd/peXXigen.c +++ b/bfd/peXXigen.c @@ -517,45 +517,26 @@ _bfd_XXi_swap_aouthdr_in (bfd * abfd, a->LoaderFlags =3D H_GET_32 (abfd, src->LoaderFlags); a->NumberOfRvaAndSizes =3D H_GET_32 (abfd, src->NumberOfRvaAndSizes); =20 - { - unsigned idx; - - /* PR 17512: Corrupt PE binaries can cause seg-faults. */ - if (a->NumberOfRvaAndSizes > IMAGE_NUMBEROF_DIRECTORY_ENTRIES) - { - /* xgettext:c-format */ - _bfd_error_handler - (_("%pB: aout header specifies an invalid number of" - " data-directory entries: %u"), abfd, a->NumberOfRvaAndSizes); - bfd_set_error (bfd_error_bad_value); - - /* Paranoia: If the number is corrupt, then assume that the - actual entries themselves might be corrupt as well. */ - a->NumberOfRvaAndSizes =3D 0; - } - - for (idx =3D 0; idx < a->NumberOfRvaAndSizes; idx++) - { - /* If data directory is empty, rva also should be 0. */ - int size =3D - H_GET_32 (abfd, src->DataDirectory[idx][1]); - - a->DataDirectory[idx].Size =3D size; + /* PR 17512: Don't blindly trust NumberOfRvaAndSizes. */ + unsigned idx; + for (idx =3D 0; + idx < a->NumberOfRvaAndSizes && idx < IMAGE_NUMBEROF_DIRECTORY_ENTR= IES; + idx++) + { + /* If data directory is empty, rva also should be 0. */ + int size =3D H_GET_32 (abfd, src->DataDirectory[idx][1]); + int vma =3D size ? H_GET_32 (abfd, src->DataDirectory[idx][0]) : 0; =20 - if (size) - a->DataDirectory[idx].VirtualAddress =3D - H_GET_32 (abfd, src->DataDirectory[idx][0]); - else - a->DataDirectory[idx].VirtualAddress =3D 0; - } + a->DataDirectory[idx].Size =3D size; + a->DataDirectory[idx].VirtualAddress =3D vma; + } =20 - while (idx < IMAGE_NUMBEROF_DIRECTORY_ENTRIES) - { - a->DataDirectory[idx].Size =3D 0; - a->DataDirectory[idx].VirtualAddress =3D 0; - idx ++; - } - } + while (idx < IMAGE_NUMBEROF_DIRECTORY_ENTRIES) + { + a->DataDirectory[idx].Size =3D 0; + a->DataDirectory[idx].VirtualAddress =3D 0; + idx++; + } =20 if (aouthdr_int->entry) { diff --git a/bfd/peicode.h b/bfd/peicode.h index 54a159f0962..3888dd47cc6 100644 --- a/bfd/peicode.h +++ b/bfd/peicode.h @@ -1519,19 +1519,41 @@ pe_bfd_object_p (bfd * abfd) if (amt > opt_hdr_size) memset (opthdr + opt_hdr_size, 0, amt - opt_hdr_size); =20 - bfd_set_error (bfd_error_no_error); - bfd_coff_swap_aouthdr_in (abfd, opthdr, & internal_a); - if (bfd_get_error () !=3D bfd_error_no_error) - return NULL; - } + bfd_coff_swap_aouthdr_in (abfd, opthdr, &internal_a); + + struct internal_extra_pe_aouthdr *a =3D &internal_a.pe; + if ((a->SectionAlignment & -a->SectionAlignment) !=3D a->SectionAlig= nment + || a->SectionAlignment >=3D 0x80000000) + { + const char **warn =3D _bfd_per_xvec_warn (abfd->xvec); + *warn =3D _("%pB: adjusting invalid SectionAlignment"); + a->SectionAlignment &=3D -a->SectionAlignment; + if (a->SectionAlignment >=3D 0x80000000) + a->SectionAlignment =3D 0x40000000; + } + + if ((a->FileAlignment & -a->FileAlignment) !=3D a->FileAlignment + || a->FileAlignment > a->SectionAlignment) + { + const char **warn =3D _bfd_per_xvec_warn (abfd->xvec); + *warn =3D _("%pB: adjusting invalid FileAlignment"); + a->FileAlignment &=3D -a->FileAlignment; + if (a->FileAlignment > a->SectionAlignment) + a->FileAlignment =3D a->SectionAlignment; + } =20 + if (a->NumberOfRvaAndSizes > IMAGE_NUMBEROF_DIRECTORY_ENTRIES) + { + const char **warn =3D _bfd_per_xvec_warn (abfd->xvec); + *warn =3D _("%pB: invalid NumberOfRvaAndSizes"); + } + } =20 result =3D coff_real_object_p (abfd, internal_f.f_nscns, &internal_f, (opt_hdr_size !=3D 0 ? &internal_a : (struct internal_aouthdr *) NULL)); =20 - if (result) { /* Now the whole header has been processed, see if there is a build-= id */