Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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 '+'.
  • Loading branch information
Florian Weimer committed Oct 2, 2015
1 parent b0f8163 commit 676599b
Show file tree
Hide file tree
Showing 22 changed files with 1,018 additions and 109 deletions.
38 changes: 38 additions & 0 deletions 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
Expand Down
10 changes: 5 additions & 5 deletions NEWS
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion grp/Makefile
Expand Up @@ -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
Expand Down
15 changes: 8 additions & 7 deletions grp/putgrent.c
Expand Up @@ -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)
Expand All @@ -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;
Expand All @@ -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? */
Expand Down
167 changes: 167 additions & 0 deletions 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"
2 changes: 1 addition & 1 deletion gshadow/Makefile
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions gshadow/putsgent.c
Expand Up @@ -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 : ""

Expand All @@ -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)
Expand Down

0 comments on commit 676599b

Please sign in to comment.