From 03d2730b44cc2236318fd978afa2651753666c55 Mon Sep 17 00:00:00 2001 From: Florian Weimer <fweimer@redhat.com> Date: Wed, 29 Apr 2015 14:41:25 +0200 Subject: [PATCH] CVE-2014-8121: Do not close NSS files database during iteration [BZ #18007] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- ChangeLog | 8 ++++ NEWS | 12 +++-- nss/Makefile | 2 +- nss/nss_files/files-XXX.c | 2 +- nss/tst-nss-getpwent.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 136 insertions(+), 6 deletions(-) create mode 100644 nss/tst-nss-getpwent.c diff -Nurp glibc-2.20.orig/ChangeLog glibc-2.20/ChangeLog --- glibc-2.20.orig/ChangeLog 2015-05-02 12:39:10.880876317 +0300 +++ glibc-2.20/ChangeLog 2015-05-02 12:41:04.868052457 +0300 @@ -1,3 +1,11 @@ +2015-04-29 Florian Weimer <fweimer@redhat.com> + + [BZ #18007] + * nss/nss_files/files-XXX.c (CONCAT): Always enable stayopen. + (CVE-2014-8121) + * nss/tst-nss-getpwent.c: New file. + * nss/Makefile (tests): Add new test. + 2015-04-21 Arjun Shankar <arjun.is@lostca.se> [BZ #18287] diff -Nurp glibc-2.20.orig/NEWS glibc-2.20/NEWS --- glibc-2.20.orig/NEWS 2015-05-02 12:39:10.880876317 +0300 +++ glibc-2.20/NEWS 2015-05-02 12:47:13.774391077 +0300 @@ -10,7 +10,7 @@ Version 2.20.1 * The following bugs are resolved with this release: 16009, 16617, 16618, 17266, 17269, 17370, 17371, 17460, 17485, 17555, - 17625, 17630, 17801, 18032, 18287. + 17625, 17630, 17801, 18007, 18032, 18287. * A buffer overflow in gethostbyname_r and related functions performing DNS requests has been fixed. If the NSS functions were called with a @@ -24,6 +24,11 @@ Version 2.20.1 buffer. The implementation now correctly computes the required buffer size when using malloc. +* CVE-2014-8121 The NSS files backend would reset the file pointer used by + the get*ent functions if any of the query functions for the same database + are used during the iteration, causing a denial-of-service condition in + some applications. + * CVE-2014-7817 The wordexp function could ignore the WRDE_NOCMD flag under certain input conditions resulting in the execution of a shell for command substitution when the applicaiton did not request it. The diff -Nurp glibc-2.20.orig/nss/Makefile glibc-2.20/nss/Makefile --- glibc-2.20.orig/nss/Makefile 2014-09-07 11:09:09.000000000 +0300 +++ glibc-2.20/nss/Makefile 2015-05-02 12:39:28.380749143 +0300 @@ -39,7 +39,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 -Nurp glibc-2.20.orig/nss/nss_files/files-XXX.c glibc-2.20/nss/nss_files/files-XXX.c --- glibc-2.20.orig/nss/nss_files/files-XXX.c 2014-09-07 11:09:09.000000000 +0300 +++ glibc-2.20/nss/nss_files/files-XXX.c 2015-05-02 12:39:28.380749143 +0300 @@ -134,7 +134,7 @@ CONCAT(_nss_files_set,ENTNAME) (int stay __libc_lock_lock (lock); - status = internal_setent (stayopen); + status = internal_setent (1); if (status == NSS_STATUS_SUCCESS && fgetpos (stream, &position) < 0) { diff -Nurp glibc-2.20.orig/nss/tst-nss-getpwent.c glibc-2.20/nss/tst-nss-getpwent.c --- glibc-2.20.orig/nss/tst-nss-getpwent.c 1970-01-01 02:00:00.000000000 +0200 +++ glibc-2.20/nss/tst-nss-getpwent.c 2015-05-02 12:39:28.380749143 +0300 @@ -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"