253 lines
9.5 KiB
Diff
253 lines
9.5 KiB
Diff
From 8fe800a06e50be3c905ab1694a2d1bfd6e70be42 Mon Sep 17 00:00:00 2001
|
||
From: Paul Eggert <eggert@cs.ucla.edu>
|
||
Date: Sat, 10 Aug 2024 22:19:17 -0700
|
||
Subject: [PATCH] head: fix overflows in elide_tail_bytes_pipe
|
||
MIME-Version: 1.0
|
||
Content-Type: text/plain; charset=UTF-8
|
||
Content-Transfer-Encoding: 8bit
|
||
|
||
Not clear that the overflows could be exploited,
|
||
but they made the code confusing.
|
||
* src/head.c (elide_tail_bytes_pipe): Don’t convert uintmax_t
|
||
to size_t first thing; wait until it’s known the value will fit,
|
||
and then use idx_t rather than size_t to prefer signed types.
|
||
Prefer idx_t in nearby code, too.
|
||
Rename locals n_elide_0 to n_elide (for consistency elsewhere)
|
||
and n_elide to in_elide.
|
||
Remove bogus (SIZE_MAX < n_elide + READ_BUFSIZE) test;
|
||
in the typical case where n_elide’s type was the same as
|
||
that of SIZE_MAX, the test never succeeded, and in the
|
||
less-common case where n_elide was wider than size_t,
|
||
the addition could silently overflow, causing the test
|
||
to fail when it should succeed. The test is not needed anyway now.
|
||
Add static asserts to document code assumptions.
|
||
Redo the ! (n_elide <= HEAD_TAIL_PIPE_BYTECOUNT_THRESHOLD) case
|
||
so that it works with enormous values of n_elide even on
|
||
32-bit platforms; for example, n_bufs is now uintmax_t not size_t.
|
||
Simplify by using xpalloc instead of by-hand code.
|
||
Remove bogus ‘if (rem)’ test, as rem is always nonzero.
|
||
---
|
||
src/head.c | 129 ++++++++++++++++++++++++-----------------------------
|
||
1 file changed, 58 insertions(+), 71 deletions(-)
|
||
|
||
diff --git a/src/head.c b/src/head.c
|
||
index a9155c24c..9715b7b73 100644
|
||
--- a/src/head.c
|
||
+++ b/src/head.c
|
||
@@ -237,17 +237,16 @@ elseek (int fd, off_t offset, int whence, char const *filename)
|
||
}
|
||
|
||
/* For an input file with name FILENAME and descriptor FD,
|
||
- output all but the last N_ELIDE_0 bytes.
|
||
+ output all but the last N_ELIDE bytes.
|
||
If CURRENT_POS is nonnegative, assume that the input file is
|
||
positioned at CURRENT_POS and that it should be repositioned to
|
||
just before the elided bytes before returning.
|
||
Return true upon success.
|
||
Give a diagnostic and return false upon error. */
|
||
static bool
|
||
-elide_tail_bytes_pipe (char const *filename, int fd, uintmax_t n_elide_0,
|
||
+elide_tail_bytes_pipe (char const *filename, int fd, uintmax_t n_elide,
|
||
off_t current_pos)
|
||
{
|
||
- size_t n_elide = n_elide_0;
|
||
uintmax_t desired_pos = current_pos;
|
||
bool ok = true;
|
||
|
||
@@ -265,16 +264,9 @@ elide_tail_bytes_pipe (char const *filename, int fd, uintmax_t n_elide_0,
|
||
#endif
|
||
|
||
#if HEAD_TAIL_PIPE_BYTECOUNT_THRESHOLD < 2 * READ_BUFSIZE
|
||
- "HEAD_TAIL_PIPE_BYTECOUNT_THRESHOLD must be at least 2 * READ_BUFSIZE"
|
||
+# error "HEAD_TAIL_PIPE_BYTECOUNT_THRESHOLD must be at least 2 * READ_BUFSIZE"
|
||
#endif
|
||
|
||
- if (SIZE_MAX < n_elide_0 + READ_BUFSIZE)
|
||
- {
|
||
- char umax_buf[INT_BUFSIZE_BOUND (n_elide_0)];
|
||
- error (EXIT_FAILURE, 0, _("%s: number of bytes is too large"),
|
||
- umaxtostr (n_elide_0, umax_buf));
|
||
- }
|
||
-
|
||
/* Two cases to consider...
|
||
1) n_elide is small enough that we can afford to double-buffer:
|
||
allocate 2 * (READ_BUFSIZE + n_elide) bytes
|
||
@@ -286,11 +278,14 @@ elide_tail_bytes_pipe (char const *filename, int fd, uintmax_t n_elide_0,
|
||
CAUTION: do not fail (out of memory) when asked to elide
|
||
a ridiculous amount, but when given only a small input. */
|
||
|
||
+ static_assert (READ_BUFSIZE <= IDX_MAX);
|
||
+ static_assert (HEAD_TAIL_PIPE_BYTECOUNT_THRESHOLD <= IDX_MAX - READ_BUFSIZE);
|
||
if (n_elide <= HEAD_TAIL_PIPE_BYTECOUNT_THRESHOLD)
|
||
{
|
||
+ idx_t in_elide = n_elide;
|
||
bool first = true;
|
||
bool eof = false;
|
||
- size_t n_to_read = READ_BUFSIZE + n_elide;
|
||
+ size_t n_to_read = READ_BUFSIZE + in_elide;
|
||
bool i;
|
||
char *b[2];
|
||
b[0] = xnmalloc (2, n_to_read);
|
||
@@ -310,7 +305,7 @@ elide_tail_bytes_pipe (char const *filename, int fd, uintmax_t n_elide_0,
|
||
}
|
||
|
||
/* reached EOF */
|
||
- if (n_read <= n_elide)
|
||
+ if (n_read <= in_elide)
|
||
{
|
||
if (first)
|
||
{
|
||
@@ -320,7 +315,7 @@ elide_tail_bytes_pipe (char const *filename, int fd, uintmax_t n_elide_0,
|
||
}
|
||
else
|
||
{
|
||
- delta = n_elide - n_read;
|
||
+ delta = in_elide - n_read;
|
||
}
|
||
}
|
||
eof = true;
|
||
@@ -330,15 +325,15 @@ elide_tail_bytes_pipe (char const *filename, int fd, uintmax_t n_elide_0,
|
||
the previous round. */
|
||
if (! first)
|
||
{
|
||
- desired_pos += n_elide - delta;
|
||
- xwrite_stdout (b[!i] + READ_BUFSIZE, n_elide - delta);
|
||
+ desired_pos += in_elide - delta;
|
||
+ xwrite_stdout (b[!i] + READ_BUFSIZE, in_elide - delta);
|
||
}
|
||
first = false;
|
||
|
||
- if (n_elide < n_read)
|
||
+ if (in_elide < n_read)
|
||
{
|
||
- desired_pos += n_read - n_elide;
|
||
- xwrite_stdout (b[i], n_read - n_elide);
|
||
+ desired_pos += n_read - in_elide;
|
||
+ xwrite_stdout (b[i], n_read - in_elide);
|
||
}
|
||
}
|
||
|
||
@@ -350,31 +345,24 @@ elide_tail_bytes_pipe (char const *filename, int fd, uintmax_t n_elide_0,
|
||
bytes. Then, for each new buffer we read, also write an old one. */
|
||
|
||
bool eof = false;
|
||
- size_t n_read;
|
||
- bool buffered_enough;
|
||
- size_t i, i_next;
|
||
+ idx_t n_read;
|
||
char **b = nullptr;
|
||
- /* Round n_elide up to a multiple of READ_BUFSIZE. */
|
||
- size_t rem = READ_BUFSIZE - (n_elide % READ_BUFSIZE);
|
||
- size_t n_elide_round = n_elide + rem;
|
||
- size_t n_bufs = n_elide_round / READ_BUFSIZE + 1;
|
||
- size_t n_alloc = 0;
|
||
- size_t n_array_alloc = 0;
|
||
-
|
||
- buffered_enough = false;
|
||
+
|
||
+ idx_t remainder = n_elide % READ_BUFSIZE;
|
||
+ /* The number of buffers needed to hold n_elide bytes plus one
|
||
+ extra buffer. They are allocated lazily, so don't report
|
||
+ overflow now simply because the number does not fit into idx_t. */
|
||
+ uintmax_t n_bufs = n_elide / READ_BUFSIZE + (remainder != 0) + 1;
|
||
+ idx_t n_alloc = 0;
|
||
+ idx_t n_array_alloc = 0;
|
||
+
|
||
+ bool buffered_enough = false;
|
||
+ idx_t i, i_next;
|
||
for (i = 0, i_next = 1; !eof; i = i_next, i_next = (i_next + 1) % n_bufs)
|
||
{
|
||
if (n_array_alloc == i)
|
||
- {
|
||
- /* reallocate between 16 and n_bufs entries. */
|
||
- if (n_array_alloc == 0)
|
||
- n_array_alloc = MIN (n_bufs, 16);
|
||
- else if (n_array_alloc <= n_bufs / 2)
|
||
- n_array_alloc *= 2;
|
||
- else
|
||
- n_array_alloc = n_bufs;
|
||
- b = xnrealloc (b, n_array_alloc, sizeof *b);
|
||
- }
|
||
+ b = xpalloc (b, &n_array_alloc, 1, MIN (n_bufs, PTRDIFF_MAX),
|
||
+ sizeof *b);
|
||
|
||
if (! buffered_enough)
|
||
{
|
||
@@ -403,43 +391,42 @@ elide_tail_bytes_pipe (char const *filename, int fd, uintmax_t n_elide_0,
|
||
}
|
||
}
|
||
|
||
- /* Output any remainder: rem bytes from b[i] + n_read. */
|
||
- if (rem)
|
||
+ /* Output the remainder: rem bytes from b[i] + n_read. */
|
||
+ idx_t rem = READ_BUFSIZE - remainder;
|
||
+ if (buffered_enough)
|
||
{
|
||
- if (buffered_enough)
|
||
+ idx_t n_bytes_left_in_b_i = READ_BUFSIZE - n_read;
|
||
+ desired_pos += rem;
|
||
+ if (rem < n_bytes_left_in_b_i)
|
||
{
|
||
- size_t n_bytes_left_in_b_i = READ_BUFSIZE - n_read;
|
||
- desired_pos += rem;
|
||
- if (rem < n_bytes_left_in_b_i)
|
||
- {
|
||
- xwrite_stdout (b[i] + n_read, rem);
|
||
- }
|
||
- else
|
||
- {
|
||
- xwrite_stdout (b[i] + n_read, n_bytes_left_in_b_i);
|
||
- xwrite_stdout (b[i_next], rem - n_bytes_left_in_b_i);
|
||
- }
|
||
+ xwrite_stdout (b[i] + n_read, rem);
|
||
}
|
||
- else if (i + 1 == n_bufs)
|
||
+ else
|
||
{
|
||
- /* This happens when n_elide < file_size < n_elide_round.
|
||
-
|
||
- |READ_BUF.|
|
||
- | | rem |
|
||
- |---------!---------!---------!---------|
|
||
- |---- n_elide ---------|
|
||
- | | x |
|
||
- | |y |
|
||
- |---- file size -----------|
|
||
- | |n_read|
|
||
- |---- n_elide_round ----------|
|
||
- */
|
||
- size_t y = READ_BUFSIZE - rem;
|
||
- size_t x = n_read - y;
|
||
- desired_pos += x;
|
||
- xwrite_stdout (b[i_next], x);
|
||
+ xwrite_stdout (b[i] + n_read, n_bytes_left_in_b_i);
|
||
+ xwrite_stdout (b[i_next], rem - n_bytes_left_in_b_i);
|
||
}
|
||
}
|
||
+ else if (i + 1 == n_bufs)
|
||
+ {
|
||
+ /* This happens when
|
||
+ n_elide < file_size < (n_bufs - 1) * READ_BUFSIZE.
|
||
+
|
||
+ |READ_BUF.|
|
||
+ | | rem |
|
||
+ |---------!---------!---------!---------|
|
||
+ |---- n_elide----------|
|
||
+ | | x |
|
||
+ | |y |
|
||
+ |---- file size -----------|
|
||
+ | |n_read|
|
||
+ |(n_bufs - 1) * READ_BUFSIZE--|
|
||
+ */
|
||
+ idx_t y = READ_BUFSIZE - rem;
|
||
+ idx_t x = n_read - y;
|
||
+ desired_pos += x;
|
||
+ xwrite_stdout (b[i_next], x);
|
||
+ }
|
||
|
||
free_mem:
|
||
for (i = 0; i < n_alloc; i++)
|
||
--
|
||
2.33.0
|
||
|