Harden putpwent, putgrent, putspent, putspent against injection [BZ #18724]

This prevents injection of ':' and '\n' into output functions which
use the NSS files database syntax.  Critical fields (user/group names
and file system paths) are checked strictly.  For backwards
compatibility, the GECOS field is rewritten instead.

The getent program is adjusted to use the put*ent functions in libc,
instead of local copies.  This changes the behavior of getent if user
names start with '-' or '+'.

22 files changed, 1018 insertions, 109 deletions

diff --git a/ChangeLog b/ChangeLog
index d410e0feef..20953eebc9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,41 @@
+2015-10-02  Florian Weimer  <fweimer@redhat.com>
+
+	[BZ #18724]
+	* include/nss.h (NSS_INVALID_FIELD_CHARACTERS): Define.
+	(__nss_invalid_field_characters, __nss_valid_field)
+	(__nss_valid_list_field, __nss_rewrite_field): Declare.
+	* nss/valid_field.c, nss/valid_list_field, nss/rewrite_field.c,
+	tst-field.c: New file.
+	* nss/Makefile (routines): Add valid_field, rewrite_field.
+	(tests-static): Define unconditionally.
+	(tests): Include tests-static.
+	[build-static-nss] (tests-static): Use append.
+	[build-static-nss] (tests): Remove modification.
+	* nss/getent.c (print_group): Call putgrent.  Report error.
+	(print_gshadow): Call putsgent.  Report error.
+	(print_passwd): Call putpwent.  Report error.
+	(print_shadow): Call putspent.  Report error.
+	* include/pwd.h: Include <nss.h> instead of <nss/nss.h>.
+	* pwd/pwd.h (putpwent): Remove incorrect nonnull attribute.
+	* pwd/putpwent.c (putpwent): Use ISO function definition.  Check
+	name, password, directory, shell fields for valid syntax.  Rewrite
+	GECOS field to match syntax.
+	* pwd/Makefile (tests): Add tst-putpwent.
+	* pwd/tst-putpwent.c: New file.
+	* grp/putgrent.c (putgrent): Convert to ISO function definition.
+	Check grName, grpasswd, gr_mem fields for valid syntax.
+	Change loop variable i to size_t.
+	* grp/Makefile (tests): Add tst-putgrent.
+	* grp/tst-putgrent.c: New file.
+	* shadow/putspent.c (putspent): Check sp_namp, sp_pwdp fields for
+	valid syntax.
+	* shadow/Makefile (tests): Add tst-putspent.
+	* shadow/tst-putspent.c: New file.
+	* gshadow/putsgent.c (putsgent): Check sg_namp, sg_passwd, sg_adm,
+	sg_mem fields for valid syntax.
+	* gshadow/Makefile (tests): Add tst-putsgent.
+	* gshadow/tst-putsgent.c: New file.
+
 2015-10-01  Gabriel F. T. Gomes  <gftg@linux.vnet.ibm.com>
 
 	* sysdeps/powerpc/powerpc64/power8/strncpy.S: Added comments to some
diff --git a/NEWS b/NEWS
index e8b59a4676..4634b74ebf 100644
--- a/NEWS
+++ b/NEWS
@@ -13,11 +13,11 @@ Version 2.23
   15918, 16141, 16296, 16347, 16415, 16517, 16519, 16520, 16521, 16620,
   16734, 16973, 16985, 17118, 17243, 17244, 17250, 17441, 17787, 17886,
   17887, 17905, 18084, 18086, 18240, 18265, 18370, 18421, 18480, 18525,
-  18595, 18610, 18618, 18647, 18661, 18674, 18675, 18681, 18757, 18778,
-  18781, 18787, 18789, 18790, 18795, 18796, 18803, 18820, 18823, 18824,
-  18825, 18857, 18863, 18870, 18872, 18873, 18875, 18887, 18921, 18951,
-  18952, 18956, 18961, 18966, 18967, 18969, 18970, 18977, 18980, 18981,
-  18985, 19003, 19016, 19032, 19046.
+  18595, 18610, 18618, 18647, 18661, 18674, 18675, 18681, 18724, 18757,
+  18778, 18781, 18787, 18789, 18790, 18795, 18796, 18803, 18820, 18823,
+  18824, 18825, 18857, 18863, 18870, 18872, 18873, 18875, 18887, 18921,
+  18951, 18952, 18956, 18961, 18966, 18967, 18969, 18970, 18977, 18980,
+  18981, 18985, 19003, 19016, 19032, 19046.
 
 * The obsolete header <regexp.h> has been removed.  Programs that require
   this header must be updated to use <regex.h> instead.
diff --git a/grp/Makefile b/grp/Makefile
index c63b552a65..ed8cc2b056 100644
--- a/grp/Makefile
+++ b/grp/Makefile
@@ -28,7 +28,7 @@ routines := fgetgrent initgroups setgroups \
 	    getgrent getgrgid getgrnam putgrent \
 	    getgrent_r getgrgid_r getgrnam_r fgetgrent_r
 
-tests := testgrp
+tests := testgrp tst-putgrent
 
 ifeq (yes,$(build-shared))
 test-srcs :=  tst_fgetgrent
diff --git a/grp/putgrent.c b/grp/putgrent.c
index e4d662c6b5..1ac7f19a0b 100644
--- a/grp/putgrent.c
+++ b/grp/putgrent.c
@@ -16,7 +16,9 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <errno.h>
+#include <nss.h>
 #include <stdio.h>
+#include <string.h>
 #include <grp.h>
 
 #define flockfile(s) _IO_flockfile (s)
@@ -27,13 +29,14 @@
 /* Write an entry to the given stream.
    This must know the format of the group file.  */
 int
-putgrent (gr, stream)
-     const struct group *gr;
-     FILE *stream;
+putgrent (const struct group *gr, FILE *stream)
 {
   int retval;
 
-  if (__glibc_unlikely (gr == NULL) || __glibc_unlikely (stream == NULL))
+  if (__glibc_unlikely (gr == NULL) || __glibc_unlikely (stream == NULL)
+      || gr->gr_name == NULL || !__nss_valid_field (gr->gr_name)
+      || !__nss_valid_field (gr->gr_passwd)
+      || !__nss_valid_list_field (gr->gr_mem))
     {
       __set_errno (EINVAL);
       return -1;
@@ -56,9 +59,7 @@ putgrent (gr, stream)
 
   if (gr->gr_mem != NULL)
     {
-      int i;
-
-      for (i = 0 ; gr->gr_mem[i] != NULL; i++)
+      for (size_t i = 0; gr->gr_mem[i] != NULL; i++)
 	if (fprintf (stream, i == 0 ? "%s" : ",%s", gr->gr_mem[i]) < 0)
 	  {
 	    /* What else can we do?  */
diff --git a/grp/tst-putgrent.c b/grp/tst-putgrent.c
new file mode 100644
index 0000000000..0a07ef6ac1
--- /dev/null
+++ b/grp/tst-putgrent.c
@@ -0,0 +1,167 @@
+/* Test for processing of invalid group entries.  [BZ #18724]
+   Copyright (C) 2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <grp.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+static bool errors;
+
+static void
+check (struct group e, const char *expected)
+{
+  char *buf;
+  size_t buf_size;
+  FILE *f = open_memstream (&buf, &buf_size);
+
+  if (f == NULL)
+    {
+      printf ("open_memstream: %m\n");
+      errors = true;
+      return;
+    }
+
+  int ret = putgrent (&e, f);
+
+  if (expected == NULL)
+    {
+      if (ret == -1)
+	{
+	  if (errno != EINVAL)
+	    {
+	      printf ("putgrent: unexpected error code: %m\n");
+	      errors = true;
+	    }
+	}
+      else
+	{
+	  printf ("putgrent: unexpected success (\"%s\", \"%s\")\n",
+		  e.gr_name, e.gr_passwd);
+	  errors = true;
+	}
+    }
+  else
+    {
+      /* Expect success.  */
+      size_t expected_length = strlen (expected);
+      if (ret == 0)
+	{
+	  long written = ftell (f);
+
+	  if (written <= 0 || fflush (f) < 0)
+	    {
+	      printf ("stream error: %m\n");
+	      errors = true;
+	    }
+	  else if (buf[written - 1] != '\n')
+	    {
+	      printf ("FAILED: \"%s\" without newline\n", expected);
+	      errors = true;
+	    }
+	  else if (strncmp (buf, expected, written - 1) != 0
+		   || written - 1 != expected_length)
+	    {
+	      buf[written - 1] = '\0';
+	      printf ("FAILED: \"%s\" (%ld), expected \"%s\" (%zu)\n",
+		      buf, written - 1, expected, expected_length);
+	      errors = true;
+	    }
+	}
+      else
+	{
+	  printf ("FAILED: putgrent (expected \"%s\"): %m\n", expected);
+	  errors = true;
+	}
+    }
+
+  fclose (f);
+  free (buf);
+}
+
+static int
+do_test (void)
+{
+  check ((struct group) {
+      .gr_name = (char *) "root",
+    },
+    "root::0:");
+  check ((struct group) {
+      .gr_name = (char *) "root",
+      .gr_passwd = (char *) "password",
+      .gr_gid = 1234,
+      .gr_mem = (char *[2]) {(char *) "member1", NULL}
+    },
+    "root:password:1234:member1");
+  check ((struct group) {
+      .gr_name = (char *) "root",
+      .gr_passwd = (char *) "password",
+      .gr_gid = 1234,
+      .gr_mem = (char *[3]) {(char *) "member1", (char *) "member2", NULL}
+    },
+    "root:password:1234:member1,member2");
+
+  /* Bad values.  */
+  {
+    static const char *const bad_strings[] = {
+      ":",
+      "\n",
+      ":bad",
+      "\nbad",
+      "b:ad",
+      "b\nad",
+      "bad:",
+      "bad\n",
+      "b:a\nd"
+      ",",
+      "\n,",
+      ":,",
+      ",bad",
+      "b,ad",
+      "bad,",
+      NULL
+    };
+    for (const char *const *bad = bad_strings; *bad != NULL; ++bad)
+      {
+	char *members[]
+	  = {(char *) "first", (char *) *bad, (char *) "last", NULL};
+	if (strpbrk (*bad, ":\n") != NULL)
+	  {
+	    check ((struct group) {
+		.gr_name = (char *) *bad,
+	      }, NULL);
+	    check ((struct group) {
+		.gr_name = (char *) "root",
+		.gr_passwd = (char *) *bad,
+	      }, NULL);
+	  }
+	check ((struct group) {
+	    .gr_name = (char *) "root",
+	    .gr_passwd = (char *) "password",
+	    .gr_mem = members,
+	  }, NULL);
+      }
+  }
+
+  return errors;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/gshadow/Makefile b/gshadow/Makefile
index 883e1c8a2a..c811041a55 100644
--- a/gshadow/Makefile
+++ b/gshadow/Makefile
@@ -26,7 +26,7 @@ headers		= gshadow.h
 routines	= getsgent getsgnam sgetsgent fgetsgent putsgent \
 		  getsgent_r getsgnam_r sgetsgent_r fgetsgent_r
 
-tests = tst-gshadow
+tests = tst-gshadow tst-putsgent
 
 CFLAGS-getsgent_r.c = -fexceptions
 CFLAGS-getsgent.c = -fexceptions
diff --git a/gshadow/putsgent.c b/gshadow/putsgent.c
index 0b2cad6eaa..c1cb921595 100644
--- a/gshadow/putsgent.c
+++ b/gshadow/putsgent.c
@@ -15,9 +15,11 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <errno.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <gshadow.h>
+#include <nss.h>
 
 #define _S(x)	x ? x : ""
 
@@ -29,6 +31,15 @@ putsgent (const struct sgrp *g, FILE *stream)
 {
   int errors = 0;
 
+  if (g->sg_namp == NULL || !__nss_valid_field (g->sg_namp)
+      || !__nss_valid_field (g->sg_passwd)
+      || !__nss_valid_list_field (g->sg_adm)
+      || !__nss_valid_list_field (g->sg_mem))
+    {
+      __set_errno (EINVAL);
+      return -1;
+    }
+
   _IO_flockfile (stream);
 
   if (fprintf (stream, "%s:%s:", g->sg_namp, _S (g->sg_passwd)) < 0)
diff --git a/gshadow/tst-putsgent.c b/gshadow/tst-putsgent.c
new file mode 100644
index 0000000000..5b0f381bc5
--- /dev/null
+++ b/gshadow/tst-putsgent.c
@@ -0,0 +1,168 @@
+/* Test for processing of invalid gshadow entries.  [BZ #18724]
+   Copyright (C) 2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <gshadow.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+static bool errors;
+
+static void
+check (struct sgrp e, const char *expected)
+{
+  char *buf;
+  size_t buf_size;
+  FILE *f = open_memstream (&buf, &buf_size);
+
+  if (f == NULL)
+    {
+      printf ("open_memstream: %m\n");
+      errors = true;
+      return;
+    }
+
+  int ret = putsgent (&e, f);
+
+  if (expected == NULL)
+    {
+      if (ret == -1)
+	{
+	  if (errno != EINVAL)
+	    {
+	      printf ("putsgent: unexpected error code: %m\n");
+	      errors = true;
+	    }
+	}
+      else
+	{
+	  printf ("putsgent: unexpected success (\"%s\")\n", e.sg_namp);
+	  errors = true;
+	}
+    }
+  else
+    {
+      /* Expect success.  */
+      size_t expected_length = strlen (expected);
+      if (ret == 0)
+	{
+	  long written = ftell (f);
+
+	  if (written <= 0 || fflush (f) < 0)
+	    {
+	      printf ("stream error: %m\n");
+	      errors = true;
+	    }
+	  else if (buf[written - 1] != '\n')
+	    {
+	      printf ("FAILED: \"%s\" without newline\n", expected);
+	      errors = true;
+	    }
+	  else if (strncmp (buf, expected, written - 1) != 0
+		   || written - 1 != expected_length)
+	    {
+	      printf ("FAILED: \"%s\" (%ld), expected \"%s\" (%zu)\n",
+		      buf, written - 1, expected, expected_length);
+	      errors = true;
+	    }
+	}
+      else
+	{
+	  printf ("FAILED: putsgent (expected \"%s\"): %m\n", expected);
+	  errors = true;
+	}
+    }
+
+  fclose (f);
+  free (buf);
+}
+
+static int
+do_test (void)
+{
+  check ((struct sgrp) {
+      .sg_namp = (char *) "root",
+    },
+    "root:::");
+  check ((struct sgrp) {
+      .sg_namp = (char *) "root",
+      .sg_passwd = (char *) "password",
+    },
+    "root:password::");
+  check ((struct sgrp) {
+      .sg_namp = (char *) "root",
+      .sg_passwd = (char *) "password",
+      .sg_adm = (char *[]) {(char *) "adm1", (char *) "adm2", NULL},
+      .sg_mem = (char *[]) {(char *) "mem1", (char *) "mem2", NULL},
+    },
+    "root:password:adm1,adm2:mem1,mem2");
+
+  /* Bad values.  */
+  {
+    static const char *const bad_strings[] = {
+      ":",
+      "\n",
+      ":bad",
+      "\nbad",
+      "b:ad",
+      "b\nad",
+      "bad:",
+      "bad\n",
+      "b:a\nd",
+      ",",
+      "\n,",
+      ":,",
+      ",bad",
+      "b,ad",
+      "bad,",
+      NULL
+    };
+    for (const char *const *bad = bad_strings; *bad != NULL; ++bad)
+      {
+	char *members[]
+	  = {(char *) "first", (char *) *bad, (char *) "last", NULL};
+	if (strpbrk (*bad, ":\n") != NULL)
+	  {
+	    check ((struct sgrp) {
+		.sg_namp = (char *) *bad,
+	      }, NULL);
+	    check ((struct sgrp) {
+		.sg_namp = (char *) "root",
+		.sg_passwd = (char *) *bad,
+	      }, NULL);
+	  }
+	check ((struct sgrp) {
+	    .sg_namp = (char *) "root",
+	    .sg_passwd = (char *) "password",
+	    .sg_adm = members
+	  }, NULL);
+	check ((struct sgrp) {
+	    .sg_namp = (char *) "root",
+	    .sg_passwd = (char *) "password",
+	    .sg_mem = members
+	  }, NULL);
+      }
+  }
+
+  return errors;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/include/nss.h b/include/nss.h
index 0541335c18..1e8cc3910d 100644
--- a/include/nss.h
+++ b/include/nss.h
@@ -1 +1,14 @@
+#ifndef _NSS_H
 #include <nss/nss.h>
+
+#define NSS_INVALID_FIELD_CHARACTERS ":\n"
+extern const char __nss_invalid_field_characters[] attribute_hidden;
+
+_Bool __nss_valid_field (const char *value)
+  attribute_hidden internal_function;
+_Bool __nss_valid_list_field (char **list)
+  attribute_hidden internal_function;
+const char *__nss_rewrite_field (const char *value, char **to_be_freed)
+  attribute_hidden internal_function;
+
+#endif /* _NSS_H */
diff --git a/include/pwd.h b/include/pwd.h
index bd7fecc16e..3b0f72540c 100644
--- a/include/pwd.h
+++ b/include/pwd.h
@@ -24,7 +24,7 @@ extern int __fgetpwent_r (FILE * __stream, struct passwd *__resultbuf,
 			  char *__buffer, size_t __buflen,
 			  struct passwd **__result);
 
-#include <nss/nss.h>
+#include <nss.h>
 
 struct parser_data;
 extern int _nss_files_parse_pwent (char *line, struct passwd *result,
diff --git a/nss/Makefile b/nss/Makefile
index 02a50160cb..bbbad85d7e 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -26,6 +26,7 @@ headers			:= nss.h
 
 # This is the trivial part which goes into libc itself.
 routines		= nsswitch getns