From 8ab91a033555c5faae1bcd615800670b91673731 Mon Sep 17 00:00:00 2001
From: Alan Modra <amodra@gmail.com>
Date: Sat, 30 Nov 2024 16:41:14 +1030
Subject: [PATCH] Re: PR32399, buffer overflow printing
 core_file_failing_command

Fix more potential buffer overflows, and correct trad-code.c and
cisco-core.c where they should be using bfd_{z}alloc rather than
bfd_{z}malloc.  To stop buffer overflows with fuzzed objects that
don't have a terminator on the core_file_failing_command string, this
patch allocates an extra byte at the end of the entire header buffer
rather than poking a NUL at the end of the name array (u_comm[] or
similar) because (a) it's better to not overwrite the file data, and
(b) it is possible that some core files make use of fields in struct
user beyond the end of u_comm to extend the command name.  The patch
also changes some unnecessary uses of bfd_zalloc to bfd_alloc.
There's not much point in clearing memeory that will shortly be
completely overwritten.

	PR 32399
	* aix5ppc-core.c (xcoff64_core_p): Allocate an extra byte to
	ensure the core_file_failing_command string is terminated.
	* netbsd-core.c (netbsd_core_file_p): Likewise.
	* ptrace-core.c (ptrace_unix_core_file_p): Likewise.
	* rs6000-core.c (rs6000coff_core_p): Likewise.
	* trad-core.c (trad_unix_core_file_p): Likewise, and bfd_alloc
	tdata rather than bfd_zmalloc.
	* cisco-core.c (cisco_core_file_validate): bfd_zalloc tdata.
---
 bfd/aix5ppc-core.c | 15 ++++++++-------
 bfd/cisco-core.c   |  2 +-
 bfd/netbsd-core.c  | 14 ++++++++------
 bfd/ptrace-core.c  |  8 +++++---
 bfd/rs6000-core.c  |  5 ++++-
 bfd/trad-core.c    |  8 +++++---
 6 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/bfd/aix5ppc-core.c b/bfd/aix5ppc-core.c
index 179a7bf5b78..a6d6449fc57 100644
--- a/bfd/aix5ppc-core.c
+++ b/bfd/aix5ppc-core.c
@@ -66,8 +66,7 @@ xcoff64_core_p (bfd *abfd)
   if (bfd_seek (abfd, 0, SEEK_SET) != 0)
     goto xcoff64_core_p_error;
 
-  if (sizeof (struct core_dumpxx)
-      != bfd_bread (&core, sizeof (struct core_dumpxx), abfd))
+  if (sizeof core != bfd_bread (&core, sizeof core, abfd))
     goto xcoff64_core_p_error;
 
   if (bfd_stat (abfd, &statbuf) < 0)
@@ -111,14 +110,16 @@ xcoff64_core_p (bfd *abfd)
       return NULL;
     }
 
-  new_core_hdr = bfd_zalloc (abfd, sizeof (struct core_dumpxx));
+  new_core_hdr = bfd_alloc (abfd, sizeof (*new_core_hdr) + 1);
   if (NULL == new_core_hdr)
     return NULL;
 
-  memcpy (new_core_hdr, &core, sizeof (struct core_dumpxx));
-  /* The core_hdr() macro is no longer used here because it would
-     expand to code relying on gcc's cast-as-lvalue extension,
-     which was removed in gcc 4.0.  */
+  memcpy (new_core_hdr, &core, sizeof (*new_core_hdr));
+
+  /* Ensure core_file_failing_command string is terminated.  This is
+     just to stop buffer overflows on fuzzed files.  */
+  ((char *) new_core_hdr)[sizeof (*new_core_hdr)] = 0;
+
   abfd->tdata.any = new_core_hdr;
 
   /* .stack section.  */
diff --git a/bfd/cisco-core.c b/bfd/cisco-core.c
index 75b11150f6d..1bbb44192ff 100644
--- a/bfd/cisco-core.c
+++ b/bfd/cisco-core.c
@@ -154,7 +154,7 @@ cisco_core_file_validate (bfd *abfd, int crash_info_loc)
   /* OK, we believe you.  You're a core file.  */
 
   amt = sizeof (struct cisco_core_struct);
-  abfd->tdata.cisco_core_data = (struct cisco_core_struct *) bfd_zmalloc (amt);
+  abfd->tdata.cisco_core_data = bfd_zalloc (abfd, amt);
   if (abfd->tdata.cisco_core_data == NULL)
     return NULL;
 
diff --git a/bfd/netbsd-core.c b/bfd/netbsd-core.c
index 647af9d7bc2..ae56f3913e4 100644
--- a/bfd/netbsd-core.c
+++ b/bfd/netbsd-core.c
@@ -47,7 +47,7 @@
 struct netbsd_core_struct
 {
   struct core core;
-} *rawptr;
+};
 
 /* Handle NetBSD-style core dump file.  */
 
@@ -60,9 +60,9 @@ netbsd_core_file_p (bfd *abfd)
   asection *asect;
   struct core core;
   struct coreseg coreseg;
-  size_t amt = sizeof core;
+  struct netbsd_core_struct *rawptr;
 
-  val = bfd_bread (&core, amt, abfd);
+  val = bfd_bread (&core, sizeof core, abfd);
   if (val != sizeof core)
     {
       /* Too small to be a core file.  */
@@ -76,13 +76,15 @@ netbsd_core_file_p (bfd *abfd)
       return 0;
     }
 
-  amt = sizeof (struct netbsd_core_struct);
-  rawptr = (struct netbsd_core_struct *) bfd_zalloc (abfd, amt);
+  rawptr = bfd_alloc (abfd, sizeof (*rawptr) + 1);
   if (rawptr == NULL)
     return 0;
 
-  rawptr->core = core;
   abfd->tdata.netbsd_core_data = rawptr;
+  rawptr->core = core;
+  /* Ensure core_file_failing_command string is terminated.  This is
+     just to stop buffer overflows on fuzzed files.  */
+  ((char *) rawptr)[sizeof (*rawptr)] = 0;
 
   offset = core.c_hdrsize;
   for (i = 0; i < core.c_nseg; i++)
diff --git a/bfd/ptrace-core.c b/bfd/ptrace-core.c
index 426d6070dc8..5952c06f8b6 100644
--- a/bfd/ptrace-core.c
+++ b/bfd/ptrace-core.c
@@ -61,7 +61,6 @@ ptrace_unix_core_file_p (bfd *abfd)
   int val;
   struct ptrace_user u;
   struct trad_core_struct *rawptr;
-  size_t amt;
   flagword flags;
 
   val = bfd_bread ((void *)&u, (bfd_size_type) sizeof u, abfd);
@@ -77,8 +76,7 @@ ptrace_unix_core_file_p (bfd *abfd)
 
   /* Allocate both the upage and the struct core_data at once, so
      a single free() will free them both.  */
-  amt = sizeof (struct trad_core_struct);
-  rawptr = (struct trad_core_struct *) bfd_zalloc (abfd, amt);
+  rawptr = bfd_alloc (abfd, sizeof (*rawptr) + 1);
 
   if (rawptr == NULL)
     return 0;
@@ -87,6 +85,10 @@ ptrace_unix_core_file_p (bfd *abfd)
 
   rawptr->u = u; /*Copy the uarea into the tdata part of the bfd */
 
+  /* Ensure core_file_failing_command string is terminated.  This is
+     just to stop buffer overflows on fuzzed files.  */
+  ((char *) rawptr)[sizeof (*rawptr)] = 0;
+
   /* Create the sections.  */
 
   flags = SEC_ALLOC + SEC_LOAD + SEC_HAS_CONTENTS;
diff --git a/bfd/rs6000-core.c b/bfd/rs6000-core.c
index 19b9f46631f..ac8b29838ad 100644
--- a/bfd/rs6000-core.c
+++ b/bfd/rs6000-core.c
@@ -476,12 +476,15 @@ rs6000coff_core_p (bfd *abfd)
 #else
   size =  sizeof (core.new_dump);
 #endif
-  tmpptr = (char *) bfd_zalloc (abfd, (bfd_size_type) size);
+  tmpptr = bfd_alloc (abfd, size + 1);
   if (!tmpptr)
     return NULL;
 
   /* Copy core file header.  */
   memcpy (tmpptr, &core, size);
+  /* Ensure core_file_failing_command string is terminated.  This is
+     just to stop buffer overflows on fuzzed files.  */
+  tmpptr[size] = 0;
   set_tdata (abfd, tmpptr);
 
   /* Set architecture.  */
diff --git a/bfd/trad-core.c b/bfd/trad-core.c
index 012bc4bdd01..06b6bdadd87 100644
--- a/bfd/trad-core.c
+++ b/bfd/trad-core.c
@@ -65,7 +65,6 @@ trad_unix_core_file_p (bfd *abfd)
   int val;
   struct user u;
   struct trad_core_struct *rawptr;
-  size_t amt;
   flagword flags;
 
 #ifdef TRAD_CORE_USER_OFFSET
@@ -132,8 +131,7 @@ trad_unix_core_file_p (bfd *abfd)
 
   /* Allocate both the upage and the struct core_data at once, so
      a single free() will free them both.  */
-  amt = sizeof (struct trad_core_struct);
-  rawptr = (struct trad_core_struct *) bfd_zmalloc (amt);
+  rawptr = bfd_alloc (abfd, sizeof (*rawptr) + 1);
   if (rawptr == NULL)
     return 0;
 
@@ -141,6 +139,10 @@ trad_unix_core_file_p (bfd *abfd)
 
   rawptr->u = u; /*Copy the uarea into the tdata part of the bfd */
 
+  /* Ensure core_file_failing_command string is terminated.  This is
+     just to stop buffer overflows on fuzzed files.  */
+  ((char *) rawptr)[sizeof (*rawptr)] = 0;
+
   /* Create the sections.  */
 
   flags = SEC_ALLOC + SEC_LOAD + SEC_HAS_CONTENTS;
-- 
2.28.0.windows.1