diff options
author | Florian Weimer <fweimer@redhat.com> | 2016-05-04 12:09:35 +0200 |
---|---|---|
committer | Aurelien Jarno <aurelien@aurel32.net> | 2016-05-11 14:56:11 +0200 |
commit | 46329bea58c143887c7109926d03901c0ccf81ed (patch) | |
tree | 4077c6715f43a38b965fc5f985940ec4d6f12257 | |
parent | d14837f6edd39d5c79c4cda7f7775c8f5e0ca20a (diff) | |
download | glibc-46329bea58c143887c7109926d03901c0ccf81ed.tar.gz glibc-46329bea58c143887c7109926d03901c0ccf81ed.tar.xz glibc-46329bea58c143887c7109926d03901c0ccf81ed.zip |
CVE-2016-1234: glob: Do not copy d_name field of struct dirent [BZ #19779]
Instead, we store the data we need from the return value of readdir in an object of the new type struct readdir_result. This type is independent of the layout of struct dirent. (cherry picked from commit 5171f3079f2cc53e0548fc4967361f4d1ce9d7ea)
-rw-r--r-- | ChangeLog | 21 | ||||
-rw-r--r-- | NEWS | 7 | ||||
-rw-r--r-- | posix/bug-glob2.c | 14 | ||||
-rw-r--r-- | posix/glob.c | 223 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/i386/glob64.c | 22 |
5 files changed, 186 insertions, 101 deletions
diff --git a/ChangeLog b/ChangeLog index be69ce3dae..0471430325 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,24 @@ +2016-05-04 Florian Weimer <fweimer@redhat.com> + + [BZ #19779] + CVE-2016-1234 + Avoid copying names of directory entries. + * posix/glob.c (DIRENT_MUST_BE, DIRENT_MIGHT_BE_SYMLINK) + (DIRENT_MIGHT_BE_DIR, CONVERT_D_INO, CONVERT_D_TYPE) + (CONVERT_DIRENT_DIRENT64, REAL_DIR_ENTRY): Remove macros. + (struct readdir_result): New type. + (D_TYPE_TO_RESULT, D_INO_TO_RESULT, READDIR_RESULT_INITIALIZER) + (GL_READDIR): New macros. + (readdir_result_might_be_symlink, readdir_result_might_be_dir) + (convert_dirent, convert_dirent64): New functions. + (glob_in_dir): Use struct readdir_result. Call convert_dirent or + convert_dirent64. Adjust references to the readdir result. + * sysdeps/unix/sysv/linux/i386/glob64.c: + (convert_dirent, GL_READDIR): Redefine for second file inclusion. + * posix/bug-glob2.c (LONG_NAME): Define. + (filesystem): Add LONG_NAME. + (my_DIR): Increase the size of room_for_dirent. + 2016-04-29 Florian Weimer <fweimer@redhat.com> glob: Simplify and document the interface for the GLOB_ALTDIRFUNC diff --git a/NEWS b/NEWS index 7b13178aa8..fe7ce0f213 100644 --- a/NEWS +++ b/NEWS @@ -25,7 +25,8 @@ Version 2.22.1 17905, 18420, 18421, 18480, 18589, 18743, 18778, 18781, 18787, 18796, 18870, 18887, 18921, 18928, 18969, 18985, 19003, 19018, 19048, 19058, - 19174, 19178, 19182, 19243, 19590, 19682, 19791, 19822, 19853, 19879. + 19174, 19178, 19182, 19243, 19590, 19682, 19791, 19822, 19853, 19879, + 19779. * The getnetbyname implementation in nss_dns had a potentially unbounded alloca call (in the form of a call to strdupa), leading to a stack @@ -43,6 +44,10 @@ Version 2.22.1 trigger this bug. Affected applications typically create create and destroy threads frequently. (Bug 19048 was reported and analyzed by Ericsson.) + +* The glob function suffered from a stack-based buffer overflow when it was + called with the GLOB_ALTDIRFUNC flag and encountered a long file name. + Reported by Alexander Cherepanov. (CVE-2016-1234) Version 2.22 diff --git a/posix/bug-glob2.c b/posix/bug-glob2.c index ac8d79a3a3..e2964b2ad6 100644 --- a/posix/bug-glob2.c +++ b/posix/bug-glob2.c @@ -40,6 +40,17 @@ # define PRINTF(fmt, args...) #endif +#define LONG_NAME \ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \ + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" static struct { @@ -58,6 +69,7 @@ static struct { ".", 3, DT_DIR, 0755 }, { "..", 3, DT_DIR, 0755 }, { "a", 3, DT_REG, 0644 }, + { LONG_NAME, 3, DT_REG, 0644 }, { "unreadable", 2, DT_DIR, 0111 }, { ".", 3, DT_DIR, 0111 }, { "..", 3, DT_DIR, 0755 }, @@ -75,7 +87,7 @@ typedef struct int level; int idx; struct dirent d; - char room_for_dirent[NAME_MAX]; + char room_for_dirent[sizeof (LONG_NAME)]; } my_DIR; diff --git a/posix/glob.c b/posix/glob.c index 56aa910ff2..60fa6c5bfd 100644 --- a/posix/glob.c +++ b/posix/glob.c @@ -24,7 +24,9 @@ #include <errno.h> #include <sys/types.h> #include <sys/stat.h> +#include <stdbool.h> #include <stddef.h> +#include <stdint.h> /* Outcomment the following line for production quality code. */ /* #define NDEBUG 1 */ @@ -73,69 +75,8 @@ # endif /* HAVE_VMSDIR_H */ #endif - -/* When used in the GNU libc the symbol _DIRENT_HAVE_D_TYPE is available - if the `d_type' member for `struct dirent' is available. - HAVE_STRUCT_DIRENT_D_TYPE plays the same role in GNULIB. */ -#if defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE -/* True if the directory entry D must be of type T. */ -# define DIRENT_MUST_BE(d, t) ((d)->d_type == (t)) - -/* True if the directory entry D might be a symbolic link. */ -# define DIRENT_MIGHT_BE_SYMLINK(d) \ - ((d)->d_type == DT_UNKNOWN || (d)->d_type == DT_LNK) - -/* True if the directory entry D might be a directory. */ -# define DIRENT_MIGHT_BE_DIR(d) \ - ((d)->d_type == DT_DIR || DIRENT_MIGHT_BE_SYMLINK (d)) - -#else /* !HAVE_D_TYPE */ -# define DIRENT_MUST_BE(d, t) false -# define DIRENT_MIGHT_BE_SYMLINK(d) true -# define DIRENT_MIGHT_BE_DIR(d) true -#endif /* HAVE_D_TYPE */ - -/* If the system has the `struct dirent64' type we use it internally. */ -#if defined _LIBC && !defined COMPILE_GLOB64 - -# if (defined POSIX || defined WINDOWS32) && !defined __GNU_LIBRARY__ -# define CONVERT_D_INO(d64, d32) -# else -# define CONVERT_D_INO(d64, d32) \ - (d64)->d_ino = (d32)->d_ino; -# endif - -# ifdef _DIRENT_HAVE_D_TYPE -# define CONVERT_D_TYPE(d64, d32) \ - (d64)->d_type = (d32)->d_type; -# else -# define CONVERT_D_TYPE(d64, d32) -# endif - -# define CONVERT_DIRENT_DIRENT64(d64, d32) \ - strcpy ((d64)->d_name, (d32)->d_name); \ - CONVERT_D_INO (d64, d32) \ - CONVERT_D_TYPE (d64, d32) -#endif - - -#if (defined POSIX || defined WINDOWS32) && !defined __GNU_LIBRARY__ -/* Posix does not require that the d_ino field be present, and some - systems do not provide it. */ -# define REAL_DIR_ENTRY(dp) 1 -#else -# define REAL_DIR_ENTRY(dp) (dp->d_ino != 0) -#endif /* POSIX */ - #include <stdlib.h> #include <string.h> - -/* NAME_MAX is usually defined in <dirent.h> or <limits.h>. */ -#include <limits.h> -#ifndef NAME_MAX -# define NAME_MAX (sizeof (((struct dirent *) 0)->d_name)) -#endif - #include <alloca.h> #ifdef _LIBC @@ -180,8 +121,111 @@ static const char *next_brace_sub (const char *begin, int flags) __THROWNL; +/* A representation of a directory entry which does not depend on the + layout of struct dirent, or the size of ino_t. */ +struct readdir_result +{ + const char *name; +# if defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE + uint8_t type; +# endif + bool skip_entry; +}; + +# if defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE +/* Initializer based on the d_type member of struct dirent. */ +# define D_TYPE_TO_RESULT(source) (source)->d_type, + +/* True if the directory entry D might be a symbolic link. */ +static bool +readdir_result_might_be_symlink (struct readdir_result d) +{ + return d.type == DT_UNKNOWN || d.type == DT_LNK; +} + +/* True if the directory entry D might be a directory. */ +static bool +readdir_result_might_be_dir (struct readdir_result d) +{ + return d.type == DT_DIR || readdir_result_might_be_symlink (d); +} +# else /* defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE */ +# define D_TYPE_TO_RESULT(source) + +/* If we do not have type information, symbolic links and directories + are always a possibility. */ + +static bool +readdir_result_might_be_symlink (struct readdir_result d) +{ + return true; +} + +static bool +readdir_result_might_be_dir (struct readdir_result d) +{ + return true; +} + +# endif /* defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE */ + +# if (defined POSIX || defined WINDOWS32) && !defined __GNU_LIBRARY__ +/* Initializer for skip_entry. POSIX does not require that the d_ino + field be present, and some systems do not provide it. */ +# define D_INO_TO_RESULT(source) false, +# else +# define D_INO_TO_RESULT(source) (source)->d_ino == 0, +# endif + +/* Construct an initializer for a struct readdir_result object from a + struct dirent *. No copy of the name is made. */ +#define READDIR_RESULT_INITIALIZER(source) \ + { \ + source->d_name, \ + D_TYPE_TO_RESULT (source) \ + D_INO_TO_RESULT (source) \ + } + #endif /* !defined _LIBC || !defined GLOB_ONLY_P */ +/* Call gl_readdir on STREAM. This macro can be overridden to reduce + type safety if an old interface version needs to be supported. */ +#ifndef GL_READDIR +# define GL_READDIR(pglob, stream) ((pglob)->gl_readdir (stream)) +#endif + +/* Extract name and type from directory entry. No copy of the name is + made. If SOURCE is NULL, result name is NULL. Keep in sync with + convert_dirent64 below. */ +static struct readdir_result +convert_dirent (const struct dirent *source) +{ + if (source == NULL) + { + struct readdir_result result = { NULL, }; + return result; + } + struct readdir_result result = READDIR_RESULT_INITIALIZER (source); + return result; +} + +#ifndef COMPILE_GLOB64 +/* Like convert_dirent, but works on struct dirent64 instead. Keep in + sync with convert_dirent above. */ +static struct readdir_result +convert_dirent64 (const struct dirent64 *source) +{ + if (source == NULL) + { + struct readdir_result result = { NULL, }; + return result; + } + struct readdir_result result = READDIR_RESULT_INITIALIZER (source); + return result; +} +#endif + + #ifndef attribute_hidden # define attribute_hidden #endif @@ -1546,55 +1590,36 @@ glob_in_dir (const char *pattern, const char *directory, int flags, while (1) { - const char *name; -#if defined _LIBC && !defined COMPILE_GLOB64 - struct dirent64 *d; - union - { - struct dirent64 d64; - char room [offsetof (struct dirent64, d_name[0]) - + NAME_MAX + 1]; - } - d64buf; - - if (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)) - { - struct dirent *d32 = (*pglob->gl_readdir) (stream); - if (d32 != NULL) - { - CONVERT_DIRENT_DIRENT64 (&d64buf.d64, d32); - d = &d64buf.d64; - } - else - d = NULL; - } - else - d = __readdir64 (stream); + struct readdir_result d; + { + if (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)) + d = convert_dirent (GL_READDIR (pglob, stream)); + else + { +#ifdef COMPILE_GLOB64 + d = convert_dirent (__readdir (stream)); #else - struct dirent *d = (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0) - ? ((struct dirent *) - (*pglob->gl_readdir) (stream)) - : __readdir (stream)); + d = convert_dirent64 (__readdir64 (stream)); #endif - if (d == NULL) + } + } + if (d.name == NULL) break; - if (! REAL_DIR_ENTRY (d)) + if (d.skip_entry) continue; /* If we shall match only directories use the information provided by the dirent call if possible. */ - if ((flags & GLOB_ONLYDIR) && !DIRENT_MIGHT_BE_DIR (d)) + if ((flags & GLOB_ONLYDIR) && !readdir_result_might_be_dir (d)) continue; - name = d->d_name; - - if (fnmatch (pattern, name, fnm_flags) == 0) + if (fnmatch (pattern, d.name, fnm_flags) == 0) { /* If the file we found is a symlink we have to make sure the target file exists. */ - if (!DIRENT_MIGHT_BE_SYMLINK (d) - || link_exists_p (dfd, directory, dirlen, name, pglob, - flags)) + if (!readdir_result_might_be_symlink (d) + || link_exists_p (dfd, directory, dirlen, d.name, + pglob, flags)) { if (cur == names->count) { @@ -1614,7 +1639,7 @@ glob_in_dir (const char *pattern, const char *directory, int flags, names = newnames; cur = 0; } - names->name[cur] = strdup (d->d_name); + names->name[cur] = strdup (d.name); if (names->name[cur] == NULL) goto memory_error; ++cur; diff --git a/sysdeps/unix/sysv/linux/i386/glob64.c b/sysdeps/unix/sysv/linux/i386/glob64.c index b4fcd1a73c..802c957d6c 100644 --- a/sysdeps/unix/sysv/linux/i386/glob64.c +++ b/sysdeps/unix/sysv/linux/i386/glob64.c @@ -1,3 +1,21 @@ +/* Two glob variants with 64-bit support, for dirent64 and __olddirent64. + Copyright (C) 1998-2016 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 <dirent.h> #include <glob.h> #include <sys/stat.h> @@ -38,11 +56,15 @@ int __old_glob64 (const char *__pattern, int __flags, #undef dirent #define dirent __old_dirent64 +#undef GL_READDIR +# define GL_READDIR(pglob, stream) \ + ((struct __old_dirent64 *) (pglob)->gl_readdir (stream)) #undef __readdir #define __readdir(dirp) __old_readdir64 (dirp) #undef glob #define glob(pattern, flags, errfunc, pglob) \ __old_glob64 (pattern, flags, errfunc, pglob) +#define convert_dirent __old_convert_dirent #define glob_in_dir __old_glob_in_dir #define GLOB_ATTRIBUTE attribute_compat_text_section |