PR32399, buffer overflow printing core_file_failing_command
Checks
Commit Message
Assorted targets do not check, as the ELF targets do, that the program
name in a core file is NUL terminated. Fix some of them. I haven't
attempted to fix all targets because editing host specific code can
easily result in build bugs, which aren't discovered until someone
build binutils for that host. (Of the files edited here, I can't
easily compile hpux-core.c and osf-core.c on a linux system.)
PR 32399
* hppabsd-core.c (hppabsd_core_core_file_p): Ensure core_command
string is terminated.
* hpux-core.c (hpux_core_core_file_p): Likewise.
* irix-core.c (irix_core_core_file_p): Likewise.
* lynx-core.c (lynx_core_file_p): Likewise.
* osf-core.c (osf_core_core_file_p): Likewise.
* mach-o.c (bfd_mach_o_core_file_failing_command): Likewise.
Comments
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 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_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 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_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 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_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 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;
@@ -179,7 +179,8 @@ hppabsd_core_core_file_p (bfd *abfd)
goto fail;
core_regsec (abfd)->vma = 0;
- strncpy (core_command (abfd), u.u_comm, MAXCOMLEN + 1);
+ strncpy (core_command (abfd), u.u_comm, MAXCOMLEN);
+ core_command (abfd)[MAXCOMLEN] = 0;
core_signal (abfd) = u.u_code;
return _bfd_no_cleanup;
@@ -177,7 +177,8 @@ hpux_core_core_file_p (bfd *abfd)
struct proc_exec proc_exec;
if (bfd_read (&proc_exec, core_header.len, abfd) != core_header.len)
break;
- strncpy (core_command (abfd), proc_exec.cmd, MAXCOMLEN + 1);
+ strncpy (core_command (abfd), proc_exec.cmd, MAXCOMLEN);
+ core_command (abfd)[MAXCOMLEN] = 0;
good_sections++;
}
break;
@@ -203,7 +203,8 @@ irix_core_core_file_p (bfd *abfd)
if (!core_hdr (abfd))
return NULL;
- strncpy (core_command (abfd), coreout.c_name, CORE_NAMESIZE);
+ strncpy (core_command (abfd), coreout.c_name, CORE_NAMESIZE - 1);
+ core_command (abfd)[CORE_NAMESIZE - 1] = 0;
core_signal (abfd) = coreout.c_sigcause;
if (bfd_seek (abfd, coreout.c_vmapoffset, SEEK_SET) != 0)
@@ -120,7 +120,8 @@ lynx_core_file_p (bfd *abfd)
if (!core_hdr (abfd))
return NULL;
- strncpy (core_command (abfd), pss.pname, PNMLEN + 1);
+ strncpy (core_command (abfd), pss.pname, PNMLEN);
+ core_command (abfd)[PNMLEN] = 0;
/* Compute the size of the thread contexts */
@@ -6019,9 +6019,9 @@ bfd_mach_o_core_file_failing_command (bfd *abfd)
int ret;
ret = bfd_mach_o_core_fetch_environment (abfd, &buf, &len);
- if (ret < 0)
+ if (ret < 0 || len == 0)
return NULL;
-
+ buf[len - 1] = 0;
return (char *) buf;
}
@@ -92,7 +92,8 @@ osf_core_core_file_p (bfd *abfd)
if (!core_hdr (abfd))
return NULL;
- strncpy (core_command (abfd), core_header.name, MAXCOMLEN + 1);
+ strncpy (core_command (abfd), core_header.name, MAXCOMLEN);
+ core_command (abfd)[MAXCOMLEN] = 0;
core_signal (abfd) = core_header.signo;
for (i = 0; i < core_header.nscns; i++)