On 6/13/24 05:34, Wasser Mai wrote:
Error: INTEGER_OVERFLOW (CWE-190):
diffutils-3.10/lib/stackvma.c:198:23: tainted_data_return: Called
function ""read(fd, rof->buffer + rof->filled, size - rof->filled)"",
and a possible return value may be less than zero.
diffutils-3.10/lib/stackvma.c:198:23: cast_overflow: An assign that
casts to a different type, which might trigger an overflow.
diffutils-3.10/lib/stackvma.c:213:23: overflow: The expression
""rof->filled"" is considered to have possibly overflowed.
diffutils-3.10/lib/stackvma.c:198:23: overflow: The expression ""size
- rof->filled"" is deemed overflowed because at least one of its
arguments has overflowed.
diffutils-3.10/lib/stackvma.c:198:23: overflow_sink: ""size -
rof->filled"", which might have underflowed, is passed to ""read(fd,
rof->buffer + rof->filled, size - rof->filled)"". [Note: The source
code implementation of the function has been overridden by a builtin
model.]
# 196| for (;;)
# 197| {
# 198|-> n = read (fd, rof->buffer +
rof->filled, size - rof->filled);
# 199| if (n < 0 && errno == EINTR)
# 200| goto retry;"
As near as I can make out, this was the only defect report by Coverity
that was not a false alarm. I installed the attached patch into Gnulib
to fix the bug, which appears to be so unlikely that it's not worth
losing sleep over.
Marking the diffutils bug as done since the other defect reports were
false alarms.From 459da066b3b4fce2dc6cdfa09840508d5de729d8 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Thu, 20 Jun 2024 00:19:42 -0400
Subject: [PATCH] sigsegv: avoid unlikely undefined behavior
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Problem found by Coverity for diffutils and reported by Wasser Mai in:
https://bugs.gnu.org/71535
* lib/stackvma.c (rof_open) [__linux__ || __FreeBSD__ || etc]:
Don’t assume result of ‘read’ fits in int.
Avoid undefined behavior if ‘n + MIN_LEFTOVER’ would overflow.
Also, move a test to be after an (n == 0) test, to help the compiler.
---
ChangeLog | 10 ++++++++++
lib/stackvma.c | 10 +++++-----
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 1c156b791e..edbf067136 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2024-06-20 Paul Eggert <eggert@re>
+
+ sigsegv: avoid unlikely undefined behavior
+ Problem found by Coverity for diffutils and reported by Wasser Mai in:
+ https://bugs.gnu.org/71535
+ * lib/stackvma.c (rof_open) [__linux__ || __FreeBSD__ || etc]:
+ Don’t assume result of ‘read’ fits in int.
+ Avoid undefined behavior if ‘n + MIN_LEFTOVER’ would overflow.
+ Also, move a test to be after an (n == 0) test, to help the compiler.
+
2024-06-19 Bruno Haible <br...@clisp.org>
vasnprintf, u*-asnprintf tests: Add test of huge %ls arguments.
diff --git a/lib/stackvma.c b/lib/stackvma.c
index e93f939556..4424c6f58c 100644
--- a/lib/stackvma.c
+++ b/lib/stackvma.c
@@ -176,7 +176,7 @@ rof_open (struct rofile *rof, const char *filename)
/* Attempt to read the contents in a single system call. */
if (size > MIN_LEFTOVER)
{
- int n = read (fd, rof->buffer, size);
+ ssize_t n = read (fd, rof->buffer, size);
if (n < 0 && errno == EINTR)
goto retry;
# if defined __DragonFly__
@@ -186,7 +186,7 @@ rof_open (struct rofile *rof, const char *filename)
if (n <= 0)
/* Empty file. */
goto fail1;
- if (n + MIN_LEFTOVER <= size)
+ if (MIN_LEFTOVER <= size - n)
{
/* The buffer was sufficiently large. */
rof->filled = n;
@@ -201,15 +201,15 @@ rof_open (struct rofile *rof, const char *filename)
if (n < 0)
/* Some error. */
goto fail1;
- if (n + MIN_LEFTOVER > size - rof->filled)
- /* Allocate a larger buffer. */
- break;
if (n == 0)
{
/* Reached the end of file. */
close (fd);
return 0;
}
+ if (size - rof->filled - n < MIN_LEFTOVER)
+ /* Allocate a larger buffer. */
+ break;
rof->filled += n;
}
# else
--
2.34.1
