about summary refs log tree commit diff
path: root/nss/nss_files
diff options
context:
space:
mode:
authorFlorian Weimer <fweimer@redhat.com>2015-04-29 14:41:25 +0200
committerFlorian Weimer <fweimer@redhat.com>2015-04-29 14:41:26 +0200
commit03d2730b44cc2236318fd978afa2651753666c55 (patch)
tree8846df11bd7a2d2b8bd0be49dbf9d69654240e05 /nss/nss_files
parent7d0b2575416aec2717e8665287d0ab77826a0ade (diff)
downloadglibc-03d2730b44cc2236318fd978afa2651753666c55.tar.gz
glibc-03d2730b44cc2236318fd978afa2651753666c55.tar.xz
glibc-03d2730b44cc2236318fd978afa2651753666c55.zip
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/nss_files')
-rw-r--r--nss/nss_files/files-XXX.c2
1 files changed, 1 insertions, 1 deletions
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)
     {