[PATCH] gshadow: Handle the parser's full buffer error code

[David Michael <fedora dot dm0 at gmail dot com>]

    * gshadow/fgetsgent_r.c (__fgetsgent_r): Return ERANGE when the
    parse_line function returns its out-of-space error.
---

Hi,

The fgetgsent function isn't handling errors from parse_line.  That
means it can run out of buffer space when adding pointers to group
members and exit early without setting all members of the static result
struct.  The static result's members will remain pointing at buffer
locations from the previous line, which have been overwritten with
incompatible data, causing segfaults after it is returned normally.

This was detected due to systemd segfaulting.  See:
https://github.com/coreos/bugs/issues/1394

If you don't want to mess with your /etc/gshadow to test it, the
following program will also segfault (tested on Fedora and CoreOS).

Thanks.

David


#include <gshadow.h>
#include <stdio.h>
int main() {
  struct sgrp *entry;
  char **member;
  FILE *input = fdopen (0, "rb");
  while (entry = fgetsgent (input))
    {
      printf ("%s", entry->sg_namp);
      for (member = entry->sg_mem; *member; member++)
        printf(", %s", *member);
      printf ("\n");
    }
  fclose (input);
  return 0;
}

Feed this through stdin.  It should fit in the allocated buffer on
x86_64 and succeed if you delete the last character from the second
line.

grp0:*::root
grp1:*::somebody.a1,somebody.a2,somebody.a3,somebody.a4,somebody.a5,somebody.a6,somebody.a7,somebody.a8,somebody.a9,somebody.a10,somebody.a11,somebody.a12,somebody.a13,somebody.a14,somebody.a15,somebody.a16,somebody.a17,somebody.a18,somebody.a19,somebody.a20,somebody.a21,somebody.a22,somebody.a23,somebody.a24,somebody.a25,somebody.a26,somebody.a27,somebody.a28,somebody.a29,somebody.a30,somebody.a31,somebody.a32,somebody.a33,somebody.a34,somebody.a35,somebody.a36,somebody.a37,somebody.a38,somebody.a39,somebody.a40,somebody.a41,somebody.a42,somebody.a43,somebody.a44,somebody.a45,somebody.a46,somebody.a47,a1234
grp2:*::root

 gshadow/fgetsgent_r.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/gshadow/fgetsgent_r.c b/gshadow/fgetsgent_r.c
index b70f6fa..8c13c55 100644
--- a/gshadow/fgetsgent_r.c
+++ b/gshadow/fgetsgent_r.c
@@ -37,6 +37,7 @@ __fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen,
 	       struct sgrp **result)
 {
   char *p;
+  int rc;
 
   _IO_flockfile (stream);
   do
@@ -64,11 +65,18 @@ __fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen,
     } while (*p == '\0' || *p == '#' ||	/* Ignore empty and comment lines.  */
 	     /* Parse the line.  If it is invalid, loop to
 		get the next line of the file to parse.  */
-	     ! parse_line (buffer, (void *) resbuf, (void *) buffer, buflen,
-			   &errno));
+	     !(rc = parse_line (buffer, (void *) resbuf,
+				(void *) buffer, buflen, &errno)));
 
   _IO_funlockfile (stream);
 
+  if (rc < 0)
+    {
+      *result = NULL;
+      __set_errno (ERANGE);
+      return errno;
+    }
+
   *result = resbuf;
   return 0;
 }
-- 
2.7.4