From: Alan Modra <amodra@gmail.com>
To: binutils@sourceware.org
Subject: Re: asan: buffer overflow in peXXigen.c
Date: Thu, 17 Mar 2022 21:28:51 +1030 [thread overview]
Message-ID: <YjMUa1B2Dvtj6pfu@squeak.grove.modra.org> (raw)
In the process of fixing a buffer overflow in commit fe69d4fcf0194a,
I managed to introduce a fairly obvious NULL pointer dereference..
* peXXigen.c (_bfd_XX_bfd_copy_private_bfd_data_common): Don't
segfault on not finding section. Wrap overlong lines.
diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index d11ea01c554..50e4face50c 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -2984,64 +2984,72 @@ _bfd_XX_bfd_copy_private_bfd_data_common (bfd * ibfd, bfd * obfd)
one. */
bfd_vma last = addr + size - 1;
asection *section = find_section_by_vma (obfd, last);
- bfd_byte *data;
- bfd_vma dataoff = addr - section->vma;
- /* PR 17512: file: 0f15796a. */
- if (section
- && (addr < section->vma
- || section->size < dataoff
- || section->size - dataoff < size))
+ if (section != NULL)
{
- /* xgettext:c-format */
- _bfd_error_handler
- (_("%pB: Data Directory (%lx bytes at %" PRIx64 ") "
- "extends across section boundary at %" PRIx64),
- obfd, ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size,
- (uint64_t) addr, (uint64_t) section->vma);
- return false;
- }
+ bfd_byte *data;
+ bfd_vma dataoff = addr - section->vma;
- if (section && bfd_malloc_and_get_section (obfd, section, &data))
- {
- unsigned int i;
- struct external_IMAGE_DEBUG_DIRECTORY *dd =
- (struct external_IMAGE_DEBUG_DIRECTORY *)(data + dataoff);
+ /* PR 17512: file: 0f15796a. */
+ if (addr < section->vma
+ || section->size < dataoff
+ || section->size - dataoff < size)
+ {
+ /* xgettext:c-format */
+ _bfd_error_handler
+ (_("%pB: Data Directory (%lx bytes at %" PRIx64 ") "
+ "extends across section boundary at %" PRIx64),
+ obfd, ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size,
+ (uint64_t) addr, (uint64_t) section->vma);
+ return false;
+ }
- for (i = 0; i < ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size
- / sizeof (struct external_IMAGE_DEBUG_DIRECTORY); i++)
+ if (bfd_malloc_and_get_section (obfd, section, &data))
{
- asection *ddsection;
- struct external_IMAGE_DEBUG_DIRECTORY *edd = &(dd[i]);
- struct internal_IMAGE_DEBUG_DIRECTORY idd;
+ unsigned int i;
+ struct external_IMAGE_DEBUG_DIRECTORY *dd =
+ (struct external_IMAGE_DEBUG_DIRECTORY *)(data + dataoff);
+
+ for (i = 0; i < ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size
+ / sizeof (struct external_IMAGE_DEBUG_DIRECTORY); i++)
+ {
+ asection *ddsection;
+ struct external_IMAGE_DEBUG_DIRECTORY *edd = &(dd[i]);
+ struct internal_IMAGE_DEBUG_DIRECTORY idd;
+ bfd_vma idd_vma;
- _bfd_XXi_swap_debugdir_in (obfd, edd, &idd);
+ _bfd_XXi_swap_debugdir_in (obfd, edd, &idd);
- if (idd.AddressOfRawData == 0)
- continue; /* RVA 0 means only offset is valid, not handled yet. */
+ /* RVA 0 means only offset is valid, not handled yet. */
+ if (idd.AddressOfRawData == 0)
+ continue;
- ddsection = find_section_by_vma (obfd, idd.AddressOfRawData + ope->pe_opthdr.ImageBase);
- if (!ddsection)
- continue; /* Not in a section! */
+ idd_vma = idd.AddressOfRawData + ope->pe_opthdr.ImageBase;
+ ddsection = find_section_by_vma (obfd, idd_vma);
+ if (!ddsection)
+ continue; /* Not in a section! */
- idd.PointerToRawData = ddsection->filepos + (idd.AddressOfRawData
- + ope->pe_opthdr.ImageBase) - ddsection->vma;
+ idd.PointerToRawData
+ = ddsection->filepos + idd_vma - ddsection->vma;
+ _bfd_XXi_swap_debugdir_out (obfd, &idd, edd);
+ }
- _bfd_XXi_swap_debugdir_out (obfd, &idd, edd);
+ if (!bfd_set_section_contents (obfd, section, data, 0,
+ section->size))
+ {
+ _bfd_error_handler (_("failed to update file offsets"
+ " in debug directory"));
+ free (data);
+ return false;
+ }
+ free (data);
}
-
- if (!bfd_set_section_contents (obfd, section, data, 0, section->size))
+ else
{
- _bfd_error_handler (_("failed to update file offsets in debug directory"));
- free (data);
+ _bfd_error_handler (_("%pB: failed to read "
+ "debug data section"), obfd);
return false;
}
- free (data);
- }
- else if (section)
- {
- _bfd_error_handler (_("%pB: failed to read debug data section"), obfd);
- return false;
}
}
--
Alan Modra
Australia Development Lab, IBM
next reply other threads:[~2022-03-17 10:58 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-17 10:58 Alan Modra [this message]
-- strict thread matches above, loose matches on Subject: below --
2022-02-16 11:30 Alan Modra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YjMUa1B2Dvtj6pfu@squeak.grove.modra.org \
--to=amodra@gmail.com \
--cc=binutils@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).