diff options
author | Florian Weimer <fweimer@redhat.com> | 2015-04-29 14:41:25 +0200 |
---|---|---|
committer | Florian Weimer <fweimer@redhat.com> | 2015-04-29 14:41:26 +0200 |
commit | 03d2730b44cc2236318fd978afa2651753666c55 (patch) | |
tree | 8846df11bd7a2d2b8bd0be49dbf9d69654240e05 /nss | |
parent | 7d0b2575416aec2717e8665287d0ab77826a0ade (diff) | |
download | glibc-03d2730b44cc2236318fd978afa2651753666c55.tar.gz |
CVE-2014-8121: Do not close NSS files database during iteration [BZ #18007]
Robin Hack discovered Samba would enter an infinite loop processing
certain quota-related requests. We eventually tracked this down to a
glibc issue.
Running a (simplified) test case under strace shows that /etc/passwd
is continuously opened and closed:
…
open("/etc/passwd", O_RDONLY|O_CLOEXEC) = 3
lseek(3, 0, SEEK_CUR) = 0
read(3, "root:x:0:0:root:/root:/bin/bash\n"..., 4096) = 2717
lseek(3, 2717, SEEK_SET) = 2717
close(3) = 0
open("/etc/passwd", O_RDONLY|O_CLOEXEC) = 3
lseek(3, 0, SEEK_CUR) = 0
lseek(3, 0, SEEK_SET) = 0
read(3, "root:x:0:0:root:/root:/bin/bash\n"..., 4096) = 2717
lseek(3, 2717, SEEK_SET) = 2717
close(3) = 0
open("/etc/passwd", O_RDONLY|O_CLOEXEC) = 3
lseek(3, 0, SEEK_CUR) = 0
…
The lookup function implementation in
nss/nss_files/files-XXX.c:DB_LOOKUP has code to prevent that. It is
supposed skip closing the input file if it was already open.
/* Reset file pointer to beginning or open file. */ \
status = internal_setent (keep_stream); \
\
if (status == NSS_STATUS_SUCCESS) \
{ \
/* Tell getent function that we have repositioned the file pointer. */ \
last_use = getby; \
\
while ((status = internal_getent (result, buffer, buflen, errnop \
H_ERRNO_ARG EXTRA_ARGS_VALUE)) \
== NSS_STATUS_SUCCESS) \
{ break_if_match } \
\
if (! keep_stream) \
internal_endent (); \
} \
keep_stream is initialized from the stayopen flag in internal_setent.
internal_setent is called from the set*ent implementation as:
status = internal_setent (stayopen);
However, for non-host database, this flag is always 0, per the
STAYOPEN magic in nss/getXXent_r.c.
Thus, the fix is this:
- status = internal_setent (stayopen);
+ status = internal_setent (1);
This is not a behavioral change even for the hosts database (where the
application can specify the stayopen flag) because with a call to
sethostent(0), the file handle is still not closed in the
implementation of gethostent.
Diffstat (limited to 'nss')
-rw-r--r-- | nss/Makefile | 2 | ||||
-rw-r--r-- | nss/nss_files/files-XXX.c | 2 | ||||
-rw-r--r-- | nss/tst-nss-getpwent.c | 118 |
3 files changed, 120 insertions, 2 deletions
diff --git a/nss/Makefile b/nss/Makefile index d75dad2ee6..65ab7b5419 100644 --- a/nss/Makefile +++ b/nss/Makefile @@ -47,7 +47,7 @@ install-bin := getent makedb makedb-modules = xmalloc hash-string extra-objs += $(makedb-modules:=.o) -tests = test-netdb tst-nss-test1 test-digits-dots +tests = test-netdb tst-nss-test1 test-digits-dots tst-nss-getpwent xtests = bug-erange # Specify rules for the nss_* modules. We have some services. diff --git a/nss/nss_files/files-XXX.c b/nss/nss_files/files-XXX.c index a7a45e5b0e..a7ce5ea97e 100644 --- a/nss/nss_files/files-XXX.c +++ b/nss/nss_files/files-XXX.c @@ -134,7 +134,7 @@ CONCAT(_nss_files_set,ENTNAME) (int stayopen) __libc_lock_lock (lock); - status = internal_setent (stayopen); + status = internal_setent (1); if (status == NSS_STATUS_SUCCESS && fgetpos (stream, &position) < 0) { diff --git a/nss/tst-nss-getpwent.c b/nss/tst-nss-getpwent.c new file mode 100644 index 0000000000..f2e8abce60 --- /dev/null +++ b/nss/tst-nss-getpwent.c @@ -0,0 +1,118 @@ +/* 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 <pwd.h> +#include <stdbool.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +int +do_test (void) +{ + /* Count the number of entries in the password database, and fetch + data from the first and last entries. */ + size_t count = 0; + struct passwd * pw; + char *first_name = NULL; + uid_t first_uid = 0; + char *last_name = NULL; + uid_t last_uid = 0; + setpwent (); + while ((pw = getpwent ()) != NULL) + { + if (first_name == NULL) + { + first_name = strdup (pw->pw_name); + if (first_name == NULL) + { + printf ("strdup: %m\n"); + return 1; + } + first_uid = pw->pw_uid; + } + + free (last_name); + last_name = strdup (pw->pw_name); + if (last_name == NULL) + { + printf ("strdup: %m\n"); + return 1; + } + last_uid = pw->pw_uid; + ++count; + } + endpwent (); + + if (count == 0) + { + printf ("No entries in the password database.\n"); + return 0; + } + + /* Try again, this time interleaving with name-based and UID-based + lookup operations. The counts do not match if the interleaved + lookups affected the enumeration. */ + size_t new_count = 0; + setpwent (); + while ((pw = getpwent ()) != NULL) + { + if (new_count == count) + { + printf ("Additional entry in the password database.\n"); + return 1; + } + ++new_count; + struct passwd *pw2 = getpwnam (first_name); + if (pw2 == NULL) + { + printf ("getpwnam (%s) failed: %m\n", first_name); + return 1; + } + pw2 = getpwnam (last_name); + if (pw2 == NULL) + { + printf ("getpwnam (%s) failed: %m\n", last_name); + return 1; + } + pw2 = getpwuid (first_uid); + if (pw2 == NULL) + { + printf ("getpwuid (%llu) failed: %m\n", + (unsigned long long) first_uid); + return 1; + } + pw2 = getpwuid (last_uid); + if (pw2 == NULL) + { + printf ("getpwuid (%llu) failed: %m\n", + (unsigned long long) last_uid); + return 1; + } + } + endpwent (); + if (new_count < count) + { + printf ("Missing entry in the password database.\n"); + return 1; + } + + return 0; +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" |