Skip to content

Commit

Permalink
Separate internal state between getXXent and getXXbyYY NSS calls (bug…
Browse files Browse the repository at this point in the history 
… 18007)
  • Loading branch information
Andreas Schwab committed May 11, 2015
1 parent e1b6cb0 commit b13b96c
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 185 deletions.
38 changes: 38 additions & 0 deletions ChangeLog
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,41 @@
2015-05-11 Andreas Schwab <schwab@suse.de>

[BZ #18007]
* nis/nss_compat/compat-grp.c (internal_endgrent): Don't call
nss_endgrent.
(_nss_compat_endgrent): Call nss_endgrent.
* nis/nss_compat/compat-pwd.c (internal_endpwent): Don't call
nss_endpwent.
(_nss_compat_endpwent): Call nss_endpwent.
* nis/nss_compat/compat-spwd.c (internal_setspent): Add parameter
needent, call nss_setspent only if non-zero.
(_nss_compat_setspent, _nss_compat_getspent_r): Pass non-zero.
(internal_endspent): Don't call nss_endspent.
(_nss_compat_endspent): Call nss_endspent.
* nss/nss_files/files-XXX.c (position, last_use, keep_stream):
Remove. All uses removed.
(internal_setent): Remove parameter stayopen, add parameter
stream. Use it instead of global variable.
(CONCAT(_nss_files_set,ENTNAME)): Pass global stream.
(internal_endent, internal_getent): Add parameter stream. Use it
instead of global variable.
(CONCAT(_nss_files_end,ENTNAME))
(CONCAT(_nss_files_get,ENTNAME_r)): Pass global stream.
(_nss_files_get##name##_r): Pass local stream. Remove locking.
* nss/nss_files/files-alias.c (position, last_use): Remove. All
uses removed.
(internal_setent, internal_endent): Add parameter stream. Use it
instead of global variable.
(_nss_files_setaliasent, _nss_files_endaliasent): Pass global
stream.
(get_next_alias): Add parameter stream.
(_nss_files_getaliasent_r): Pass global stream.
(_nss_files_getaliasbyname_r): Pass local stream. Remove locking.
* nss/nss_files/files-hosts.c (_nss_files_gethostbyname3_r)
(_nss_files_gethostbyname4_r): Pass local stream to
internal_setent, internal_getent and internal_endent. Remove
locking.

2015-05-11 Stefan Liebler <stli@linux.vnet.ibm.com>

* tst-strfmon1.c (tests): Update expected currency symbol.
Expand Down
7 changes: 3 additions & 4 deletions NEWS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,9 @@ Version 2.22
Hat). These updates cause user visible changes, such as the fix for bug
17998.

* 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-8121 The NSS backends shared internal state between the getXXent
and getXXbyYY NSS calls for the same database, causing a denial-of-service
condition in some applications.

Version 2.21

Expand Down
6 changes: 3 additions & 3 deletions nis/nss_compat/compat-grp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,6 @@ _nss_compat_setgrent (int stayopen)
static enum nss_status
internal_endgrent (ent_t *ent)
{
if (nss_endgrent)
nss_endgrent ();

if (ent->stream != NULL)
{
fclose (ent->stream);
Expand All @@ -222,6 +219,9 @@ _nss_compat_endgrent (void)

__libc_lock_lock (lock);

if (nss_endgrent)
nss_endgrent ();

result = internal_endgrent (&ext_ent);

__libc_lock_unlock (lock);
Expand Down
6 changes: 3 additions & 3 deletions nis/nss_compat/compat-pwd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,6 @@ _nss_compat_setpwent (int stayopen)
static enum nss_status
internal_endpwent (ent_t *ent)
{
if (nss_endpwent)
nss_endpwent ();

if (ent->stream != NULL)
{
fclose (ent->stream);
Expand Down Expand Up @@ -346,6 +343,9 @@ _nss_compat_endpwent (void)

__libc_lock_lock (lock);

if (nss_endpwent)
nss_endpwent ();

result = internal_endpwent (&ext_ent);

__libc_lock_unlock (lock);
Expand Down
16 changes: 8 additions & 8 deletions nis/nss_compat/compat-spwd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ copy_spwd_changes (struct spwd *dest, struct spwd *src,
}

static enum nss_status
internal_setspent (ent_t *ent, int stayopen)
internal_setspent (ent_t *ent, int stayopen, int needent)
{
enum nss_status status = NSS_STATUS_SUCCESS;

Expand Down Expand Up @@ -239,7 +239,7 @@ internal_setspent (ent_t *ent, int stayopen)

give_spwd_free (&ent->pwd);

if (status == NSS_STATUS_SUCCESS && nss_setspent)
if (needent && status == NSS_STATUS_SUCCESS && nss_setspent)
ent->setent_status = nss_setspent (stayopen);

return status;
Expand All @@ -256,7 +256,7 @@ _nss_compat_setspent (int stayopen)
if (ni == NULL)
init_nss_interface ();

result = internal_setspent (&ext_ent, stayopen);
result = internal_setspent (&ext_ent, stayopen, 1);

__libc_lock_unlock (lock);

Expand All @@ -267,9 +267,6 @@ _nss_compat_setspent (int stayopen)
static enum nss_status
internal_endspent (ent_t *ent)
{
if (nss_endspent)
nss_endspent ();

if (ent->stream != NULL)
{
fclose (ent->stream);
Expand Down Expand Up @@ -303,6 +300,9 @@ _nss_compat_endspent (void)

__libc_lock_lock (lock);

if (nss_endspent)
nss_endspent ();

result = internal_endspent (&ext_ent);

__libc_lock_unlock (lock);
Expand Down Expand Up @@ -658,7 +658,7 @@ _nss_compat_getspent_r (struct spwd *pwd, char *buffer, size_t buflen,
init_nss_interface ();

if (ext_ent.stream == NULL)
result = internal_setspent (&ext_ent, 1);
result = internal_setspent (&ext_ent, 1, 1);

if (result == NSS_STATUS_SUCCESS)
result = internal_getspent_r (pwd, &ext_ent, buffer, buflen, errnop);
Expand Down Expand Up @@ -830,7 +830,7 @@ _nss_compat_getspnam_r (const char *name, struct spwd *pwd,

__libc_lock_unlock (lock);

result = internal_setspent (&ent, 0);
result = internal_setspent (&ent, 0, 0);

if (result == NSS_STATUS_SUCCESS)
result = internal_getspnam_r (name, pwd, &ent, buffer, buflen, errnop);
Expand Down
109 changes: 27 additions & 82 deletions nss/nss_files/files-XXX.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,24 +60,23 @@
/* Locks the static variables in this file. */
__libc_lock_define_initialized (static, lock)

/* Maintenance of the shared stream open on the database file. */
/* Maintenance of the stream open on the database file. For getXXent
operations the stream needs to be held open across calls, the other
getXXbyYY operations all use their own stream. */

static FILE *stream;
static fpos_t position;
static enum { nouse, getent, getby } last_use;
static int keep_stream;

/* Open database file if not already opened. */
static enum nss_status
internal_setent (int stayopen)
internal_setent (FILE **stream)
{
enum nss_status status = NSS_STATUS_SUCCESS;

if (stream == NULL)
if (*stream == NULL)
{
stream = fopen (DATAFILE, "rce");
*stream = fopen (DATAFILE, "rce");

if (stream == NULL)
if (*stream == NULL)
status = errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
else
{
Expand All @@ -90,7 +89,7 @@ internal_setent (int stayopen)
int result;
int flags;

result = flags = fcntl (fileno (stream), F_GETFD, 0);
result = flags = fcntl (fileno (*stream), F_GETFD, 0);
if (result >= 0)
{
# ifdef O_CLOEXEC
Expand All @@ -100,27 +99,23 @@ internal_setent (int stayopen)
# endif
{
flags |= FD_CLOEXEC;
result = fcntl (fileno (stream), F_SETFD, flags);
result = fcntl (fileno (*stream), F_SETFD, flags);
}
}
if (result < 0)
{
/* Something went wrong. Close the stream and return a
failure. */
fclose (stream);
stream = NULL;
fclose (*stream);
*stream = NULL;
status = NSS_STATUS_UNAVAIL;
}
}
#endif
}
}
else
rewind (stream);

/* Remember STAYOPEN flag. */
if (stream != NULL)
keep_stream |= stayopen;
rewind (*stream);

return status;
}
Expand All @@ -134,16 +129,7 @@ CONCAT(_nss_files_set,ENTNAME) (int stayopen)

__libc_lock_lock (lock);

status = internal_setent (1);

if (status == NSS_STATUS_SUCCESS && fgetpos (stream, &position) < 0)
{
fclose (stream);
stream = NULL;
status = NSS_STATUS_UNAVAIL;
}

last_use = getent;
status = internal_setent (&stream);

__libc_lock_unlock (lock);

Expand All @@ -153,12 +139,12 @@ CONCAT(_nss_files_set,ENTNAME) (int stayopen)

/* Close the database file. */
static void
internal_endent (void)
internal_endent (FILE **stream)
{
if (stream != NULL)
if (*stream != NULL)
{
fclose (stream);
stream = NULL;
fclose (*stream);
*stream = NULL;
}
}

Expand All @@ -169,10 +155,7 @@ CONCAT(_nss_files_end,ENTNAME) (void)
{
__libc_lock_lock (lock);

internal_endent ();

/* Reset STAYOPEN flag. */
keep_stream = 0;
internal_endent (&stream);

__libc_lock_unlock (lock);

Expand Down Expand Up @@ -227,7 +210,7 @@ get_contents (char *linebuf, size_t len, FILE *stream)

/* Parsing the database file into `struct STRUCTURE' data structures. */
static enum nss_status
internal_getent (struct STRUCTURE *result,
internal_getent (FILE *stream, struct STRUCTURE *result,
char *buffer, size_t buflen, int *errnop H_ERRNO_PROTO
EXTRA_ARGS_DECL)
{
Expand Down Expand Up @@ -300,45 +283,14 @@ CONCAT(_nss_files_get,ENTNAME_r) (struct STRUCTURE *result, char *buffer,
{
int save_errno = errno;

status = internal_setent (0);
status = internal_setent (&stream);

__set_errno (save_errno);

if (status == NSS_STATUS_SUCCESS && fgetpos (stream, &position) < 0)
{
fclose (stream);
stream = NULL;
status = NSS_STATUS_UNAVAIL;
}
}

if (status == NSS_STATUS_SUCCESS)
{
/* If the last use was not by the getent function we need the
position the stream. */
if (last_use != getent)
{
if (fsetpos (stream, &position) < 0)
status = NSS_STATUS_UNAVAIL;
else
last_use = getent;
}

if (status == NSS_STATUS_SUCCESS)
{
status = internal_getent (result, buffer, buflen, errnop
H_ERRNO_ARG EXTRA_ARGS_VALUE);

/* Remember this position if we were successful. If the
operation failed we give the user a chance to repeat the
operation (perhaps the buffer was too small). */
if (status == NSS_STATUS_SUCCESS)
fgetpos (stream, &position);
else
/* We must make sure we reposition the stream the next call. */
last_use = nouse;
}
}
status = internal_getent (stream, result, buffer, buflen, errnop
H_ERRNO_ARG EXTRA_ARGS_VALUE);

__libc_lock_unlock (lock);

Expand All @@ -364,27 +316,20 @@ _nss_files_get##name##_r (proto, \
size_t buflen, int *errnop H_ERRNO_PROTO) \
{ \
enum nss_status status; \
FILE *stream = NULL; \
\
__libc_lock_lock (lock); \
\
/* Reset file pointer to beginning or open file. */ \
status = internal_setent (keep_stream); \
/* Open file. */ \
status = internal_setent (&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 \
while ((status = internal_getent (stream, result, buffer, buflen, errnop \
H_ERRNO_ARG EXTRA_ARGS_VALUE)) \
== NSS_STATUS_SUCCESS) \
{ break_if_match } \
\
if (! keep_stream) \
internal_endent (); \
internal_endent (&stream); \
} \
\
__libc_lock_unlock (lock); \
\
return status; \
}
Loading

0 comments on commit b13b96c

Please sign in to comment.