As sometimes happens (in my case, while walking the dog) I thought of
one or two more little problems in that area, and installed the attached
patches to fix them. The first patch merely refactors; the second one
does the fix; the third one adds test cases.From 617220e970f267fbeea80d5cd8b62aec2ba7eaf6 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
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 <egg...@cs.ucla.edu>
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 <egg...@cs.ucla.edu>
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
