Subject: bug#18269: closed (Re: bug#18269: incorrect undossify_input prototype - possible integer overflow)

From: [email-masked] (GNU bug Tracking System)
Date: Fri, 15 Aug 2014 01:42:03 +0000

Your bug report

#18269: incorrect undossify_input prototype - possible integer overflow

which was filed against the grep package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
18269: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=18269
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#18269: incorrect undossify_input prototype - possible integer overflow
From: Paul Eggert <[email-masked]>
To: Vincent Lefevre <[email-masked]>, [email-masked]
Date: Thu, 14 Aug 2014 18:41:32 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Thanks for reporting that.  Plus, there's a related bug in the use of 
safe_read.  I fixed them both with the attached patch.
From 9fdcf2ccf1f9bde999f602a10d37d89dbe384fab Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Thu, 14 Aug 2014 18:38:49 -0700
Subject: [PATCH] grep: fix integer-width bugs in undossify_input etc.

undossify_input bug reported by Vincent Lefevre in:
http://bugs.gnu.org/18269
* src/dosbuf.c (undossify_input): Return size_t, not int.
* src/grep.c (fillbuf): Work portably even if safe_read returns a
value greater than SSIZE_MAX, e.g., if there's an I/O error.
---
 src/dosbuf.c | 10 +++++-----
 src/grep.c   |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/dosbuf.c b/src/dosbuf.c
index 9ac2d13..3b4052a 100644
--- a/src/dosbuf.c
+++ b/src/dosbuf.c
@@ -90,15 +90,15 @@ guess_type (char *buf, size_t buflen)
 }
 
 /* Convert external DOS file representation to internal.
-   Return the count of characters left in the buffer.
+   Return the count of bytes left in the buffer.
    Build table to map character positions when reporting byte counts.  */
-static int
+static size_t
 undossify_input (char *buf, size_t buflen)
 {
   if (! O_BINARY)
     return buflen;
 
-  int chars_left = 0;
+  size_t bytes_left = 0;
 
   if (totalcc == 0)
     {
@@ -126,7 +126,7 @@ undossify_input (char *buf, size_t buflen)
           if (*buf != '\r')
             {
               *destp++ = *buf++;
-              chars_left++;
+              bytes_left++;
             }
           else
             {
@@ -176,7 +176,7 @@ undossify_input (char *buf, size_t buflen)
             }
         }
 
-      return chars_left;
+      return bytes_left;
     }
 
   return buflen;
diff --git a/src/grep.c b/src/grep.c
index 5e6c5c6..6b930dc 100644
--- a/src/grep.c
+++ b/src/grep.c
@@ -397,7 +397,7 @@ static int grepdesc (int, int);
 
 static void dos_binary (void);
 static void dos_unix_byte_offsets (void);
-static int undossify_input (char *, size_t);
+static size_t undossify_input (char *, size_t);
 
 static int
 is_device_mode (mode_t m)
@@ -583,7 +583,7 @@ reset (int fd, struct stat const *st)
 static int
 fillbuf (size_t save, struct stat const *st)
 {
-  ssize_t fillsize;
+  size_t fillsize;
   int cc = 1;
   char *readbuf;
   size_t readsize;
@@ -645,7 +645,7 @@ fillbuf (size_t save, struct stat const *st)
   readsize -= readsize % pagesize;
 
   fillsize = safe_read (bufdesc, readbuf, readsize);
-  if (fillsize < 0)
+  if (fillsize == SAFE_READ_ERROR)
     fillsize = cc = 0;
   bufoffset += fillsize;
   fillsize = undossify_input (readbuf, fillsize);
-- 
1.9.3

Subject: incorrect undossify_input prototype - possible integer overflow
From: Vincent Lefevre <[email-masked]>
To: [email-masked]
Date: Fri, 15 Aug 2014 02:56:19 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

In grep 2.20, grep.c contains:

  ssize_t fillsize;
  size_t readsize;
[...]
  fillsize = safe_read (bufdesc, readbuf, readsize);
  if (fillsize < 0)
    fillsize = cc = 0;
  bufoffset += fillsize;
  fillsize = undossify_input (readbuf, fillsize);

In practice, readsize can be large on a 64-bit machine (more than
2 GB), so that the return value of safe_read(), fillsize, can also
be large since a read() is called with readsize as the 3rd argument.
But dosbuf.c has:

static int
undossify_input (char *buf, size_t buflen)
{
  if (! O_BINARY)
    return buflen;
[...]

meaning that the potentially large buflen (> 2 GB) is returned as an
int, whose usual size is 32 bits only, yielding an integer overflow.

undossify_input should be rewritten in such a way that some int's are
changed to size_t or ssize_t.

Note: This bug is currently not visible under Linux due to a limitation
in the kernel (breaking POSIX compliance, BTW): the read() return value
is limited to 0x7ffff000.

-- 
Vincent Lefèvre <[email-masked]> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)