[PATCH] CVE-2014-8121: Fix nss_files file management [BZ#18007]

[Florian Weimer]

On 03/25/2015 04:40 PM, Andreas Schwab wrote:
> You need to take a global view at the bug.  The sharing of global state
> between getXXent and getXXbyYY is broken by design.

I don't feel qualified to review this.   It looks like reasonable change.

It does fix bug 18007 (and more).  It does not change the fopen/fclose
behavior of getent in my simple tests (for nss_files); the file is not
kept open before and after the patch.

> diff --git a/nis/nss_compat/compat-grp.c b/nis/nss_compat/compat-grp.c
> index 5e9c527..987d914 100644
> --- a/nis/nss_compat/compat-grp.c
> +++ b/nis/nss_compat/compat-grp.c

>  static enum nss_status
> -internal_endgrent (ent_t *ent)
> +internal_endgrent (ent_t *ent, int needent)
>  {
> -  if (nss_endgrent)
> +  if (needent && nss_endgrent)
>      nss_endgrent ();

I think the nss_endgrent call should be moved into _nss_compat_endgrent,
where it belongs.  Then the needent argument is not necessary.  This
applies to the other files as well.

> diff --git a/nss/nss_files/files-XXX.c b/nss/nss_files/files-XXX.c
> index a7a45e5..aabd4ea 100644
> --- a/nss/nss_files/files-XXX.c
> +++ b/nss/nss_files/files-XXX.c
> @@ -63,21 +63,18 @@ __libc_lock_define_initialized (static, lock)
>  /* Maintenance of the shared stream open on the database file.  */

This comment is now outdated, and

  /* Locks the static variables in this file.  */

should be updated as well (the lock only covers the “file” variable).

Similarly for the parallel places in the other files.

-- 
Florian Weimer / Red Hat Product Security