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.

diff --git a/bfd/aix5ppc-core.c b/bfd/aix5ppc-core.c
index 179a7bf5b785c2d134af410eb68b523ee0bcdd08..a6d6449fc571b6b497940b40e0f3a07a9ce96a44 100644 (file)
--- 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_read (&core, sizeof (struct core_dumpxx), abfd))
+  if (sizeof core != bfd_read (&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 75b11150f6dd2bf5df48b7ea05b97eacba5cbc0f..1bbb44192ffda88fd3c6e139727fa2822377a550 100644 (file)
--- 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 647af9d7bc233d6ddf327486e84d5a83ccf1efc0..ae56f3913e41af03b77624de8b03b3d23408aefe 100644 (file)
--- 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_read (&core, amt, abfd);
+  val = bfd_read (&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 426d6070dc8bd0cccfc30dcff585e119d046c737..5952c06f8b62f70216ad226a6db6e1b22106b182 100644 (file)
--- 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_read (&u, 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 19b9f46631facee61b621f5d64fa65f851fe167e..ac8b29838ad18aaf4a405ad1338d43640eeb63bb 100644 (file)
--- 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 012bc4bdd014912da6fd0fab70146f1eae4b3447..06b6bdadd879a25f17cf38d258cc5f6614877b8d 100644 (file)
--- 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;