PR32399, buffer overflow printing core_file_failing_command
Alan Modra
amodra@gmail.com
Sun Dec 1 13:38:13 GMT 2024
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;
--
Alan Modra
More information about the Binutils
mailing list