]> git.ipfire.org Git - thirdparty/glibc.git/commitdiff
CVE-2014-8121: Do not close NSS files database during iteration [BZ #18007]
authorFlorian Weimer <fweimer@redhat.com>
Wed, 29 Apr 2015 12:41:25 +0000 (14:41 +0200)
committerAurelien Jarno <aurelien@aurel32.net>
Mon, 19 Oct 2015 10:27:37 +0000 (12:27 +0200)
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.

(cherry picked from commit 03d2730b44cc2236318fd978afa2651753666c55)

Conflicts:
ChangeLog
NEWS

ChangeLog
NEWS
nss/Makefile
nss/nss_files/files-XXX.c
nss/tst-nss-getpwent.c [new file with mode: 0644]

index 8d621b240b8a600b43cb93a6212998e1a03d1444..a7207b1b65721d1499033d0ced7c18de861d8464 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -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-02-22  Paul Pluzhnikov  <ppluzhnikov@google.com>
 
        [BZ #17269]
diff --git a/NEWS b/NEWS
index 9e7316b4a77f6cc3eba0b3505c12569c083a0bd4..e00543f14f90e4fc66127eb55708ec28d8857306 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -11,7 +11,7 @@ Version 2.19.1
 
   15946, 16545, 16574, 16623, 16657, 16695, 16743, 16878, 16882, 16885,
   16916, 16932, 16943, 16958, 17048, 17069, 17079, 17137, 17153, 17213,
-  17263, 17269, 17325, 17555, 18032, 18287.
+  17263, 17269, 17325, 17555, 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
@@ -57,6 +57,11 @@ Version 2.19.1
   IBM937, IBM939, IBM1364 could result in an out-of-bounds array read,
   resulting a denial-of-service security vulnerability in applications which
   use functions related to iconv. (CVE-2014-6040)
+
+* 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.
 \f
 Version 2.19
 
index c8880c061c80f93f29192356a753209410b89caa..3f9d2d0b508f5c3fddbbbbdde657f4d98744c9b9 100644 (file)
@@ -37,7 +37,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
 
 include ../Makeconfig
index d4cd95e26cd7113566c89b0bdfcb8e14e3364e48..3b90f7e6b4a2961b03cbe570481813fad185d787 100644 (file)
@@ -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 (file)
index 0000000..f2e8abc
--- /dev/null
@@ -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"