Daniel Koh Ji Yang via bug-diffutils via "All diffutils discussion."
<[email protected]> writes:
> A signed-overflow bug in diffutils 3.12, found by automated testing under
> UBSan. I don't have a local diffutils build; I confirmed the lines by reading
> the pinned 3.12 source (git.savannah.gnu.org, tag v3.12) and current master -
> both still have it - against the replay's output.
>
> diff's old-style numeric option (a bare "-<digits>", e.g. "diff -3") is
> parsed digit-by-digit in src/diff.c with checked arithmetic that correctly
> clamps an overflowing digit string to LIN_MAX (the max the `lin` type holds).
> That clamped value flows into context and then horizon_lines. The problem is
> that LIN_MAX itself reaches src/io.c:1206 in find_identical_ends(), which
> computes:
>
> lin i = horizon_lines + !((buffer0 == p0 || p0[-1] == '\n') && (buffer1 ==
> p1 || p1[-1] == '\n'));
>
> When horizon_lines == LIN_MAX and the boolean term is 1 (the common case),
> that's LIN_MAX + 1, which overflows.
>
> diff -u -999999999999999999999999999999 A B
> io.c:1206:29: runtime error: signed integer overflow: 9223372036854775807 +
> 1 cannot be represented in type 'long'
> find_identical_ends <- read_files <- diff_2_files <- ... <- main
>
> On a normal build this wraps to a large negative value that then bounds a
> "while (i-- && p0 != end0)" loop; I didn't establish whether that wrapped
> bound can walk end0 past the buffer (would need a non-trapping build to
> observe), so I'm reporting the overflow itself, not a memory-safety claim.
>
> Fix: keep the clamp below the type's true maximum (clamp old-style digit
> strings and --horizon-lines to LIN_MAX - 1, or whatever margin the largest
> downstream "+1" needs), or compute the boolean term first at io.c:1206 and
> use saturating addition, rather than relying on horizon_lines never actually
> reaching LIN_MAX.
>
> Found by replaying automatically generated tests under ASan/UBSan.
Thanks for the report. I attached a patch which should fix it. Limiting
the value to LIN_MAX - 1 seems simpler than trying to handle the
overflow when we see it. I also doubt anyone will complain about the
possible output context being lowered by 1 in this case.
It looks like it was introduced in 3.11, from the following commit:
$ git log 6341b346cd0c74c344f4e1c5c7f3556267547fe2 -n 1
commit 6341b346cd0c74c344f4e1c5c7f3556267547fe2
Author: Paul Eggert <[email protected]>
AuthorDate: Sat Jun 3 15:24:54 2023 -0700
Commit: Paul Eggert <[email protected]>
CommitDate: Sat Jun 3 15:50:33 2023 -0700
diff: don’t limit contexts arbitrarily
* src/context.c (find_hunk):
Check for overflow when computing non-ignorable threshold.
* src/diff.c (main): Limit ‘context’ to LIN_MAX since it is a lin.
* src/system.h (CONTEXT_MAX): Remove.
$ git tag --contains 6341b346cd0c74c344f4e1c5c7f3556267547fe2 | sort
--version-sort
v3.11
v3.12
Similar to you, I didn't see any memory saftey issues from my testing.
Things seemed to behave fine aside from UBSAN rightfully complaining.
Collin
>From 02ca82eb40713a8bc70071f7a2b3b8bd1adfc22c Mon Sep 17 00:00:00 2001
Message-ID: <02ca82eb40713a8bc70071f7a2b3b8bd1adfc22c.1790552134.git.collin.fu...@gmail.com>
From: Collin Funk <[email protected]>
Date: Sun, 27 Sep 2026 16:21:44 -0700
Subject: [PATCH] diff: fix an integer overflow
* NEWS: Mention the bug fix.
* src/diff.c (main): Limit OCONTEXT to LIN_MAX - 1.
* tests/basic: Add a test case that triggers UBSAN without the fix.
* THANKS: Add Daniel Koh Ji Yang for reporting the bug.
Fixes https://bugs.gnu.org/81937
---
NEWS | 3 +++
THANKS | 1 +
src/diff.c | 7 +++++--
tests/basic | 14 ++++++++++++++
4 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/NEWS b/NEWS
index f92bed9..a8288bc 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,9 @@ GNU diffutils NEWS -*- outline -*-
or unexpectedly truncated data.
[bugs present since "the beginning"]
+ diff -u no longer overflows a signed integer when PTRDIFF_MAX lines of
+ output context are requested.
+ [bug#81937 introduced in 3.11]
* Noteworthy changes in release 3.12 (2025-04-08) [stable]
diff --git a/THANKS b/THANKS
index a372954..61b95c8 100644
--- a/THANKS
+++ b/THANKS
@@ -21,4 +21,5 @@ Randy Smith <[email protected]>
Richard Stallman <[email protected]>
Leonard H. Tower Jr. <[email protected]>
Larry Wall <[email protected]>
+Daniel Koh Ji Yang <[email protected]>
Eli Zaretskii <[email protected]>
diff --git a/src/diff.c b/src/diff.c
index 002a22d..6ffeeea 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -360,9 +360,12 @@ main (int argc, char **argv)
case '9':
if (! c_isdigit (prev))
ocontext = 0;
+ /* Limit OCONTEXT to LIN_MAX - 1, so we can add 1 to
+ HORIZON_LINES in find_identical_ends. */
if (ckd_mul (&ocontext, ocontext, 10)
- || ckd_add (&ocontext, ocontext, c - '0'))
- ocontext = LIN_MAX;
+ || ckd_add (&ocontext, ocontext, c - '0')
+ || ocontext == LIN_MAX)
+ ocontext = LIN_MAX - 1;
break;
case 'a':
diff --git a/tests/basic b/tests/basic
index 161262d..519d165 100755
--- a/tests/basic
+++ b/tests/basic
@@ -39,4 +39,18 @@ for opt in '' -u -c; do
compare exp-$(echo $opt|tr ' ' _) out || fail=1
done
+# This would overflow a signed integer in diffutils-3.12 and earlier.
+returns_ 1 diff -u -9999999999999999999999999999 a b >out-t 2>err || fail=1
+# Remove date and time.
+sed -e 's/^\([-+*][-+*][-+*] [^ ]*\) .*/\1/' out-t >out || framework_failure_
+cat <<\EOF >exp || framework_failure
+--- a
++++ b
+@@ -1 +1 @@
+-a
++b
+EOF
+compare exp out || fail=1
+compare /dev/null err || fail=1
+
Exit $fail
--
2.55.0
