* [binutils-gdb] PR29653, objcopy/strip: fuzzed small input file induces large output file
@ 2022-10-07 3:10 Alan Modra
0 siblings, 0 replies; only message in thread
From: Alan Modra @ 2022-10-07 3:10 UTC (permalink / raw)
To: bfd-cvs
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ea4e4a19b7f6c192c307b5a37c67d141f3aea074
commit ea4e4a19b7f6c192c307b5a37c67d141f3aea074
Author: Alan Modra <amodra@gmail.com>
Date: Fri Oct 7 10:23:05 2022 +1030
PR29653, objcopy/strip: fuzzed small input file induces large output file
_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.
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.
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 SectionAlignment,
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 = H_GET_32 (abfd, src->LoaderFlags);
a->NumberOfRvaAndSizes = H_GET_32 (abfd, src->NumberOfRvaAndSizes);
- {
- 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 = 0;
- }
-
- for (idx = 0; idx < a->NumberOfRvaAndSizes; idx++)
- {
- /* If data directory is empty, rva also should be 0. */
- int size =
- H_GET_32 (abfd, src->DataDirectory[idx][1]);
-
- a->DataDirectory[idx].Size = size;
+ /* PR 17512: Don't blindly trust NumberOfRvaAndSizes. */
+ unsigned idx;
+ for (idx = 0;
+ idx < a->NumberOfRvaAndSizes && idx < IMAGE_NUMBEROF_DIRECTORY_ENTRIES;
+ idx++)
+ {
+ /* If data directory is empty, rva also should be 0. */
+ int size = H_GET_32 (abfd, src->DataDirectory[idx][1]);
+ int vma = size ? H_GET_32 (abfd, src->DataDirectory[idx][0]) : 0;
- if (size)
- a->DataDirectory[idx].VirtualAddress =
- H_GET_32 (abfd, src->DataDirectory[idx][0]);
- else
- a->DataDirectory[idx].VirtualAddress = 0;
- }
+ a->DataDirectory[idx].Size = size;
+ a->DataDirectory[idx].VirtualAddress = vma;
+ }
- while (idx < IMAGE_NUMBEROF_DIRECTORY_ENTRIES)
- {
- a->DataDirectory[idx].Size = 0;
- a->DataDirectory[idx].VirtualAddress = 0;
- idx ++;
- }
- }
+ while (idx < IMAGE_NUMBEROF_DIRECTORY_ENTRIES)
+ {
+ a->DataDirectory[idx].Size = 0;
+ a->DataDirectory[idx].VirtualAddress = 0;
+ idx++;
+ }
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);
- bfd_set_error (bfd_error_no_error);
- bfd_coff_swap_aouthdr_in (abfd, opthdr, & internal_a);
- if (bfd_get_error () != bfd_error_no_error)
- return NULL;
- }
+ bfd_coff_swap_aouthdr_in (abfd, opthdr, &internal_a);
+
+ struct internal_extra_pe_aouthdr *a = &internal_a.pe;
+ if ((a->SectionAlignment & -a->SectionAlignment) != a->SectionAlignment
+ || a->SectionAlignment >= 0x80000000)
+ {
+ const char **warn = _bfd_per_xvec_warn (abfd->xvec);
+ *warn = _("%pB: adjusting invalid SectionAlignment");
+ a->SectionAlignment &= -a->SectionAlignment;
+ if (a->SectionAlignment >= 0x80000000)
+ a->SectionAlignment = 0x40000000;
+ }
+
+ if ((a->FileAlignment & -a->FileAlignment) != a->FileAlignment
+ || a->FileAlignment > a->SectionAlignment)
+ {
+ const char **warn = _bfd_per_xvec_warn (abfd->xvec);
+ *warn = _("%pB: adjusting invalid FileAlignment");
+ a->FileAlignment &= -a->FileAlignment;
+ if (a->FileAlignment > a->SectionAlignment)
+ a->FileAlignment = a->SectionAlignment;
+ }
+ if (a->NumberOfRvaAndSizes > IMAGE_NUMBEROF_DIRECTORY_ENTRIES)
+ {
+ const char **warn = _bfd_per_xvec_warn (abfd->xvec);
+ *warn = _("%pB: invalid NumberOfRvaAndSizes");
+ }
+ }
result = coff_real_object_p (abfd, internal_f.f_nscns, &internal_f,
(opt_hdr_size != 0
? &internal_a
: (struct internal_aouthdr *) NULL));
-
if (result)
{
/* Now the whole header has been processed, see if there is a build-id */
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-10-07 3:10 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-07 3:10 [binutils-gdb] PR29653, objcopy/strip: fuzzed small input file induces large output file Alan Modra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).