Re: Int overflow in diff(1), rcs(1), cvs(1)

[Martin Natano]

On Wed, Feb 10, 2016 at 09:29:26AM -0700, Todd C. Miller wrote:
> On Wed, 10 Feb 2016 08:36:42 -0700, "Todd C. Miller" wrote:
> 
> > Making those values unsigned int should be sufficient but it is a
> > fairly intrusive change.
> 
> Actually, this is trickier than it appears since diff uses the sign
> bit in a clever way to save space.

What do you mean by it being used in a clever way? I see the sign bit
being used as part of the hash itself, like all the other bits in the
sum. That shouldn't change when using unsigned instead. Am I missing
something?

For what it's worth, converting the hash to unsigned at least doesn't
seem to break anything obvious for me. See the diff below (generated
with the patched binary).

--- diffreg.c.orig      Mon Feb  8 02:48:35 2016
+++ diffreg.c   Mon Feb  8 03:48:44 2016
@@ -160,8 +160,8 @@
 };
 
 struct line {
-       int     serial;
-       int     value;
+       int             serial;
+       unsigned int    value;
 } *file[2];
 
 /*
@@ -200,7 +200,7 @@
 static int      skipline(FILE *);
 static int      isqrt(int);
 static int      stone(int *, int, int *, int *, int);
-static int      readhash(FILE *, int);
+static unsigned int readhash(FILE *, int);
 static int      files_differ(FILE *, FILE *, int);
 static char    *match_function(const long *, int, FILE *);
 static char    *preadline(int, size_t, off_t);
@@ -495,7 +495,8 @@
 prepare(int i, FILE *fd, off_t filesize, int flags)
 {
        struct line *p;
-       int j, h;
+       int j;
+       unsigned int h;
        size_t sz;
 
        rewind(fd);
@@ -1168,11 +1169,11 @@
 /*
  * Hash function taken from Robert Sedgewick, Algorithms in C, 3d ed., p 578.
  */
-static int
+static unsigned int
 readhash(FILE *f, int flags)
 {
        int i, t, space;
-       int sum;
+       unsigned int sum;
 
        sum = 1;
        space = 0;

natano