CVE-2014-8121: Do not close NSS files database during iteration [BZ #18007]
commit53d405329ab189725e72b317f18cd939c6ad240a
authorFlorian Weimer <fweimer@redhat.com>
Wed, 29 Apr 2015 12:41:25 +0000 (29 14:41 +0200)
committerTulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
Tue, 26 May 2015 15:03:38 +0000 (26 12:03 -0300)
tree9470d9a517eccba2f9bbffe735c65f6896196676
parent3c7fb252298c48ef424e65fe63ea818d688f1088
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.

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