head: fix overflows in elide_tail_bytes_pipe

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.

diff --git a/src/head.c b/src/head.c
index a9155c24c787dedfd6d0b2dba30ddae956834bfe..9715b7b73b0e0cbcaca19eeda59772e316afdf4c 100644 (file)
--- 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++)