diff mbox

Harden put*ent functions against data injection [BZ #18724]

Message ID 55B64BE2.9060905@redhat.com
State New
Headers show

Commit Message

Florian Weimer July 27, 2015, 3:18 p.m. UTC
This patch addresses a “Bobby Tables” issue in the put*ent functions and
the getent program, similar to one of the recent libuser issues.

I believe this is just hardening because users of the put*ent functions
already have appropriate checks before they call these functions, so
this is definitely post-freeze material.

Tested on x86_64-redhat-linux-gnu.  Okay to commit after master reopens?

Comments

Carlos O'Donell July 27, 2015, 7:59 p.m. UTC | #1
On 07/27/2015 11:18 AM, Florian Weimer wrote:
> This patch addresses a “Bobby Tables” issue in the put*ent functions and
> the getent program, similar to one of the recent libuser issues.
> 
> I believe this is just hardening because users of the put*ent functions
> already have appropriate checks before they call these functions, so
> this is definitely post-freeze material.
> 
> Tested on x86_64-redhat-linux-gnu.  Okay to commit after master reopens?

Looks good to me for 2.23 with testsuite comment nits fixed.

> -- Florian Weimer / Red Hat Product Security
> 
> 
> 0001-Harden-putpwent-putgrent-putspent-putspent-against-i.patch
> 
> 
> From ca909fd7c61e7821df24109182f8ec3201690e67 Mon Sep 17 00:00:00 2001
> Message-Id: <ca909fd7c61e7821df24109182f8ec3201690e67.1438010163.git.fweimer@redhat.com>
> From: Florian Weimer <fweimer@redhat.com>
> Date: Mon, 27 Jul 2015 17:13:05 +0200
> Subject: [PATCH] Harden putpwent, putgrent, putspent, putspent against
>  injection [BZ #18724]
> To: libc-alpha@sourceware.org
> 
> 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 '+'.
> ---
>  ChangeLog              |  38 +++++++++++
>  NEWS                   |   3 +-
>  grp/Makefile           |   2 +-
>  grp/putgrent.c         |  15 ++---
>  grp/tst-putgrent.c     | 165 ++++++++++++++++++++++++++++++++++++++++++++++++
>  gshadow/Makefile       |   2 +-
>  gshadow/putsgent.c     |  11 ++++
>  gshadow/tst-putsgent.c | 166 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/nss.h          |  13 ++++
>  include/pwd.h          |   2 +-
>  nss/Makefile           |   8 ++-
>  nss/getent.c           |  76 +++-------------------
>  nss/rewrite_field.c    |  51 +++++++++++++++
>  nss/tst-field.c        | 100 +++++++++++++++++++++++++++++
>  nss/valid_field.c      |  31 +++++++++
>  nss/valid_list_field.c |  35 +++++++++++
>  pwd/Makefile           |   2 +-
>  pwd/putpwent.c         |  52 +++++++++-------
>  pwd/pwd.h              |   2 +-
>  pwd/tst-putpwent.c     | 166 +++++++++++++++++++++++++++++++++++++++++++++++++
>  shadow/Makefile        |   2 +-
>  shadow/putspent.c      |   9 +++
>  shadow/tst-putspent.c  | 162 +++++++++++++++++++++++++++++++++++++++++++++++
>  23 files changed, 1007 insertions(+), 106 deletions(-)
>  create mode 100644 grp/tst-putgrent.c
>  create mode 100644 gshadow/tst-putsgent.c
>  create mode 100644 nss/rewrite_field.c
>  create mode 100644 nss/tst-field.c
>  create mode 100644 nss/valid_field.c
>  create mode 100644 nss/valid_list_field.c
>  create mode 100644 pwd/tst-putpwent.c
>  create mode 100644 shadow/tst-putspent.c
> 
> diff --git a/ChangeLog b/ChangeLog
> index 3e202c4..e9d0335 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,41 @@
> +2015-07-27  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-07-26  Chung-Lin Tang  <cltang@codesourcery.com>
>  
>  	* sysdeps/nios2/dl-sysdep.h (DL_EXTERN_PROTECTED_DATA): Define.
> diff --git a/NEWS b/NEWS
> index 65b2172..649c04b 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -27,7 +27,8 @@ Version 2.22
>    18520, 18522, 18527, 18528, 18529, 18530, 18532, 18533, 18534, 18536,
>    18539, 18540, 18542, 18544, 18545, 18546, 18547, 18549, 18553, 18557,
>    18558, 18569, 18583, 18585, 18586, 18592, 18593, 18594, 18602, 18612,
> -  18613, 18619, 18633, 18641, 18643, 18648, 18657, 18676, 18694, 18696.
> +  18613, 18619, 18633, 18641, 18643, 18648, 18657, 18676, 18694, 18696,
> +  18724.
>  
>  * Cache information can be queried via sysconf() function on s390 e.g. with
>    _SC_LEVEL1_ICACHE_SIZE as argument.
> diff --git a/grp/Makefile b/grp/Makefile
> index c63b552..ed8cc2b 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 e4d662c..3c06d1a 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))

OK.

>      {
>        __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++)

OK.

>  	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 0000000..3f259ca
> --- /dev/null
> +++ b/grp/tst-putgrent.c
> @@ -0,0 +1,165 @@

Needs first line description of test with reference BZ #.

> +/* 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 <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +static int 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;
> +      return;
> +    }
> +
> +  int ret = putgrent (&e, f);
> +
> +  if (expected == NULL)
> +    {
> +      if (ret == -1)
> +	{
> +	  if (errno != EINVAL)
> +	    {
> +	      printf ("putgrent: unexpected error code: %m\n");
> +	      ++errors;
> +	    }
> +	}
> +      else
> +	{
> +	  printf ("putgrent: unexpected success (\"%s\", \"%s\")\n",
> +		  e.gr_name, e.gr_passwd);
> +	  ++errors;
> +	}
> +    }
> +  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;
> +	    }
> +	  else if (buf[written - 1] != '\n')
> +	    {
> +	      printf ("FAILED: \"%s\" without newline\n", expected);
> +	      ++errors;
> +	    }
> +	  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;
> +	    }
> +	}
> +      else
> +	{
> +	  printf ("FAILED: putgrent (expected \"%s\"): %m\n", expected);
> +	  ++errors;
> +	}
> +    }
> +
> +  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 > 0;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> diff --git a/gshadow/Makefile b/gshadow/Makefile
> index 883e1c8..c811041 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 0b2cad6..c1cb921 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;
> +    }

OK.

> +
>    _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 0000000..f628fe6
> --- /dev/null
> +++ b/gshadow/tst-putsgent.c
> @@ -0,0 +1,166 @@

Needs first line description of test with reference BZ #.

> +/* 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 <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +static int 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;
> +      return;
> +    }
> +
> +  int ret = putsgent (&e, f);
> +
> +  if (expected == NULL)
> +    {
> +      if (ret == -1)
> +	{
> +	  if (errno != EINVAL)
> +	    {
> +	      printf ("putsgent: unexpected error code: %m\n");
> +	      ++errors;
> +	    }
> +	}
> +      else
> +	{
> +	  printf ("putsgent: unexpected success (\"%s\")\n", e.sg_namp);
> +	  ++errors;
> +	}
> +    }
> +  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;
> +	    }
> +	  else if (buf[written - 1] != '\n')
> +	    {
> +	      printf ("FAILED: \"%s\" without newline\n", expected);
> +	      ++errors;
> +	    }
> +	  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;
> +	    }
> +	}
> +      else
> +	{
> +	  printf ("FAILED: putsgent (expected \"%s\"): %m\n", expected);
> +	  ++errors;
> +	}
> +    }
> +
> +  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 > 0;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> diff --git a/include/nss.h b/include/nss.h
> index 0541335..1e8cc39 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;

OK.


> +
> +#endif /* _NSS_H */
> diff --git a/include/pwd.h b/include/pwd.h
> index bd7fecc..3b0f725 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>

OK.

>  
>  struct parser_data;
>  extern int _nss_files_parse_pwent (char *line, struct passwd *result,
> diff --git a/nss/Makefile b/nss/Makefile
> index 65ab7b5..9da8b15 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 getnssent getnssent_r digits_dots \
> +			  valid_field valid_list_field rewrite_field \

OK.

>  			  $(addsuffix -lookup,$(databases))
>  
>  # These are the databases that go through nss dispatch.
> @@ -47,7 +48,9 @@ install-bin             := getent makedb
>  makedb-modules = xmalloc hash-string
>  extra-objs		+= $(makedb-modules:=.o)
>  
> -tests			= test-netdb tst-nss-test1 test-digits-dots tst-nss-getpwent
> +tests-static            = tst-field
> +tests			= test-netdb tst-nss-test1 test-digits-dots tst-nss-getpwent \
> +			  $(tests-static)

OK.

>  xtests			= bug-erange
>  
>  # Specify rules for the nss_* modules.  We have some services.
> @@ -82,8 +85,7 @@ libnss_db-inhibit-o	= $(filter-out .os,$(object-suffixes))
>  ifeq ($(build-static-nss),yes)
>  routines                += $(libnss_files-routines)
>  static-only-routines    += $(libnss_files-routines)
> -tests-static		= tst-nss-static
> -tests			+= $(tests-static)
> +tests-static		+= tst-nss-static

OK.

>  endif
>  
>  include ../Rules
> diff --git a/nss/getent.c b/nss/getent.c
> index 34df848..996d378 100644
> --- a/nss/getent.c
> +++ b/nss/getent.c
> @@ -184,20 +184,8 @@ ethers_keys (int number, char *key[])
>  static void
>  print_group (struct group *grp)
>  {
> -  unsigned int i = 0;
> -
> -  printf ("%s:%s:%lu:", grp->gr_name ? grp->gr_name : "",
> -	  grp->gr_passwd ? grp->gr_passwd : "",
> -	  (unsigned long int) grp->gr_gid);
> -
> -  while (grp->gr_mem[i] != NULL)
> -    {
> -      fputs_unlocked (grp->gr_mem[i], stdout);
> -      ++i;
> -      if (grp->gr_mem[i] != NULL)
> -	putchar_unlocked (',');
> -    }
> -  putchar_unlocked ('\n');
> +  if (putgrent (grp, stdout) != 0)
> +    fprintf (stderr, "error writing group entry: %m\n");

OK.

>  }
>  
>  static int
> @@ -241,32 +229,8 @@ group_keys (int number, char *key[])
>  static void
>  print_gshadow (struct sgrp *sg)
>  {
> -  unsigned int i = 0;
> -
> -  printf ("%s:%s:",
> -	  sg->sg_namp ? sg->sg_namp : "",
> -	  sg->sg_passwd ? sg->sg_passwd : "");
> -
> -  while (sg->sg_adm[i] != NULL)
> -    {
> -      fputs_unlocked (sg->sg_adm[i], stdout);
> -      ++i;
> -      if (sg->sg_adm[i] != NULL)
> -	putchar_unlocked (',');
> -    }
> -
> -  putchar_unlocked (':');
> -
> -  i = 0;
> -  while (sg->sg_mem[i] != NULL)
> -    {
> -      fputs_unlocked (sg->sg_mem[i], stdout);
> -      ++i;
> -      if (sg->sg_mem[i] != NULL)
> -	putchar_unlocked (',');
> -    }
> -
> -  putchar_unlocked ('\n');
> +  if (putsgent (sg, stdout) != 0)
> +    fprintf (stderr, "error writing gshadow entry: %m\n");

OK.

>  }
>  
>  static int
> @@ -603,14 +567,8 @@ networks_keys (int number, char *key[])
>  static void
>  print_passwd (struct passwd *pwd)
>  {
> -  printf ("%s:%s:%lu:%lu:%s:%s:%s\n",
> -	  pwd->pw_name ? pwd->pw_name : "",
> -	  pwd->pw_passwd ? pwd->pw_passwd : "",
> -	  (unsigned long int) pwd->pw_uid,
> -	  (unsigned long int) pwd->pw_gid,
> -	  pwd->pw_gecos ? pwd->pw_gecos : "",
> -	  pwd->pw_dir ? pwd->pw_dir : "",
> -	  pwd->pw_shell ? pwd->pw_shell : "");
> +  if (putpwent (pwd, stdout) != 0)
> +    fprintf (stderr, "error writing passwd entry: %m\n");

OK.

>  }
>  
>  static int
> @@ -812,26 +770,8 @@ services_keys (int number, char *key[])
>  static void
>  print_shadow (struct spwd *sp)
>  {
> -  printf ("%s:%s:",
> -	  sp->sp_namp ? sp->sp_namp : "",
> -	  sp->sp_pwdp ? sp->sp_pwdp : "");
> -
> -#define SHADOW_FIELD(n) \
> -  if (sp->n == -1)							      \
> -    putchar_unlocked (':');						      \
> -  else									      \
> -    printf ("%ld:", sp->n)
> -
> -  SHADOW_FIELD (sp_lstchg);
> -  SHADOW_FIELD (sp_min);
> -  SHADOW_FIELD (sp_max);
> -  SHADOW_FIELD (sp_warn);
> -  SHADOW_FIELD (sp_inact);
> -  SHADOW_FIELD (sp_expire);
> -  if (sp->sp_flag == ~0ul)
> -    putchar_unlocked ('\n');
> -  else
> -    printf ("%lu\n", sp->sp_flag);
> +  if (putspent (sp, stdout) != 0)
> +    fprintf (stderr, "error writing shadow entry: %m\n");

OK.

>  }
>  
>  static int
> diff --git a/nss/rewrite_field.c b/nss/rewrite_field.c
> new file mode 100644
> index 0000000..fb9d274
> --- /dev/null
> +++ b/nss/rewrite_field.c
> @@ -0,0 +1,51 @@
> +/* 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 <nss.h>
> +#include <string.h>
> +
> +/* Rewrite VALUE to a valid field value in the NSS database.  Invalid
> +   characters are replaced with a single space character ' '.  If
> +   VALUE is NULL, the empty string is returned.  *TO_BE_FREED is
> +   overwritten with a pointer the caller has to free if the function
> +   returns successfully.  On failure, return NULL.  */
> +const char *
> +__nss_rewrite_field (const char *value, char **to_be_freed)
> +{
> +  *to_be_freed = NULL;
> +  if (value == NULL)
> +    return "";
> +
> +  /* Search for non-allowed characters.  */
> +  const char *p = strpbrk (value, __nss_invalid_field_characters);
> +  if (p == NULL)
> +    return value;
> +  *to_be_freed = __strdup (value);
> +  if (*to_be_freed == NULL)
> +    return NULL;
> +
> +  /* Switch pointer to freshly-allocated buffer.  */
> +  char *bad = *to_be_freed + (p - value);
> +  do
> +    {
> +      *bad = ' ';
> +      bad = strpbrk (bad + 1, __nss_invalid_field_characters);
> +    }
> +  while (bad != NULL);
> +
> +  return *to_be_freed;

OK.

> +}
> diff --git a/nss/tst-field.c b/nss/tst-field.c
> new file mode 100644
> index 0000000..4aab29c
> --- /dev/null
> +++ b/nss/tst-field.c
> @@ -0,0 +1,100 @@

Needs first line description of test with reference BZ #.

> +/* 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/>.  */
> +
> +/* This test needs to be statically linked because it access hidden
> +   functions.  */
> +
> +#include <nss.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +static int errors;
> +
> +static void
> +check (const char *what, _Bool expr)
> +{
> +  if (!expr)
> +    {
> +      printf ("FAIL: %s\n", what);
> +      ++errors;
> +    }
> +}
> +
> +#define CHECK(expr) check (#expr, (expr))
> +
> +static void
> +check_rewrite (const char *input, const char *expected)
> +{
> +  char *to_free;
> +  const char *result = __nss_rewrite_field (input, &to_free);
> +  CHECK (result != NULL);
> +  if (result != NULL && strcmp (result, expected) != 0)
> +    {
> +      printf ("FAIL: rewrite \"%s\" -> \"%s\", expected \"%s\"\n",
> +	      input, result, expected);
> +      ++errors;
> +    }
> +  free (to_free);
> +}
> +
> +static int
> +do_test (void)
> +{
> +  CHECK (__nss_valid_field (NULL));
> +  CHECK (__nss_valid_field (""));
> +  CHECK (__nss_valid_field ("+"));
> +  CHECK (__nss_valid_field ("-"));
> +  CHECK (__nss_valid_field (" "));
> +  CHECK (__nss_valid_field ("abcdef"));
> +  CHECK (__nss_valid_field ("abc def"));
> +  CHECK (__nss_valid_field ("abc\tdef"));
> +  CHECK (!__nss_valid_field ("abcdef:"));
> +  CHECK (!__nss_valid_field ("abcde:f"));
> +  CHECK (!__nss_valid_field (":abcdef"));
> +  CHECK (!__nss_valid_field ("abcdef\n"));
> +  CHECK (!__nss_valid_field ("\nabcdef"));
> +  CHECK (!__nss_valid_field (":"));
> +  CHECK (!__nss_valid_field ("\n"));
> +
> +  CHECK (__nss_valid_list_field (NULL));
> +  CHECK (__nss_valid_list_field ((char *[]) {(char *) "good", NULL}));
> +  CHECK (!__nss_valid_list_field ((char *[]) {(char *) "g,ood", NULL}));
> +  CHECK (!__nss_valid_list_field ((char *[]) {(char *) "g\nood", NULL}));
> +  CHECK (!__nss_valid_list_field ((char *[]) {(char *) "g:ood", NULL}));
> +
> +  check_rewrite (NULL, "");
> +  check_rewrite ("", "");
> +  check_rewrite ("abc", "abc");
> +  check_rewrite ("abc\n", "abc ");
> +  check_rewrite ("abc:", "abc ");
> +  check_rewrite ("\nabc", " abc");
> +  check_rewrite (":abc", " abc");
> +  check_rewrite (":", " ");
> +  check_rewrite ("\n", " ");
> +  check_rewrite ("a:b:c", "a b c");
> +  check_rewrite ("a\nb\nc", "a b c");
> +  check_rewrite ("a\nb:c", "a b c");
> +  check_rewrite ("aa\nbb\ncc", "aa bb cc");
> +  check_rewrite ("aa\nbb:cc", "aa bb cc");
> +
> +  return errors > 0;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> +
> diff --git a/nss/valid_field.c b/nss/valid_field.c
> new file mode 100644
> index 0000000..5fcddc5
> --- /dev/null
> +++ b/nss/valid_field.c
> @@ -0,0 +1,31 @@
> +/* 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 <nss.h>
> +#include <string.h>
> +
> +const char __nss_invalid_field_characters[] = NSS_INVALID_FIELD_CHARACTERS;
> +
> +/* Check that VALUE is either NULL or a NUL-terminated string which
> +   does not contain characters not permitted in NSS database
> +   fields.  */
> +_Bool
> +__nss_valid_field (const char *value)
> +{
> +  return value == NULL
> +    || strpbrk (value, __nss_invalid_field_characters) == NULL;
> +}

OK.

> diff --git a/nss/valid_list_field.c b/nss/valid_list_field.c
> new file mode 100644
> index 0000000..98ab93b
> --- /dev/null
> +++ b/nss/valid_list_field.c
> @@ -0,0 +1,35 @@
> +/* 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 <nss.h>
> +#include <stdbool.h>
> +#include <string.h>
> +
> +static const char invalid_characters[] = NSS_INVALID_FIELD_CHARACTERS ",";
> +
> +/* Check that all list members match the field syntax requirements and
> +   do not contain the character ','.  */
> +_Bool
> +__nss_valid_list_field (char **list)
> +{
> +  if (list == NULL)
> +    return true;
> +  for (; *list != NULL; ++list)
> +    if (strpbrk (*list, invalid_characters) != NULL)
> +      return false;
> +  return true;
> +}

OK.

> diff --git a/pwd/Makefile b/pwd/Makefile
> index 7f6de03..af09734 100644
> --- a/pwd/Makefile
> +++ b/pwd/Makefile
> @@ -28,7 +28,7 @@ routines := fgetpwent getpw putpwent \
>  	    getpwent getpwnam getpwuid \
>  	    getpwent_r getpwnam_r getpwuid_r fgetpwent_r
>  
> -tests := tst-getpw
> +tests := tst-getpw tst-putpwent
>  
>  include ../Rules
>  
> diff --git a/pwd/putpwent.c b/pwd/putpwent.c
> index 8be58c2..16b9c6f 100644
> --- a/pwd/putpwent.c
> +++ b/pwd/putpwent.c
> @@ -18,37 +18,47 @@
>  #include <errno.h>
>  #include <stdio.h>
>  #include <pwd.h>
> +#include <nss.h>
>  
>  #define _S(x)	x ?: ""
>  
> -/* Write an entry to the given stream.
> -   This must know the format of the password file.  */
> +/* Write an entry to the given stream.  This must know the format of
> +   the password file.  If the input contains invalid characters,
> +   return EINVAL, or replace them with spaces (if they are contained
> +   in the GECOS field).  */

OK.

>  int
> -putpwent (p, stream)
> -     const struct passwd *p;
> -     FILE *stream;
> +putpwent (const struct passwd *p, FILE *stream)
>  {
> -  if (p == NULL || stream == NULL)
> +  if (p == NULL || stream == NULL
> +      || p->pw_name == NULL || !__nss_valid_field (p->pw_name)
> +      || !__nss_valid_field (p->pw_passwd)
> +      || !__nss_valid_field (p->pw_dir)
> +      || !__nss_valid_field (p->pw_shell))

OK.

>      {
>        __set_errno (EINVAL);
>        return -1;
>      }
>  
> +  int ret;
> +  char *gecos_alloc;
> +  const char *gecos = __nss_rewrite_field (p->pw_gecos, &gecos_alloc);
> +
> +  if (gecos == NULL)
> +    return -1;
> +
>    if (p->pw_name[0] == '+' || p->pw_name[0] == '-')
> -    {
> -      if (fprintf (stream, "%s:%s:::%s:%s:%s\n",
> -		   p->pw_name, _S (p->pw_passwd),
> -		   _S (p->pw_gecos), _S (p->pw_dir), _S (p->pw_shell)) < 0)
> -	return -1;
> -    }
> +      ret = fprintf (stream, "%s:%s:::%s:%s:%s\n",
> +		     p->pw_name, _S (p->pw_passwd),
> +		     gecos, _S (p->pw_dir), _S (p->pw_shell));
>    else
> -    {
> -      if (fprintf (stream, "%s:%s:%lu:%lu:%s:%s:%s\n",
> -		   p->pw_name, _S (p->pw_passwd),
> -		   (unsigned long int) p->pw_uid,
> -		   (unsigned long int) p->pw_gid,
> -		   _S (p->pw_gecos), _S (p->pw_dir), _S (p->pw_shell)) < 0)
> -	return -1;
> -    }
> -  return 0;
> +      ret = fprintf (stream, "%s:%s:%lu:%lu:%s:%s:%s\n",
> +		     p->pw_name, _S (p->pw_passwd),
> +		     (unsigned long int) p->pw_uid,
> +		     (unsigned long int) p->pw_gid,
> +		     gecos, _S (p->pw_dir), _S (p->pw_shell));
> +
> +  free (gecos_alloc);
> +  if (ret >= 0)
> +    ret = 0;
> +  return ret;
>  }

OK.

> diff --git a/pwd/pwd.h b/pwd/pwd.h
> index fcfb2ab..70a051d 100644
> --- a/pwd/pwd.h
> +++ b/pwd/pwd.h
> @@ -100,7 +100,7 @@ extern struct passwd *fgetpwent (FILE *__stream) __nonnull ((1));
>     or due to the implementation it is a cancellation point and
>     therefore not marked with __THROW.  */
>  extern int putpwent (const struct passwd *__restrict __p,
> -		     FILE *__restrict __f) __nonnull ((1, 2));
> +		     FILE *__restrict __f);
>  #endif
>  
>  /* Search for an entry with a matching user ID.
> diff --git a/pwd/tst-putpwent.c b/pwd/tst-putpwent.c
> new file mode 100644
> index 0000000..cff239c
> --- /dev/null
> +++ b/pwd/tst-putpwent.c
> @@ -0,0 +1,166 @@

Needs first line description of test with reference BZ #.

> +/* 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 <pwd.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +static int errors;
> +
> +static void
> +check (struct passwd p, 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;
> +      return;
> +    }
> +
> +  int ret = putpwent (&p, f);
> +
> +  if (expected == NULL)
> +    {
> +      if (ret == -1)
> +	{
> +	  if (errno != EINVAL)
> +	    {
> +	      printf ("putpwent: unexpected error code: %m\n");
> +	      ++errors;
> +	    }
> +	}
> +      else
> +	{
> +	  printf ("putpwent: unexpected success (\"%s\")\n", p.pw_name);
> +	  ++errors;
> +	}
> +    }
> +  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;
> +	    }
> +	  else if (buf[written - 1] != '\n')
> +	    {
> +	      printf ("FAILED: \"%s\" without newline\n", expected);
> +	      ++errors;
> +	    }
> +	  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;
> +	    }
> +	}
> +      else
> +	{
> +	  printf ("FAILED: putpwent (expected \"%s\"): %m\n", expected);
> +	  ++errors;
> +	}
> +    }
> +
> +  fclose (f);
> +  free (buf);
> +}
> +
> +static int
> +do_test (void)
> +{
> +  check ((struct passwd) {
> +      .pw_name = (char *) "root",
> +    },
> +    "root::0:0:::");
> +  check ((struct passwd) {
> +      .pw_name = (char *) "root",
> +      .pw_passwd = (char *) "password",
> +    },
> +    "root:password:0:0:::");
> +  check ((struct passwd) {
> +      .pw_name = (char *) "root",
> +      .pw_passwd = (char *) "password",
> +      .pw_uid = 12,
> +      .pw_gid = 34,
> +      .pw_gecos = (char *) "gecos",
> +      .pw_dir = (char *) "home",
> +      .pw_shell = (char *) "shell",
> +    },
> +    "root:password:12:34:gecos:home:shell");
> +  check ((struct passwd) {
> +      .pw_name = (char *) "root",
> +      .pw_passwd = (char *) "password",
> +      .pw_uid = 12,
> +      .pw_gid = 34,
> +      .pw_gecos = (char *) ":ge\n:cos\n",
> +      .pw_dir = (char *) "home",
> +      .pw_shell = (char *) "shell",
> +    },
> +    "root:password:12:34: ge  cos :home:shell");
> +
> +  /* Bad values.  */
> +  {
> +    static const char *const bad_strings[] = {
> +      ":",
> +      "\n",
> +      ":bad",
> +      "\nbad",
> +      "b:ad",
> +      "b\nad",
> +      "bad:",
> +      "bad\n",
> +      "b:a\nd",
> +      NULL
> +    };
> +    for (const char *const *bad = bad_strings; *bad != NULL; ++bad)
> +      {
> +	check ((struct passwd) {
> +	    .pw_name = (char *) *bad,
> +	  }, NULL);
> +	check ((struct passwd) {
> +	    .pw_name = (char *) "root",
> +	    .pw_passwd = (char *) *bad,
> +	  }, NULL);
> +	check ((struct passwd) {
> +	    .pw_name = (char *) "root",
> +	    .pw_dir = (char *) *bad,
> +	  }, NULL);
> +	check ((struct passwd) {
> +	    .pw_name = (char *) "root",
> +	    .pw_shell = (char *) *bad,
> +	  }, NULL);
> +      }
> +  }
> +
> +  return errors > 0;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> diff --git a/shadow/Makefile b/shadow/Makefile
> index 8291c61..6990f33 100644
> --- a/shadow/Makefile
> +++ b/shadow/Makefile
> @@ -27,7 +27,7 @@ routines	= getspent getspnam sgetspent fgetspent putspent \
>  		  getspent_r getspnam_r sgetspent_r fgetspent_r \
>  		  lckpwdf
>  
> -tests = tst-shadow
> +tests = tst-shadow tst-putspent
>  
>  CFLAGS-getspent_r.c = -fexceptions
>  CFLAGS-getspent.c = -fexceptions
> diff --git a/shadow/putspent.c b/shadow/putspent.c
> index 142e697..ba8230a 100644
> --- a/shadow/putspent.c
> +++ b/shadow/putspent.c
> @@ -15,6 +15,8 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> +#include <errno.h>
> +#include <nss.h>
>  #include <stdio.h>
>  #include <shadow.h>
>  
> @@ -31,6 +33,13 @@ putspent (const struct spwd *p, FILE *stream)
>  {
>    int errors = 0;
>  
> +  if (p->sp_namp == NULL || !__nss_valid_field (p->sp_namp)
> +      || !__nss_valid_field (p->sp_pwdp))
> +    {
> +      __set_errno (EINVAL);
> +      return -1;
> +    }
> +

OK.

>    flockfile (stream);
>  
>    if (fprintf (stream, "%s:%s:", p->sp_namp, _S (p->sp_pwdp)) < 0)
> diff --git a/shadow/tst-putspent.c b/shadow/tst-putspent.c
> new file mode 100644
> index 0000000..4554ee4
> --- /dev/null
> +++ b/shadow/tst-putspent.c
> @@ -0,0 +1,162 @@

Needs first line description of test with reference BZ #.

> +/* 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 <shadow.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +static int errors;
> +
> +static void
> +check (struct spwd p, 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;
> +      return;
> +    }
> +
> +  int ret = putspent (&p, f);
> +
> +  if (expected == NULL)
> +    {
> +      if (ret == -1)
> +	{
> +	  if (errno != EINVAL)
> +	    {
> +	      printf ("putspent: unexpected error code: %m\n");
> +	      ++errors;
> +	    }
> +	}
> +      else
> +	{
> +	  printf ("putspent: unexpected success (\"%s\")\n", p.sp_namp);
> +	  ++errors;
> +	}
> +    }
> +  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;
> +	    }
> +	  else if (buf[written - 1] != '\n')
> +	    {
> +	      printf ("FAILED: \"%s\" without newline\n", expected);
> +	      ++errors;
> +	    }
> +	  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;
> +	    }
> +	}
> +      else
> +	{
> +	  printf ("FAILED: putspent (expected \"%s\"): %m\n", expected);
> +	  ++errors;
> +	}
> +    }
> +
> +  fclose (f);
> +  free (buf);
> +}
> +
> +static int
> +do_test (void)
> +{
> +  check ((struct spwd) {
> +      .sp_namp = (char *) "root",
> +    },
> +    "root::0:0:0:0:0:0:0");
> +  check ((struct spwd) {
> +      .sp_namp = (char *) "root",
> +      .sp_pwdp = (char *) "password",
> +    },
> +    "root:password:0:0:0:0:0:0:0");
> +  check ((struct spwd) {
> +      .sp_namp = (char *) "root",
> +      .sp_pwdp = (char *) "password",
> +      .sp_lstchg = -1,
> +      .sp_min = -1,
> +      .sp_max = -1,
> +      .sp_warn = -1,
> +      .sp_inact = -1,
> +      .sp_expire = -1,
> +      .sp_flag = -1
> +    },
> +    "root:password:::::::");
> +  check ((struct spwd) {
> +      .sp_namp = (char *) "root",
> +      .sp_pwdp = (char *) "password",
> +      .sp_lstchg = 1,
> +      .sp_min = 2,
> +      .sp_max = 3,
> +      .sp_warn = 4,
> +      .sp_inact = 5,
> +      .sp_expire = 6,
> +      .sp_flag = 7
> +    },
> +    "root:password:1:2:3:4:5:6:7");
> +
> +  /* Bad values.  */
> +  {
> +    static const char *const bad_strings[] = {
> +      ":",
> +      "\n",
> +      ":bad",
> +      "\nbad",
> +      "b:ad",
> +      "b\nad",
> +      "bad:",
> +      "bad\n",
> +      "b:a\nd",
> +      NULL
> +    };
> +    for (const char *const *bad = bad_strings; *bad != NULL; ++bad)
> +      {
> +	check ((struct spwd) {
> +	    .sp_namp = (char *) *bad,
> +	  }, NULL);
> +	check ((struct spwd) {
> +	    .sp_namp = (char *) "root",
> +	    .sp_pwdp = (char *) *bad,
> +	  }, NULL);
> +      }
> +  }
> +
> +  return errors > 0;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> -- 2.4.3
Mike Frysinger July 28, 2015, 3:19 a.m. UTC | #2
On 27 Jul 2015 17:18, Florian Weimer wrote:
> -      int i;
> -
> -      for (i = 0 ; gr->gr_mem[i] != NULL; i++)
> +      for (size_t i = 0 ; gr->gr_mem[i] != NULL; i++)

if you're tweaking style(ish), should trim the space before the first ; too


> --- /dev/null
> +++ b/grp/tst-putgrent.c
>
> +      ++errors;
> ...
> +  return errors > 0;

is an error count really necessary ?  just make it a bool.
<paranoid>don't want it to overflow</paranoid>

> +check (const char *what, _Bool expr)

why not "bool" ?
-mike
Florian Weimer July 28, 2015, 12:30 p.m. UTC | #3
On 07/28/2015 05:19 AM, Mike Frysinger wrote:
> On 27 Jul 2015 17:18, Florian Weimer wrote:
>> -      int i;
>> -
>> -      for (i = 0 ; gr->gr_mem[i] != NULL; i++)
>> +      for (size_t i = 0 ; gr->gr_mem[i] != NULL; i++)
> 
> if you're tweaking style(ish), should trim the space before the first ; too

Fixed.

Carlos, I also added the BZ# references to the test cases.

>> --- /dev/null
>> +++ b/grp/tst-putgrent.c
>>
>> +      ++errors;
>> ...
>> +  return errors > 0;
> 
> is an error count really necessary ?  just make it a bool.

“++errors;” is clearer to me than “errors |= true;”.  Other test suite
code uses the counter approach, too.

> <paranoid>don't want it to overflow</paranoid>

Can't really happen here. :)

>> +check (const char *what, _Bool expr)
> 
> why not "bool" ?

It requires #include <stdbool.h>.
Mike Frysinger July 28, 2015, 3:50 p.m. UTC | #4
On 28 Jul 2015 14:30, Florian Weimer wrote:
> On 07/28/2015 05:19 AM, Mike Frysinger wrote:
> > On 27 Jul 2015 17:18, Florian Weimer wrote:
> >> --- /dev/null
> >> +++ b/grp/tst-putgrent.c
> >>
> >> +      ++errors;
> >> ...
> >> +  return errors > 0;
> > 
> > is an error count really necessary ?  just make it a bool.
> 
> “++errors;” is clearer to me than “errors |= true;”.  Other test suite
> code uses the counter approach, too.

i meant:
	error = true;
or:
	error = 1;

there's no point in using |= operations here

> >> +check (const char *what, _Bool expr)
> > 
> > why not "bool" ?
> 
> It requires #include <stdbool.h>.

so ?  include it then.  better imo than using private _Bool types.
-mike
Andreas Schwab July 28, 2015, 4:06 p.m. UTC | #5
Mike Frysinger <vapier@gentoo.org> writes:

> On 28 Jul 2015 14:30, Florian Weimer wrote:
>> On 07/28/2015 05:19 AM, Mike Frysinger wrote:
>> > On 27 Jul 2015 17:18, Florian Weimer wrote:
>> >> --- /dev/null
>> >> +++ b/grp/tst-putgrent.c
>> >>
>> >> +      ++errors;
>> >> ...
>> >> +  return errors > 0;
>> > 
>> > is an error count really necessary ?  just make it a bool.
>> 
>> “++errors;” is clearer to me than “errors |= true;”.  Other test suite
>> code uses the counter approach, too.
>
> i meant:
> 	error = true;
> or:
> 	error = 1;
>
> there's no point in using |= operations here

FWIW, ++errors is doing TRT for bool (equivalent to errors = true).

Andreas.
Florian Weimer Oct. 2, 2015, 9:38 a.m. UTC | #6
On 07/27/2015 09:59 PM, Carlos O'Donell wrote:
> On 07/27/2015 11:18 AM, Florian Weimer wrote:
>> This patch addresses a “Bobby Tables” issue in the put*ent functions and
>> the getent program, similar to one of the recent libuser issues.
>>
>> I believe this is just hardening because users of the put*ent functions
>> already have appropriate checks before they call these functions, so
>> this is definitely post-freeze material.
>>
>> Tested on x86_64-redhat-linux-gnu.  Okay to commit after master reopens?
> 
> Looks good to me for 2.23 with testsuite comment nits fixed.

Thanks, I committed this with the changes suggested in this thread.

Florian
diff mbox

Patch

From ca909fd7c61e7821df24109182f8ec3201690e67 Mon Sep 17 00:00:00 2001
Message-Id: <ca909fd7c61e7821df24109182f8ec3201690e67.1438010163.git.fweimer@redhat.com>
From: Florian Weimer <fweimer@redhat.com>
Date: Mon, 27 Jul 2015 17:13:05 +0200
Subject: [PATCH] Harden putpwent, putgrent, putspent, putspent against
 injection [BZ #18724]
To: libc-alpha@sourceware.org

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 '+'.
---
 ChangeLog              |  38 +++++++++++
 NEWS                   |   3 +-
 grp/Makefile           |   2 +-
 grp/putgrent.c         |  15 ++---
 grp/tst-putgrent.c     | 165 ++++++++++++++++++++++++++++++++++++++++++++++++
 gshadow/Makefile       |   2 +-
 gshadow/putsgent.c     |  11 ++++
 gshadow/tst-putsgent.c | 166 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/nss.h          |  13 ++++
 include/pwd.h          |   2 +-
 nss/Makefile           |   8 ++-
 nss/getent.c           |  76 +++-------------------
 nss/rewrite_field.c    |  51 +++++++++++++++
 nss/tst-field.c        | 100 +++++++++++++++++++++++++++++
 nss/valid_field.c      |  31 +++++++++
 nss/valid_list_field.c |  35 +++++++++++
 pwd/Makefile           |   2 +-
 pwd/putpwent.c         |  52 +++++++++-------
 pwd/pwd.h              |   2 +-
 pwd/tst-putpwent.c     | 166 +++++++++++++++++++++++++++++++++++++++++++++++++
 shadow/Makefile        |   2 +-
 shadow/putspent.c      |   9 +++
 shadow/tst-putspent.c  | 162 +++++++++++++++++++++++++++++++++++++++++++++++
 23 files changed, 1007 insertions(+), 106 deletions(-)
 create mode 100644 grp/tst-putgrent.c
 create mode 100644 gshadow/tst-putsgent.c
 create mode 100644 nss/rewrite_field.c
 create mode 100644 nss/tst-field.c
 create mode 100644 nss/valid_field.c
 create mode 100644 nss/valid_list_field.c
 create mode 100644 pwd/tst-putpwent.c
 create mode 100644 shadow/tst-putspent.c

diff --git a/ChangeLog b/ChangeLog
index 3e202c4..e9d0335 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,41 @@ 
+2015-07-27  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-07-26  Chung-Lin Tang  <cltang@codesourcery.com>
 
 	* sysdeps/nios2/dl-sysdep.h (DL_EXTERN_PROTECTED_DATA): Define.
diff --git a/NEWS b/NEWS
index 65b2172..649c04b 100644
--- a/NEWS
+++ b/NEWS
@@ -27,7 +27,8 @@  Version 2.22
   18520, 18522, 18527, 18528, 18529, 18530, 18532, 18533, 18534, 18536,
   18539, 18540, 18542, 18544, 18545, 18546, 18547, 18549, 18553, 18557,
   18558, 18569, 18583, 18585, 18586, 18592, 18593, 18594, 18602, 18612,
-  18613, 18619, 18633, 18641, 18643, 18648, 18657, 18676, 18694, 18696.
+  18613, 18619, 18633, 18641, 18643, 18648, 18657, 18676, 18694, 18696,
+  18724.
 
 * Cache information can be queried via sysconf() function on s390 e.g. with
   _SC_LEVEL1_ICACHE_SIZE as argument.
diff --git a/grp/Makefile b/grp/Makefile
index c63b552..ed8cc2b 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 e4d662c..3c06d1a 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 0000000..3f259ca
--- /dev/null
+++ b/grp/tst-putgrent.c
@@ -0,0 +1,165 @@ 
+/* 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 <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+static int 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;
+      return;
+    }
+
+  int ret = putgrent (&e, f);
+
+  if (expected == NULL)
+    {
+      if (ret == -1)
+	{
+	  if (errno != EINVAL)
+	    {
+	      printf ("putgrent: unexpected error code: %m\n");
+	      ++errors;
+	    }
+	}
+      else
+	{
+	  printf ("putgrent: unexpected success (\"%s\", \"%s\")\n",
+		  e.gr_name, e.gr_passwd);
+	  ++errors;
+	}
+    }
+  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;
+	    }
+	  else if (buf[written - 1] != '\n')
+	    {
+	      printf ("FAILED: \"%s\" without newline\n", expected);
+	      ++errors;
+	    }
+	  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;
+	    }
+	}
+      else
+	{
+	  printf ("FAILED: putgrent (expected \"%s\"): %m\n", expected);
+	  ++errors;
+	}
+    }
+
+  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 > 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/gshadow/Makefile b/gshadow/Makefile
index 883e1c8..c811041 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 0b2cad6..c1cb921 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 0000000..f628fe6
--- /dev/null
+++ b/gshadow/tst-putsgent.c
@@ -0,0 +1,166 @@ 
+/* 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 <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+static int 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;
+      return;
+    }
+
+  int ret = putsgent (&e, f);
+
+  if (expected == NULL)
+    {
+      if (ret == -1)
+	{
+	  if (errno != EINVAL)
+	    {
+	      printf ("putsgent: unexpected error code: %m\n");
+	      ++errors;
+	    }
+	}
+      else
+	{
+	  printf ("putsgent: unexpected success (\"%s\")\n", e.sg_namp);
+	  ++errors;
+	}
+    }
+  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;
+	    }
+	  else if (buf[written - 1] != '\n')
+	    {
+	      printf ("FAILED: \"%s\" without newline\n", expected);
+	      ++errors;
+	    }
+	  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;
+	    }
+	}
+      else
+	{
+	  printf ("FAILED: putsgent (expected \"%s\"): %m\n", expected);
+	  ++errors;
+	}
+    }
+
+  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 > 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/include/nss.h b/include/nss.h
index 0541335..1e8cc39 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 bd7fecc..3b0f725 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 65ab7b5..9da8b15 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 getnssent getnssent_r digits_dots \
+			  valid_field valid_list_field rewrite_field \
 			  $(addsuffix -lookup,$(databases))
 
 # These are the databases that go through nss dispatch.
@@ -47,7 +48,9 @@  install-bin             := getent makedb
 makedb-modules = xmalloc hash-string
 extra-objs		+= $(makedb-modules:=.o)
 
-tests			= test-netdb tst-nss-test1 test-digits-dots tst-nss-getpwent
+tests-static            = tst-field
+tests			= test-netdb tst-nss-test1 test-digits-dots tst-nss-getpwent \
+			  $(tests-static)
 xtests			= bug-erange
 
 # Specify rules for the nss_* modules.  We have some services.
@@ -82,8 +85,7 @@  libnss_db-inhibit-o	= $(filter-out .os,$(object-suffixes))
 ifeq ($(build-static-nss),yes)
 routines                += $(libnss_files-routines)
 static-only-routines    += $(libnss_files-routines)
-tests-static		= tst-nss-static
-tests			+= $(tests-static)
+tests-static		+= tst-nss-static
 endif
 
 include ../Rules
diff --git a/nss/getent.c b/nss/getent.c
index 34df848..996d378 100644
--- a/nss/getent.c
+++ b/nss/getent.c
@@ -184,20 +184,8 @@  ethers_keys (int number, char *key[])
 static void
 print_group (struct group *grp)
 {
-  unsigned int i = 0;
-
-  printf ("%s:%s:%lu:", grp->gr_name ? grp->gr_name : "",
-	  grp->gr_passwd ? grp->gr_passwd : "",
-	  (unsigned long int) grp->gr_gid);
-
-  while (grp->gr_mem[i] != NULL)
-    {
-      fputs_unlocked (grp->gr_mem[i], stdout);
-      ++i;
-      if (grp->gr_mem[i] != NULL)
-	putchar_unlocked (',');
-    }
-  putchar_unlocked ('\n');
+  if (putgrent (grp, stdout) != 0)
+    fprintf (stderr, "error writing group entry: %m\n");
 }
 
 static int
@@ -241,32 +229,8 @@  group_keys (int number, char *key[])
 static void
 print_gshadow (struct sgrp *sg)
 {
-  unsigned int i = 0;
-
-  printf ("%s:%s:",
-	  sg->sg_namp ? sg->sg_namp : "",
-	  sg->sg_passwd ? sg->sg_passwd : "");
-
-  while (sg->sg_adm[i] != NULL)
-    {
-      fputs_unlocked (sg->sg_adm[i], stdout);
-      ++i;
-      if (sg->sg_adm[i] != NULL)
-	putchar_unlocked (',');
-    }
-
-  putchar_unlocked (':');
-
-  i = 0;
-  while (sg->sg_mem[i] != NULL)
-    {
-      fputs_unlocked (sg->sg_mem[i], stdout);
-      ++i;
-      if (sg->sg_mem[i] != NULL)
-	putchar_unlocked (',');
-    }
-
-  putchar_unlocked ('\n');
+  if (putsgent (sg, stdout) != 0)
+    fprintf (stderr, "error writing gshadow entry: %m\n");
 }
 
 static int
@@ -603,14 +567,8 @@  networks_keys (int number, char *key[])
 static void
 print_passwd (struct passwd *pwd)
 {
-  printf ("%s:%s:%lu:%lu:%s:%s:%s\n",
-	  pwd->pw_name ? pwd->pw_name : "",
-	  pwd->pw_passwd ? pwd->pw_passwd : "",
-	  (unsigned long int) pwd->pw_uid,
-	  (unsigned long int) pwd->pw_gid,
-	  pwd->pw_gecos ? pwd->pw_gecos : "",
-	  pwd->pw_dir ? pwd->pw_dir : "",
-	  pwd->pw_shell ? pwd->pw_shell : "");
+  if (putpwent (pwd, stdout) != 0)
+    fprintf (stderr, "error writing passwd entry: %m\n");
 }
 
 static int
@@ -812,26 +770,8 @@  services_keys (int number, char *key[])
 static void
 print_shadow (struct spwd *sp)
 {
-  printf ("%s:%s:",
-	  sp->sp_namp ? sp->sp_namp : "",
-	  sp->sp_pwdp ? sp->sp_pwdp : "");
-
-#define SHADOW_FIELD(n) \
-  if (sp->n == -1)							      \
-    putchar_unlocked (':');						      \
-  else									      \
-    printf ("%ld:", sp->n)
-
-  SHADOW_FIELD (sp_lstchg);
-  SHADOW_FIELD (sp_min);
-  SHADOW_FIELD (sp_max);
-  SHADOW_FIELD (sp_warn);
-  SHADOW_FIELD (sp_inact);
-  SHADOW_FIELD (sp_expire);
-  if (sp->sp_flag == ~0ul)
-    putchar_unlocked ('\n');
-  else
-    printf ("%lu\n", sp->sp_flag);
+  if (putspent (sp, stdout) != 0)
+    fprintf (stderr, "error writing shadow entry: %m\n");
 }
 
 static int
diff --git a/nss/rewrite_field.c b/nss/rewrite_field.c
new file mode 100644
index 0000000..fb9d274
--- /dev/null
+++ b/nss/rewrite_field.c
@@ -0,0 +1,51 @@ 
+/* 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 <nss.h>
+#include <string.h>
+
+/* Rewrite VALUE to a valid field value in the NSS database.  Invalid
+   characters are replaced with a single space character ' '.  If
+   VALUE is NULL, the empty string is returned.  *TO_BE_FREED is
+   overwritten with a pointer the caller has to free if the function
+   returns successfully.  On failure, return NULL.  */
+const char *
+__nss_rewrite_field (const char *value, char **to_be_freed)
+{
+  *to_be_freed = NULL;
+  if (value == NULL)
+    return "";
+
+  /* Search for non-allowed characters.  */
+  const char *p = strpbrk (value, __nss_invalid_field_characters);
+  if (p == NULL)
+    return value;
+  *to_be_freed = __strdup (value);
+  if (*to_be_freed == NULL)
+    return NULL;
+
+  /* Switch pointer to freshly-allocated buffer.  */
+  char *bad = *to_be_freed + (p - value);
+  do
+    {
+      *bad = ' ';
+      bad = strpbrk (bad + 1, __nss_invalid_field_characters);
+    }
+  while (bad != NULL);
+
+  return *to_be_freed;
+}
diff --git a/nss/tst-field.c b/nss/tst-field.c
new file mode 100644
index 0000000..4aab29c
--- /dev/null
+++ b/nss/tst-field.c
@@ -0,0 +1,100 @@ 
+/* 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/>.  */
+
+/* This test needs to be statically linked because it access hidden
+   functions.  */
+
+#include <nss.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+static int errors;
+
+static void
+check (const char *what, _Bool expr)
+{
+  if (!expr)
+    {
+      printf ("FAIL: %s\n", what);
+      ++errors;
+    }
+}
+
+#define CHECK(expr) check (#expr, (expr))
+
+static void
+check_rewrite (const char *input, const char *expected)
+{
+  char *to_free;
+  const char *result = __nss_rewrite_field (input, &to_free);
+  CHECK (result != NULL);
+  if (result != NULL && strcmp (result, expected) != 0)
+    {
+      printf ("FAIL: rewrite \"%s\" -> \"%s\", expected \"%s\"\n",
+	      input, result, expected);
+      ++errors;
+    }
+  free (to_free);
+}
+
+static int
+do_test (void)
+{
+  CHECK (__nss_valid_field (NULL));
+  CHECK (__nss_valid_field (""));
+  CHECK (__nss_valid_field ("+"));
+  CHECK (__nss_valid_field ("-"));
+  CHECK (__nss_valid_field (" "));
+  CHECK (__nss_valid_field ("abcdef"));
+  CHECK (__nss_valid_field ("abc def"));
+  CHECK (__nss_valid_field ("abc\tdef"));
+  CHECK (!__nss_valid_field ("abcdef:"));
+  CHECK (!__nss_valid_field ("abcde:f"));
+  CHECK (!__nss_valid_field (":abcdef"));
+  CHECK (!__nss_valid_field ("abcdef\n"));
+  CHECK (!__nss_valid_field ("\nabcdef"));
+  CHECK (!__nss_valid_field (":"));
+  CHECK (!__nss_valid_field ("\n"));
+
+  CHECK (__nss_valid_list_field (NULL));
+  CHECK (__nss_valid_list_field ((char *[]) {(char *) "good", NULL}));
+  CHECK (!__nss_valid_list_field ((char *[]) {(char *) "g,ood", NULL}));
+  CHECK (!__nss_valid_list_field ((char *[]) {(char *) "g\nood", NULL}));
+  CHECK (!__nss_valid_list_field ((char *[]) {(char *) "g:ood", NULL}));
+
+  check_rewrite (NULL, "");
+  check_rewrite ("", "");
+  check_rewrite ("abc", "abc");
+  check_rewrite ("abc\n", "abc ");
+  check_rewrite ("abc:", "abc ");
+  check_rewrite ("\nabc", " abc");
+  check_rewrite (":abc", " abc");
+  check_rewrite (":", " ");
+  check_rewrite ("\n", " ");
+  check_rewrite ("a:b:c", "a b c");
+  check_rewrite ("a\nb\nc", "a b c");
+  check_rewrite ("a\nb:c", "a b c");
+  check_rewrite ("aa\nbb\ncc", "aa bb cc");
+  check_rewrite ("aa\nbb:cc", "aa bb cc");
+
+  return errors > 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
diff --git a/nss/valid_field.c b/nss/valid_field.c
new file mode 100644
index 0000000..5fcddc5
--- /dev/null
+++ b/nss/valid_field.c
@@ -0,0 +1,31 @@ 
+/* 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 <nss.h>
+#include <string.h>
+
+const char __nss_invalid_field_characters[] = NSS_INVALID_FIELD_CHARACTERS;
+
+/* Check that VALUE is either NULL or a NUL-terminated string which
+   does not contain characters not permitted in NSS database
+   fields.  */
+_Bool
+__nss_valid_field (const char *value)
+{
+  return value == NULL
+    || strpbrk (value, __nss_invalid_field_characters) == NULL;
+}
diff --git a/nss/valid_list_field.c b/nss/valid_list_field.c
new file mode 100644
index 0000000..98ab93b
--- /dev/null
+++ b/nss/valid_list_field.c
@@ -0,0 +1,35 @@ 
+/* 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 <nss.h>
+#include <stdbool.h>
+#include <string.h>
+
+static const char invalid_characters[] = NSS_INVALID_FIELD_CHARACTERS ",";
+
+/* Check that all list members match the field syntax requirements and
+   do not contain the character ','.  */
+_Bool
+__nss_valid_list_field (char **list)
+{
+  if (list == NULL)
+    return true;
+  for (; *list != NULL; ++list)
+    if (strpbrk (*list, invalid_characters) != NULL)
+      return false;
+  return true;
+}
diff --git a/pwd/Makefile b/pwd/Makefile
index 7f6de03..af09734 100644
--- a/pwd/Makefile
+++ b/pwd/Makefile
@@ -28,7 +28,7 @@  routines := fgetpwent getpw putpwent \
 	    getpwent getpwnam getpwuid \
 	    getpwent_r getpwnam_r getpwuid_r fgetpwent_r
 
-tests := tst-getpw
+tests := tst-getpw tst-putpwent
 
 include ../Rules
 
diff --git a/pwd/putpwent.c b/pwd/putpwent.c
index 8be58c2..16b9c6f 100644
--- a/pwd/putpwent.c
+++ b/pwd/putpwent.c
@@ -18,37 +18,47 @@ 
 #include <errno.h>
 #include <stdio.h>
 #include <pwd.h>
+#include <nss.h>
 
 #define _S(x)	x ?: ""
 
-/* Write an entry to the given stream.
-   This must know the format of the password file.  */
+/* Write an entry to the given stream.  This must know the format of
+   the password file.  If the input contains invalid characters,
+   return EINVAL, or replace them with spaces (if they are contained
+   in the GECOS field).  */
 int
-putpwent (p, stream)
-     const struct passwd *p;
-     FILE *stream;
+putpwent (const struct passwd *p, FILE *stream)
 {
-  if (p == NULL || stream == NULL)
+  if (p == NULL || stream == NULL
+      || p->pw_name == NULL || !__nss_valid_field (p->pw_name)
+      || !__nss_valid_field (p->pw_passwd)
+      || !__nss_valid_field (p->pw_dir)
+      || !__nss_valid_field (p->pw_shell))
     {
       __set_errno (EINVAL);
       return -1;
     }
 
+  int ret;
+  char *gecos_alloc;
+  const char *gecos = __nss_rewrite_field (p->pw_gecos, &gecos_alloc);
+
+  if (gecos == NULL)
+    return -1;
+
   if (p->pw_name[0] == '+' || p->pw_name[0] == '-')
-    {
-      if (fprintf (stream, "%s:%s:::%s:%s:%s\n",
-		   p->pw_name, _S (p->pw_passwd),
-		   _S (p->pw_gecos), _S (p->pw_dir), _S (p->pw_shell)) < 0)
-	return -1;
-    }
+      ret = fprintf (stream, "%s:%s:::%s:%s:%s\n",
+		     p->pw_name, _S (p->pw_passwd),
+		     gecos, _S (p->pw_dir), _S (p->pw_shell));
   else
-    {
-      if (fprintf (stream, "%s:%s:%lu:%lu:%s:%s:%s\n",
-		   p->pw_name, _S (p->pw_passwd),
-		   (unsigned long int) p->pw_uid,
-		   (unsigned long int) p->pw_gid,
-		   _S (p->pw_gecos), _S (p->pw_dir), _S (p->pw_shell)) < 0)
-	return -1;
-    }
-  return 0;
+      ret = fprintf (stream, "%s:%s:%lu:%lu:%s:%s:%s\n",
+		     p->pw_name, _S (p->pw_passwd),
+		     (unsigned long int) p->pw_uid,
+		     (unsigned long int) p->pw_gid,
+		     gecos, _S (p->pw_dir), _S (p->pw_shell));
+
+  free (gecos_alloc);
+  if (ret >= 0)
+    ret = 0;
+  return ret;
 }
diff --git a/pwd/pwd.h b/pwd/pwd.h
index fcfb2ab..70a051d 100644
--- a/pwd/pwd.h
+++ b/pwd/pwd.h
@@ -100,7 +100,7 @@  extern struct passwd *fgetpwent (FILE *__stream) __nonnull ((1));
    or due to the implementation it is a cancellation point and
    therefore not marked with __THROW.  */
 extern int putpwent (const struct passwd *__restrict __p,
-		     FILE *__restrict __f) __nonnull ((1, 2));
+		     FILE *__restrict __f);
 #endif
 
 /* Search for an entry with a matching user ID.
diff --git a/pwd/tst-putpwent.c b/pwd/tst-putpwent.c
new file mode 100644
index 0000000..cff239c
--- /dev/null
+++ b/pwd/tst-putpwent.c
@@ -0,0 +1,166 @@ 
+/* 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 <pwd.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+static int errors;
+
+static void
+check (struct passwd p, 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;
+      return;
+    }
+
+  int ret = putpwent (&p, f);
+
+  if (expected == NULL)
+    {
+      if (ret == -1)
+	{
+	  if (errno != EINVAL)
+	    {
+	      printf ("putpwent: unexpected error code: %m\n");
+	      ++errors;
+	    }
+	}
+      else
+	{
+	  printf ("putpwent: unexpected success (\"%s\")\n", p.pw_name);
+	  ++errors;
+	}
+    }
+  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;
+	    }
+	  else if (buf[written - 1] != '\n')
+	    {
+	      printf ("FAILED: \"%s\" without newline\n", expected);
+	      ++errors;
+	    }
+	  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;
+	    }
+	}
+      else
+	{
+	  printf ("FAILED: putpwent (expected \"%s\"): %m\n", expected);
+	  ++errors;
+	}
+    }
+
+  fclose (f);
+  free (buf);
+}
+
+static int
+do_test (void)
+{
+  check ((struct passwd) {
+      .pw_name = (char *) "root",
+    },
+    "root::0:0:::");
+  check ((struct passwd) {
+      .pw_name = (char *) "root",
+      .pw_passwd = (char *) "password",
+    },
+    "root:password:0:0:::");
+  check ((struct passwd) {
+      .pw_name = (char *) "root",
+      .pw_passwd = (char *) "password",
+      .pw_uid = 12,
+      .pw_gid = 34,
+      .pw_gecos = (char *) "gecos",
+      .pw_dir = (char *) "home",
+      .pw_shell = (char *) "shell",
+    },
+    "root:password:12:34:gecos:home:shell");
+  check ((struct passwd) {
+      .pw_name = (char *) "root",
+      .pw_passwd = (char *) "password",
+      .pw_uid = 12,
+      .pw_gid = 34,
+      .pw_gecos = (char *) ":ge\n:cos\n",
+      .pw_dir = (char *) "home",
+      .pw_shell = (char *) "shell",
+    },
+    "root:password:12:34: ge  cos :home:shell");
+
+  /* Bad values.  */
+  {
+    static const char *const bad_strings[] = {
+      ":",
+      "\n",
+      ":bad",
+      "\nbad",
+      "b:ad",
+      "b\nad",
+      "bad:",
+      "bad\n",
+      "b:a\nd",
+      NULL
+    };
+    for (const char *const *bad = bad_strings; *bad != NULL; ++bad)
+      {
+	check ((struct passwd) {
+	    .pw_name = (char *) *bad,
+	  }, NULL);
+	check ((struct passwd) {
+	    .pw_name = (char *) "root",
+	    .pw_passwd = (char *) *bad,
+	  }, NULL);
+	check ((struct passwd) {
+	    .pw_name = (char *) "root",
+	    .pw_dir = (char *) *bad,
+	  }, NULL);
+	check ((struct passwd) {
+	    .pw_name = (char *) "root",
+	    .pw_shell = (char *) *bad,
+	  }, NULL);
+      }
+  }
+
+  return errors > 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/shadow/Makefile b/shadow/Makefile
index 8291c61..6990f33 100644
--- a/shadow/Makefile
+++ b/shadow/Makefile
@@ -27,7 +27,7 @@  routines	= getspent getspnam sgetspent fgetspent putspent \
 		  getspent_r getspnam_r sgetspent_r fgetspent_r \
 		  lckpwdf
 
-tests = tst-shadow
+tests = tst-shadow tst-putspent
 
 CFLAGS-getspent_r.c = -fexceptions
 CFLAGS-getspent.c = -fexceptions
diff --git a/shadow/putspent.c b/shadow/putspent.c
index 142e697..ba8230a 100644
--- a/shadow/putspent.c
+++ b/shadow/putspent.c
@@ -15,6 +15,8 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <errno.h>
+#include <nss.h>
 #include <stdio.h>
 #include <shadow.h>
 
@@ -31,6 +33,13 @@  putspent (const struct spwd *p, FILE *stream)
 {
   int errors = 0;
 
+  if (p->sp_namp == NULL || !__nss_valid_field (p->sp_namp)
+      || !__nss_valid_field (p->sp_pwdp))
+    {
+      __set_errno (EINVAL);
+      return -1;
+    }
+
   flockfile (stream);
 
   if (fprintf (stream, "%s:%s:", p->sp_namp, _S (p->sp_pwdp)) < 0)
diff --git a/shadow/tst-putspent.c b/shadow/tst-putspent.c
new file mode 100644
index 0000000..4554ee4
--- /dev/null
+++ b/shadow/tst-putspent.c
@@ -0,0 +1,162 @@ 
+/* 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 <shadow.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+static int errors;
+
+static void
+check (struct spwd p, 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;
+      return;
+    }
+
+  int ret = putspent (&p, f);
+
+  if (expected == NULL)
+    {
+      if (ret == -1)
+	{
+	  if (errno != EINVAL)
+	    {
+	      printf ("putspent: unexpected error code: %m\n");
+	      ++errors;
+	    }
+	}
+      else
+	{
+	  printf ("putspent: unexpected success (\"%s\")\n", p.sp_namp);
+	  ++errors;
+	}
+    }
+  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;
+	    }
+	  else if (buf[written - 1] != '\n')
+	    {
+	      printf ("FAILED: \"%s\" without newline\n", expected);
+	      ++errors;
+	    }
+	  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;
+	    }
+	}
+      else
+	{
+	  printf ("FAILED: putspent (expected \"%s\"): %m\n", expected);
+	  ++errors;
+	}
+    }
+
+  fclose (f);
+  free (buf);
+}
+
+static int
+do_test (void)
+{
+  check ((struct spwd) {
+      .sp_namp = (char *) "root",
+    },
+    "root::0:0:0:0:0:0:0");
+  check ((struct spwd) {
+      .sp_namp = (char *) "root",
+      .sp_pwdp = (char *) "password",
+    },
+    "root:password:0:0:0:0:0:0:0");
+  check ((struct spwd) {
+      .sp_namp = (char *) "root",
+      .sp_pwdp = (char *) "password",
+      .sp_lstchg = -1,
+      .sp_min = -1,
+      .sp_max = -1,
+      .sp_warn = -1,
+      .sp_inact = -1,
+      .sp_expire = -1,
+      .sp_flag = -1
+    },
+    "root:password:::::::");
+  check ((struct spwd) {
+      .sp_namp = (char *) "root",
+      .sp_pwdp = (char *) "password",
+      .sp_lstchg = 1,
+      .sp_min = 2,
+      .sp_max = 3,
+      .sp_warn = 4,
+      .sp_inact = 5,
+      .sp_expire = 6,
+      .sp_flag = 7
+    },
+    "root:password:1:2:3:4:5:6:7");
+
+  /* Bad values.  */
+  {
+    static const char *const bad_strings[] = {
+      ":",
+      "\n",
+      ":bad",
+      "\nbad",
+      "b:ad",
+      "b\nad",
+      "bad:",
+      "bad\n",
+      "b:a\nd",
+      NULL
+    };
+    for (const char *const *bad = bad_strings; *bad != NULL; ++bad)
+      {
+	check ((struct spwd) {
+	    .sp_namp = (char *) *bad,
+	  }, NULL);
+	check ((struct spwd) {
+	    .sp_namp = (char *) "root",
+	    .sp_pwdp = (char *) *bad,
+	  }, NULL);
+      }
+  }
+
+  return errors > 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
-- 
2.4.3