Subject: Re: bug#78880: od Heap-buffer overflow

From: Pádraig Brady <[email-masked]>
Date: Tue, 24 Jun 2025 01:16:44 +0100

On 23/06/2025 09:24, Jaehoon Jang wrote:
> =================================================================
> ==1151699==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x6150000004f9 at pc 0x0000004d153f bp 0x7fff937f0410 sp 0x7fff937f0408
> WRITE of size 1 at 0x6150000004f9 thread T0
>      #0 0x4d153e in dump_strings coreutils/src/od.c:1570:14

Nice fuzzing.

There looks to be all sorts of off by one errors in the dump_strings() function.
The issue is most easily demonstrated with:

   printf '%100s' | tr ' ' . | valgrind od -N100 -S99

The following should fix this I think.
I've only analyzed it for a few minutes, so I'll look more tomorrow.
The following should also fix the printed offset,
and also support the -N100 -S100 combination.

thanks!

Pádraig.

diff --git a/src/od.c b/src/od.c
index 88d467c73..7f0cd5ab7 100644
--- a/src/od.c
+++ b/src/od.c
@@ -1513,7 +1513,7 @@ dump (void)
  static bool
  dump_strings (void)
  {
-  idx_t bufsize = MAX (100, string_min);
+  idx_t bufsize = MAX (100, string_min + 1);
    char *buf = xmalloc (bufsize);
    uintmax_t address = n_bytes_to_skip;
    bool ok = true;
@@ -1527,7 +1527,7 @@ dump_strings (void)
      tryline:

        if (limit_bytes_to_format
-          && (end_offset < string_min || end_offset - string_min <= address))
+          && (end_offset < string_min || end_offset - string_min < address))
          break;

        for (i = 0; i < string_min; i++)
@@ -1549,7 +1549,7 @@ dump_strings (void)
           Now see if it is terminated with a null byte.  */
        while (!limit_bytes_to_format || address < end_offset)
          {
-          if (i == bufsize)
+          if (i == bufsize - 1)
              buf = xpalloc (buf, &bufsize, 1, -1, sizeof *buf);
            ok &= read_char (&c);
            address++;
@@ -1568,7 +1568,7 @@ dump_strings (void)
        /* If we get here, the string is all printable and null-terminated,
           so print it.  It is all in 'buf' and 'i' is its length.  */
        buf[i] = 0;
-      format_address (address - i - 1, ' ');
+      format_address (address - i, ' ');

        for (i = 0; (c = buf[i]); i++)
          {