bug#78880: od Heap-buffer overflow

From 617220e970f267fbeea80d5cd8b62aec2ba7eaf6 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Sun, 29 Jun 2025 18:06:22 -0700
Subject: [PATCH 1/3] od: refactor parse_old_offset

* src/od.c (parse_old_offset): Refactor for brevity and clarity.
---
 src/od.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/src/od.c b/src/od.c
index 3381ee484..8acf205e1 100644
--- a/src/od.c
+++ b/src/od.c
@@ -1416,43 +1416,29 @@ xstr2nonneg (char const *restrict nptr, int base, intmax_t *val,
 static bool
 parse_old_offset (char *s, intmax_t *offset)
 {
-  int radix;
-
   if (*s == '\0')
     return false;
 
-  /* Skip over any leading '+'. */
-  if (s[0] == '+')
-    ++s;
+  /* Skip over any leading '+'.  */
+  s += s[0] == '+';
 
-  /* Determine the radix we'll use to interpret S.  If there is a '.',
-     optionally followed by 'B' or 'b' and then end of string,
-     it's decimal, otherwise, if the string begins with '0X'or '0x',
+  /* Determine the radix for S.  If there is a '.', followed first by
+     optional 'b' or (undocumented) 'B' and then by end of string,
+     it's decimal, otherwise, if the string begins with '0X' or '0x',
      it's hexadecimal, else octal.  */
   char *dot = strchr (s, '.');
-  if (dot)
-    {
-      bool b = dot[1] == 'B' || dot[1] == 'b';
-      if (dot[b + 1])
-        dot = nullptr;
-    }
+  if (dot && dot[(dot[1] == 'b' || dot[1] == 'B') + 1])
+    dot = nullptr;
+  int radix = dot ? 10 : s[0] == '0' && (s[1] == 'x' || s[1] == 'X') ? 16 : 8;
 
   if (dot)
     {
       /* Temporarily remove the '.' from the decimal string.  */
       dot[0] = dot[1];
       dot[1] = '\0';
-      radix = 10;
-    }
-  else
-    {
-      if (s[0] == '0' && (s[1] == 'x' || s[1] == 'X'))
-        radix = 16;
-      else
-        radix = 8;
     }
 
-  enum strtol_error s_err = xstr2nonneg (s, radix, offset, "Bb");
+  enum strtol_error s_err = xstr2nonneg (s, radix, offset, "bB");
 
   if (dot)
     {
-- 
2.48.1

From 9002c04ccc51c753ef801d141810932ca9de41f3 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Sun, 29 Jun 2025 22:25:28 -0700
Subject: [PATCH 2/3] od: more minor fixes for offsets

* src/od.c (parse_old_offset): Reject invalid offsets like "++0".
Treat overflowing offsets as errors, not as file names.
---
 src/od.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/od.c b/src/od.c
index 8acf205e1..918d56725 100644
--- a/src/od.c
+++ b/src/od.c
@@ -1409,18 +1409,20 @@ xstr2nonneg (char const *restrict nptr, int base, intmax_t *val,
   return s_err != LONGINT_INVALID && *val < 0 ? LONGINT_INVALID : s_err;
 }
 
-/* If S is a valid traditional offset specification with an optional
-   leading '+' return true and set *OFFSET to the offset it denotes.
-   Otherwise return false and possibly set *OFFSET.  */
+/* If STR is a valid traditional offset specification with an optional
+   leading '+', and can be represented in intmax_t, return true and
+   set *OFFSET to the offset it denotes.  If it is valid but cannot be
+   represented, diagnose the problem and exit.  Otherwise return false
+   and possibly set *OFFSET.  */
 
 static bool
-parse_old_offset (char *s, intmax_t *offset)
+parse_old_offset (char *str, intmax_t *offset)
 {
-  if (*s == '\0')
-    return false;
-
   /* Skip over any leading '+'.  */
-  s += s[0] == '+';
+  char *s = str + (str[0] == '+');
+
+  if (! c_isdigit (s[0]))
+    return false;
 
   /* Determine the radix for S.  If there is a '.', followed first by
      optional 'b' or (undocumented) 'B' and then by end of string,
@@ -1447,6 +1449,8 @@ parse_old_offset (char *s, intmax_t *offset)
       dot[0] = '.';
     }
 
+  if (s_err == LONGINT_OVERFLOW)
+    error (EXIT_FAILURE, ERANGE, "%s", quotearg_colon (str));
   return s_err == LONGINT_OK;
 }
 
-- 
2.48.1

From 5c5f069a4c8f33a0177acd021df2f4cad6860024 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Sun, 29 Jun 2025 22:31:01 -0700
Subject: [PATCH 3/3] od: add test cases for recent fix

* tests/od/od.pl: New tests for the offset issues
---
 NEWS           |  3 +++
 tests/od/od.pl | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/NEWS b/NEWS
index a85840dda..87ba2659c 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   od '+N.' (where N is a decimal number) works again as per POSIX.
   [bug introduced in textutils-2.0]
 
+  'od /dev/null ++0' no longer mistakenly treats the ++0 as an offset.
+  [This bug was present in "the beginning".]
+
   sort with key character offsets of SIZE_MAX, could induce
   a read of 1 byte before an allocated heap buffer. For example:
   'sort +0.18446744073709551615R input' on 64 bit systems.
diff --git a/tests/od/od.pl b/tests/od/od.pl
index 9688607c6..e4e328ad6 100755
--- a/tests/od/od.pl
+++ b/tests/od/od.pl
@@ -64,6 +64,24 @@ my @Tests =
      ['trad-dot1', '+1.', {IN_PIPE=>'a'}, {OUT=>"0000001\n"}],
      ['trad-dot512', '+1.b', {IN_PIPE => 'a' x 512}, {OUT=>"0001000\n"}],
 
+     # Invalid traditional offsets should be treated as file names.
+     ['invalid-off-1', '++0', {IN_PIPE=>""}, {EXIT=>1},
+      {ERR=>"$prog: ++0: No such file or directory\n"}],
+     ['invalid-off-2', '+-0', {IN_PIPE=>""}, {EXIT=>1},
+      {ERR=>"$prog: +-0: No such file or directory\n"}],
+     ['invalid-off-3', '"+ 0"', {IN_PIPE=>""}, {EXIT=>1},
+      {ERR=>"$prog: '+ 0': No such file or directory\n"}],
+     ['invalid-off-4', '-- -0', {IN_PIPE=>""}, {EXIT=>1},
+      {ERR=>"$prog: -0: No such file or directory\n"}],
+
+     # Overflowing traditional offsets should be diagnosed.
+     ['overflow-off-1', '-', '7' x 255, {IN_PIPE=>""}, {EXIT=>1},
+      {ERR=>"od: ".('7' x 255).": Numerical result out of range\n"}],
+     ['overflow-off-2', '-', ('9' x 254).'.', {IN_PIPE=>""}, {EXIT=>1},
+      {ERR=>"od: ".('9' x 254).".: Numerical result out of range\n"}],
+     ['overflow-off-3', '-', '0x'.('f' x 253), {IN_PIPE=>""}, {EXIT=>1},
+      {ERR=>"od: 0x".('f' x 253).": Numerical result out of range\n"}],
+
      # Ensure that a large width does not cause trouble.
      # From coreutils-7.0 through coreutils-8.21, these would print
      # approximately 128KiB of padding.
-- 
2.48.1