Re: [Bug-gnulib] getline bugfix submitted via findutils

[David Newall]

On Mon, 2004-04-19 at 23:17, Bruno Haible wrote:
> In the findutils CVS, it's now around line 376 of locate.c. The bug is still
> there: No validity checks are made on the 'count' variable after c and
> optionally two more bytes have been read from the file.

There's merit to Bruno's idea that the database must have been corrupt,
although it must necessarily have been created corrupt.  4.1.20 creates
the database in a completely different way to 4.1.7, and unfortunately
I've made major changes to my file system since last updatedb and the
bug is no longer triggered.  Offset should always be in the range
[0..length of last path] and so the best I can suggest is an assertion
to ensure that it's never out of range.

It turns out that there's a completely new bug in the 4.1.20 locate, an
off by relating to the null terminator appended by getdelim2().

Here's a patch which corrects the off by one bug, and which adds the
requisite assert.  It's untested to the extent that I haven't reproduced
a database which exposes the bug in 4.1.7 so I don't know if 4.1.20 has
the same problem.  My guess is it doesn't because updatedb works in such
a diferent way.  The patch certainly doesn't cause any new problems.

Regards,

David

--- locate/locate.c.orig        2004-04-20 00:05:16.839339329 +0930
+++ locate/locate.c     2004-04-20 02:19:09.393747328 +0930
@@ -225,7 +225,7 @@
     {
       assert(p != NULL);
       
-      needed = offs + nread;
+      needed = offs + nread + 1;
       if (needed > (*siz))
        {
          char *pnew = realloc(*buf, needed);
@@ -270,6 +270,8 @@
   char *path;
   /* Amount allocated for it.  */
   size_t pathsize;
+  /* The length of the last path */
+  size_t last_pathlen = 0;
 
   /* The length of the prefix shared with the previous database entry.  */
   int count = 0;
@@ -372,6 +374,7 @@
            count += c - 256;
          else
            count += c;
+         assert(count >= 0 && count <= last_pathlen);
 
          /* Overlay the old path with the remainder of the new.  */
          nread = locate_read_str (&path, &pathsize, fp, 0, count); 
@@ -379,6 +382,7 @@
            break;
          c = getc (fp);
          s = path + count + nread - 2; /* Move to the last char in path.  */
+         last_pathlen = count + nread - 1;
          assert (s[0] != '\0');
          assert (s[1] == '\0'); /* Our terminator.  */
          assert (s[2] == '\0'); /* Added by locate_read_str.  */