* [PATCH v2 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 @ 2022-01-19 8:21 Siddhesh Poyarekar 2022-01-19 8:21 ` [PATCH v2 1/3] support: Add helpers to create paths longer than PATH_MAX Siddhesh Poyarekar ` (3 more replies) 0 siblings, 4 replies; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-19 8:21 UTC (permalink / raw) To: libc-alpha Add functions to make directory trees with paths longer than PATH_MAX and use them to test fixes for CVE-2021-3998 and CVE-2021-3999. Tested on x86_64 and i686. Changes from v1: - Try reducing directory name size to meet lower limits of some fuse filesystems - Fixed review comments - Credited Qualys in NEWS - Use x* functions wherever possible - Drop size check in linux getcwd implementation and rely only on the posix one to flag the error - Fix formatting issues I had missed before. Siddhesh Poyarekar (3): support: Add helpers to create paths longer than PATH_MAX realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) NEWS | 10 + stdlib/Makefile | 1 + stdlib/canonicalize.c | 12 +- stdlib/tst-realpath-toolong.c | 49 ++++ support/temp_file.c | 173 ++++++++++++- support/temp_file.h | 9 + sysdeps/posix/getcwd.c | 7 + sysdeps/unix/sysv/linux/Makefile | 7 +- .../unix/sysv/linux/tst-getcwd-smallbuff.c | 245 ++++++++++++++++++ 9 files changed, 501 insertions(+), 12 deletions(-) create mode 100644 stdlib/tst-realpath-toolong.c create mode 100644 sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2 1/3] support: Add helpers to create paths longer than PATH_MAX 2022-01-19 8:21 [PATCH v2 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 Siddhesh Poyarekar @ 2022-01-19 8:21 ` Siddhesh Poyarekar 2022-01-19 10:13 ` Siddhesh Poyarekar 2022-01-19 8:21 ` [PATCH v2 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX (CVE-2021-3998) Siddhesh Poyarekar ` (2 subsequent siblings) 3 siblings, 1 reply; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-19 8:21 UTC (permalink / raw) To: libc-alpha Add new helpers support_create_and_chdir_toolong_temp_directory and support_chdir_toolong_temp_directory to create and descend into directory trees longer than PATH_MAX. Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- support/temp_file.c | 173 +++++++++++++++++++++++++++++++++++++++++--- support/temp_file.h | 9 +++ 2 files changed, 173 insertions(+), 9 deletions(-) diff --git a/support/temp_file.c b/support/temp_file.c index e7bb8aadb9..59060e573f 100644 --- a/support/temp_file.c +++ b/support/temp_file.c @@ -1,5 +1,6 @@ /* Temporary file handling for tests. Copyright (C) 1998-2022 Free Software Foundation, Inc. + Copyright The GNU Tools Authors. This file is part of the GNU C Library. The GNU C Library is free software; you can redistribute it and/or @@ -20,15 +21,17 @@ some 32-bit platforms. */ #define _FILE_OFFSET_BITS 64 +#include <support/check.h> #include <support/temp_file.h> #include <support/temp_file-internal.h> #include <support/support.h> +#include <errno.h> #include <paths.h> #include <stdio.h> #include <stdlib.h> #include <string.h> -#include <unistd.h> +#include <xunistd.h> /* List of temporary files. */ static struct temp_name_list @@ -36,14 +39,32 @@ static struct temp_name_list struct temp_name_list *next; char *name; pid_t owner; + bool toolong; } *temp_name_list; /* Location of the temporary files. Set by the test skeleton via support_set_test_dir. The string is not be freed. */ static const char *test_dir = _PATH_TMP; -void -add_temp_file (const char *name) +/* Name of subdirectories in a too long temporary directory tree. */ +static char *toolong_subdir; +static bool toolong_subdir_initialized; + +/* Return the maximum size of path on the target. */ +static inline size_t +get_path_max (void) +{ +#ifdef PATH_MAX + return PATH_MAX; +#else + size_t path_max = pathconf ("/", _PC_PATH_MAX); + return (path_max < 0 ? 1024 + : path_max <= PTRDIFF_MAX ? path_max : PTRDIFF_MAX); +#endif +} + +static void +add_temp_file_internal (const char *name, bool toolong) { struct temp_name_list *newp = (struct temp_name_list *) xcalloc (sizeof (*newp), 1); @@ -53,12 +74,19 @@ add_temp_file (const char *name) newp->name = newname; newp->next = temp_name_list; newp->owner = getpid (); + newp->toolong = toolong; temp_name_list = newp; } else free (newp); } +void +add_temp_file (const char *name) +{ + add_temp_file_internal (name, false); +} + int create_temp_file_in_dir (const char *base, const char *dir, char **filename) { @@ -90,8 +118,8 @@ create_temp_file (const char *base, char **filename) return create_temp_file_in_dir (base, test_dir, filename); } -char * -support_create_temp_directory (const char *base) +static char * +create_temp_directory_internal (const char *base, bool toolong) { char *path = xasprintf ("%s/%sXXXXXX", test_dir, base); if (mkdtemp (path) == NULL) @@ -99,16 +127,132 @@ support_create_temp_directory (const char *base) printf ("error: mkdtemp (\"%s\"): %m", path); exit (1); } - add_temp_file (path); + add_temp_file_internal (path, toolong); return path; } -/* Helper functions called by the test skeleton follow. */ +char * +support_create_temp_directory (const char *base) +{ + return create_temp_directory_internal (base, false); +} + +static void +ensure_toolong_subdir_initialized (void) +{ + if (!toolong_subdir_initialized) + FAIL_EXIT1 ("uninitialized toolong directory tree\n"); +} + +static void +initialize_toolong_subdir (void) +{ + size_t sz = NAME_MAX; + char testname[NAME_MAX]; + int ret; + + if (toolong_subdir_initialized) + return; + + memset (testname, 'X', sz - 1); + + /* Explore the name limit on the file system. This should typically be + NAME_MAX, but it could be less on some fuse filesystems. */ + do + { + struct stat statbuf; + + testname[sz - 1] = '\0'; + ret = stat (testname, &statbuf); + } + while (ret != 0 && errno == ENAMETOOLONG && (sz = sz / 2) > 0); + + if (ret != 0 && errno == ENAMETOOLONG) + FAIL_EXIT1 ("stat (%s) failed with ENAMETOOLONG\n", testname); + + toolong_subdir = xmalloc (sz); + strcpy (toolong_subdir, testname); + toolong_subdir_initialized = true; +} + +char * +support_create_and_chdir_toolong_temp_directory (const char *basename) +{ + size_t path_max = get_path_max (); + + char *base = create_temp_directory_internal (basename, true); + + xchdir (base); + + initialize_toolong_subdir (); + + size_t sz = strlen (toolong_subdir); + + /* Create directories and descend into them so that the final path is larger + than PATH_MAX. */ + for (size_t i = 0; i <= path_max / sz; i++) + { + xmkdir (toolong_subdir, S_IRWXU); + xchdir (toolong_subdir); + } + return base; +} void -support_set_test_dir (const char *path) +support_chdir_toolong_temp_directory (const char *base) { - test_dir = path; + size_t path_max = get_path_max (); + ensure_toolong_subdir_initialized (); + + xchdir (base); + + size_t sz = strlen (toolong_subdir); + for (size_t i = 0; i <= path_max / sz; i++) + xchdir (toolong_subdir); +} + +/* Helper functions called by the test skeleton follow. */ + +static void +remove_toolong_subdirs (const char *base) +{ + size_t path_max = get_path_max (); + + ensure_toolong_subdir_initialized (); + + if (chdir (base) != 0) + { + printf ("warning: toolong cleanup base failed: chdir (\"%s\"): %m\n", + base); + return; + } + + /* Descend. */ + int levels = 0; + size_t sz = strlen (toolong_subdir); + for (levels = 0; levels <= path_max / sz; levels++) + if (chdir (toolong_subdir) != 0) + { + printf ("warning: toolong cleanup failed: chdir (\"%s\"): %m\n", + toolong_subdir); + return; + } + + /* Ascend and remove. */ + while (--levels >= 0) + { + if (chdir ("..") != 0) + { + printf ("warning: toolong cleanup failed: chdir (\"..\"): %m\n"); + return; + } + if (remove (toolong_subdir) != 0) + { + printf ("warning: could not remove subdirectory: %s: %m\n", + toolong_subdir); + return; + } + } } void @@ -123,6 +267,9 @@ support_delete_temp_files (void) around, to prevent PID reuse.) */ if (temp_name_list->owner == pid) { + if (temp_name_list->toolong) + remove_toolong_subdirs (temp_name_list->name); + if (remove (temp_name_list->name) != 0) printf ("warning: could not remove temporary file: %s: %m\n", temp_name_list->name); @@ -133,6 +280,8 @@ support_delete_temp_files (void) free (temp_name_list); temp_name_list = next; } + + free (toolong_subdir); } void @@ -147,3 +296,9 @@ support_print_temp_files (FILE *f) fprintf (f, ")\n"); } } + +void +support_set_test_dir (const char *path) +{ + test_dir = path; +} diff --git a/support/temp_file.h b/support/temp_file.h index 50a443abe4..8459ddda72 100644 --- a/support/temp_file.h +++ b/support/temp_file.h @@ -44,6 +44,15 @@ int create_temp_file_in_dir (const char *base, const char *dir, returns. The caller should free this string. */ char *support_create_temp_directory (const char *base); +/* Create a temporary directory tree that is longer than PATH_MAX and schedule + it for deletion. BASENAME is used as a prefix for the unique directory + name, which the function returns. The caller should free this string. */ +char *support_create_and_chdir_toolong_temp_directory (const char *basename); + +/* Change into the innermost directory of the directory tree BASE, which was + created using support_create_and_chdir_toolong_temp_directory. */ +void support_chdir_toolong_temp_directory (const char *base); + __END_DECLS #endif /* SUPPORT_TEMP_FILE_H */ -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 1/3] support: Add helpers to create paths longer than PATH_MAX 2022-01-19 8:21 ` [PATCH v2 1/3] support: Add helpers to create paths longer than PATH_MAX Siddhesh Poyarekar @ 2022-01-19 10:13 ` Siddhesh Poyarekar 2022-01-20 14:47 ` Cristian Rodríguez 0 siblings, 1 reply; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-19 10:13 UTC (permalink / raw) To: Siddhesh Poyarekar, libc-alpha On 19/01/2022 13:51, Siddhesh Poyarekar via Libc-alpha wrote: > Add new helpers support_create_and_chdir_toolong_temp_directory and > support_chdir_toolong_temp_directory to create and descend into > directory trees longer than PATH_MAX. > > Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> > --- > support/temp_file.c | 173 +++++++++++++++++++++++++++++++++++++++++--- > support/temp_file.h | 9 +++ > 2 files changed, 173 insertions(+), 9 deletions(-) > > diff --git a/support/temp_file.c b/support/temp_file.c > index e7bb8aadb9..59060e573f 100644 > --- a/support/temp_file.c > +++ b/support/temp_file.c > @@ -1,5 +1,6 @@ > /* Temporary file handling for tests. > Copyright (C) 1998-2022 Free Software Foundation, Inc. > + Copyright The GNU Tools Authors. > This file is part of the GNU C Library. > > The GNU C Library is free software; you can redistribute it and/or > @@ -20,15 +21,17 @@ > some 32-bit platforms. */ > #define _FILE_OFFSET_BITS 64 > > +#include <support/check.h> > #include <support/temp_file.h> > #include <support/temp_file-internal.h> > #include <support/support.h> > > +#include <errno.h> > #include <paths.h> > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > -#include <unistd.h> > +#include <xunistd.h> > > /* List of temporary files. */ > static struct temp_name_list > @@ -36,14 +39,32 @@ static struct temp_name_list > struct temp_name_list *next; > char *name; > pid_t owner; > + bool toolong; > } *temp_name_list; > > /* Location of the temporary files. Set by the test skeleton via > support_set_test_dir. The string is not be freed. */ > static const char *test_dir = _PATH_TMP; > > -void > -add_temp_file (const char *name) > +/* Name of subdirectories in a too long temporary directory tree. */ > +static char *toolong_subdir; > +static bool toolong_subdir_initialized; > + > +/* Return the maximum size of path on the target. */ > +static inline size_t > +get_path_max (void) > +{ > +#ifdef PATH_MAX > + return PATH_MAX; > +#else > + size_t path_max = pathconf ("/", _PC_PATH_MAX); > + return (path_max < 0 ? 1024 > + : path_max <= PTRDIFF_MAX ? path_max : PTRDIFF_MAX); > +#endif > +} > + > +static void > +add_temp_file_internal (const char *name, bool toolong) > { > struct temp_name_list *newp > = (struct temp_name_list *) xcalloc (sizeof (*newp), 1); > @@ -53,12 +74,19 @@ add_temp_file (const char *name) > newp->name = newname; > newp->next = temp_name_list; > newp->owner = getpid (); > + newp->toolong = toolong; > temp_name_list = newp; > } > else > free (newp); > } > > +void > +add_temp_file (const char *name) > +{ > + add_temp_file_internal (name, false); > +} > + > int > create_temp_file_in_dir (const char *base, const char *dir, char **filename) > { > @@ -90,8 +118,8 @@ create_temp_file (const char *base, char **filename) > return create_temp_file_in_dir (base, test_dir, filename); > } > > -char * > -support_create_temp_directory (const char *base) > +static char * > +create_temp_directory_internal (const char *base, bool toolong) > { > char *path = xasprintf ("%s/%sXXXXXX", test_dir, base); > if (mkdtemp (path) == NULL) > @@ -99,16 +127,132 @@ support_create_temp_directory (const char *base) > printf ("error: mkdtemp (\"%s\"): %m", path); > exit (1); > } > - add_temp_file (path); > + add_temp_file_internal (path, toolong); > return path; > } > > -/* Helper functions called by the test skeleton follow. */ > +char * > +support_create_temp_directory (const char *base) > +{ > + return create_temp_directory_internal (base, false); > +} > + > +static void > +ensure_toolong_subdir_initialized (void) > +{ > + if (!toolong_subdir_initialized) > + FAIL_EXIT1 ("uninitialized toolong directory tree\n"); > +} > + > +static void > +initialize_toolong_subdir (void) > +{ > + size_t sz = NAME_MAX; > + char testname[NAME_MAX]; > + int ret; > + > + if (toolong_subdir_initialized) > + return; > + > + memset (testname, 'X', sz - 1); > + > + /* Explore the name limit on the file system. This should typically be > + NAME_MAX, but it could be less on some fuse filesystems. */ > + do > + { > + struct stat statbuf; > + > + testname[sz - 1] = '\0'; > + ret = stat (testname, &statbuf); > + } > + while (ret != 0 && errno == ENAMETOOLONG && (sz = sz / 2) > 0); > + > + if (ret != 0 && errno == ENAMETOOLONG) > + FAIL_EXIT1 ("stat (%s) failed with ENAMETOOLONG\n", testname); > + > + toolong_subdir = xmalloc (sz); > + strcpy (toolong_subdir, testname); > + toolong_subdir_initialized = true; > +} > + > +char * > +support_create_and_chdir_toolong_temp_directory (const char *basename) > +{ > + size_t path_max = get_path_max (); > + > + char *base = create_temp_directory_internal (basename, true); > + > + xchdir (base); > + > + initialize_toolong_subdir (); > + > + size_t sz = strlen (toolong_subdir); > + > + /* Create directories and descend into them so that the final path is larger > + than PATH_MAX. */ > + for (size_t i = 0; i <= path_max / sz; i++) > + { > + xmkdir (toolong_subdir, S_IRWXU); This still doesn't work on the trybot, which somehow fails with ENAMETOOLONG even though stat() on that length worked in initialize_toolong_subdir. I'm going to need to reproduce the environment properly to figure out what's going on. Siddhesh > + xchdir (toolong_subdir); > + } > + return base; > +} > > void > -support_set_test_dir (const char *path) > +support_chdir_toolong_temp_directory (const char *base) > { > - test_dir = path; > + size_t path_max = get_path_max (); > + ensure_toolong_subdir_initialized (); > + > + xchdir (base); > + > + size_t sz = strlen (toolong_subdir); > + for (size_t i = 0; i <= path_max / sz; i++) > + xchdir (toolong_subdir); > +} > + > +/* Helper functions called by the test skeleton follow. */ > + > +static void > +remove_toolong_subdirs (const char *base) > +{ > + size_t path_max = get_path_max (); > + > + ensure_toolong_subdir_initialized (); > + > + if (chdir (base) != 0) > + { > + printf ("warning: toolong cleanup base failed: chdir (\"%s\"): %m\n", > + base); > + return; > + } > + > + /* Descend. */ > + int levels = 0; > + size_t sz = strlen (toolong_subdir); > + for (levels = 0; levels <= path_max / sz; levels++) > + if (chdir (toolong_subdir) != 0) > + { > + printf ("warning: toolong cleanup failed: chdir (\"%s\"): %m\n", > + toolong_subdir); > + return; > + } > + > + /* Ascend and remove. */ > + while (--levels >= 0) > + { > + if (chdir ("..") != 0) > + { > + printf ("warning: toolong cleanup failed: chdir (\"..\"): %m\n"); > + return; > + } > + if (remove (toolong_subdir) != 0) > + { > + printf ("warning: could not remove subdirectory: %s: %m\n", > + toolong_subdir); > + return; > + } > + } > } > > void > @@ -123,6 +267,9 @@ support_delete_temp_files (void) > around, to prevent PID reuse.) */ > if (temp_name_list->owner == pid) > { > + if (temp_name_list->toolong) > + remove_toolong_subdirs (temp_name_list->name); > + > if (remove (temp_name_list->name) != 0) > printf ("warning: could not remove temporary file: %s: %m\n", > temp_name_list->name); > @@ -133,6 +280,8 @@ support_delete_temp_files (void) > free (temp_name_list); > temp_name_list = next; > } > + > + free (toolong_subdir); > } > > void > @@ -147,3 +296,9 @@ support_print_temp_files (FILE *f) > fprintf (f, ")\n"); > } > } > + > +void > +support_set_test_dir (const char *path) > +{ > + test_dir = path; > +} > diff --git a/support/temp_file.h b/support/temp_file.h > index 50a443abe4..8459ddda72 100644 > --- a/support/temp_file.h > +++ b/support/temp_file.h > @@ -44,6 +44,15 @@ int create_temp_file_in_dir (const char *base, const char *dir, > returns. The caller should free this string. */ > char *support_create_temp_directory (const char *base); > > +/* Create a temporary directory tree that is longer than PATH_MAX and schedule > + it for deletion. BASENAME is used as a prefix for the unique directory > + name, which the function returns. The caller should free this string. */ > +char *support_create_and_chdir_toolong_temp_directory (const char *basename); > + > +/* Change into the innermost directory of the directory tree BASE, which was > + created using support_create_and_chdir_toolong_temp_directory. */ > +void support_chdir_toolong_temp_directory (const char *base); > + > __END_DECLS > > #endif /* SUPPORT_TEMP_FILE_H */ ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 1/3] support: Add helpers to create paths longer than PATH_MAX 2022-01-19 10:13 ` Siddhesh Poyarekar @ 2022-01-20 14:47 ` Cristian Rodríguez 2022-01-20 14:56 ` Florian Weimer 2022-01-20 14:57 ` Siddhesh Poyarekar 0 siblings, 2 replies; 36+ messages in thread From: Cristian Rodríguez @ 2022-01-20 14:47 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: Siddhesh Poyarekar, libc-alpha On Wed, Jan 19, 2022 at 7:21 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote: > > On 19/01/2022 13:51, Siddhesh Poyarekar via Libc-alpha wrote: > > Add new helpers support_create_and_chdir_toolong_temp_directory and > > support_chdir_toolong_temp_directory to create and descend into > > directory trees longer than PATH_MAX. Note that recentish linux kernel versions do not allow d_name 's larger than kernel 's PATH_MAX definition, in that case getdents returns EIO, this behavior is since kernel 5.5. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 1/3] support: Add helpers to create paths longer than PATH_MAX 2022-01-20 14:47 ` Cristian Rodríguez @ 2022-01-20 14:56 ` Florian Weimer 2022-01-20 15:17 ` Cristian Rodríguez 2022-01-20 14:57 ` Siddhesh Poyarekar 1 sibling, 1 reply; 36+ messages in thread From: Florian Weimer @ 2022-01-20 14:56 UTC (permalink / raw) To: Cristian Rodríguez Cc: Siddhesh Poyarekar, Siddhesh Poyarekar, libc-alpha * Cristian Rodríguez: > On Wed, Jan 19, 2022 at 7:21 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote: >> >> On 19/01/2022 13:51, Siddhesh Poyarekar via Libc-alpha wrote: >> > Add new helpers support_create_and_chdir_toolong_temp_directory and >> > support_chdir_toolong_temp_directory to create and descend into >> > directory trees longer than PATH_MAX. > > Note that recentish linux kernel versions do not allow d_name 's > larger than kernel 's PATH_MAX definition, in that case getdents > returns EIO, this behavior is since kernel 5.5. NAME_MAX or PATH_MAX? d_name should be capped by NAME_MAX, but historically it is not. Thanks, Florian ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 1/3] support: Add helpers to create paths longer than PATH_MAX 2022-01-20 14:56 ` Florian Weimer @ 2022-01-20 15:17 ` Cristian Rodríguez 0 siblings, 0 replies; 36+ messages in thread From: Cristian Rodríguez @ 2022-01-20 15:17 UTC (permalink / raw) To: Florian Weimer; +Cc: Siddhesh Poyarekar, Siddhesh Poyarekar, libc-alpha On Thu, Jan 20, 2022 at 11:56 AM Florian Weimer <fweimer@redhat.com> wrote: > > * Cristian Rodríguez: > > > On Wed, Jan 19, 2022 at 7:21 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote: > >> > >> On 19/01/2022 13:51, Siddhesh Poyarekar via Libc-alpha wrote: > >> > Add new helpers support_create_and_chdir_toolong_temp_directory and > >> > support_chdir_toolong_temp_directory to create and descend into > >> > directory trees longer than PATH_MAX. > > > > Note that recentish linux kernel versions do not allow d_name 's > > larger than kernel 's PATH_MAX definition, in that case getdents > > returns EIO, this behavior is since kernel 5.5. > > NAME_MAX or PATH_MAX? d_name should be capped by NAME_MAX, but > historically it is not. PATH_MAX.. I wrote that correctly, see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2c6b7bcd747201441923a0d3062577a8d1fbd8f8 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 1/3] support: Add helpers to create paths longer than PATH_MAX 2022-01-20 14:47 ` Cristian Rodríguez 2022-01-20 14:56 ` Florian Weimer @ 2022-01-20 14:57 ` Siddhesh Poyarekar 1 sibling, 0 replies; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-20 14:57 UTC (permalink / raw) To: Cristian Rodríguez; +Cc: libc-alpha On 20/01/2022 20:17, Cristian Rodríguez wrote: > On Wed, Jan 19, 2022 at 7:21 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote: >> >> On 19/01/2022 13:51, Siddhesh Poyarekar via Libc-alpha wrote: >>> Add new helpers support_create_and_chdir_toolong_temp_directory and >>> support_chdir_toolong_temp_directory to create and descend into >>> directory trees longer than PATH_MAX. > > Note that recentish linux kernel versions do not allow d_name 's > larger than kernel 's PATH_MAX definition, in that case getdents > returns EIO, this behavior is since kernel 5.5. > The d_name in these helpers is limited to _PC_NAME_MAX so AFAICT we shouldn't run into issues with that. Thanks, Siddhesh ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX (CVE-2021-3998) 2022-01-19 8:21 [PATCH v2 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 Siddhesh Poyarekar 2022-01-19 8:21 ` [PATCH v2 1/3] support: Add helpers to create paths longer than PATH_MAX Siddhesh Poyarekar @ 2022-01-19 8:21 ` Siddhesh Poyarekar 2022-01-19 8:21 ` [PATCH v2 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) Siddhesh Poyarekar 2022-01-20 9:32 ` [PATCH v3 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 Siddhesh Poyarekar 3 siblings, 0 replies; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-19 8:21 UTC (permalink / raw) To: libc-alpha realpath returns an allocated string when the result exceeds PATH_MAX, which is unexpected when its second argument is not NULL. This results in the second argument (resolved) being uninitialized and also results in a memory leak since the caller expects resolved to be the same as the returned value. Return NULL and set errno to ENAMETOOLONG if the result exceeds PATH_MAX. This fixes BZ #28770, which is CVE-2021-3998. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- NEWS | 4 +++ stdlib/Makefile | 1 + stdlib/canonicalize.c | 12 +++++++-- stdlib/tst-realpath-toolong.c | 49 +++++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 stdlib/tst-realpath-toolong.c diff --git a/NEWS b/NEWS index 6ed9fa9787..4c392a445e 100644 --- a/NEWS +++ b/NEWS @@ -166,6 +166,10 @@ Security related changes: CVE-2022-23218: Passing an overlong file name to the svcunix_create legacy function could result in a stack-based buffer overflow. + CVE-2021-3998: Passing a path longer than PATH_MAX to the realpath + function could result in a memory leak and potential access of + uninitialized memory. Reported by Qualys. + The following bugs are resolved with this release: [The release manager will add the list generated by diff --git a/stdlib/Makefile b/stdlib/Makefile index 1e81f98fac..8236741984 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -109,6 +109,7 @@ tests := \ tst-random \ tst-random2 \ tst-realpath \ + tst-realpath-toolong \ tst-secure-getenv \ tst-setcontext \ tst-setcontext2 \ diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c index f36bdf4c76..732dc7ea46 100644 --- a/stdlib/canonicalize.c +++ b/stdlib/canonicalize.c @@ -400,8 +400,16 @@ realpath_stk (const char *name, char *resolved, error: *dest++ = '\0'; - if (resolved != NULL && dest - rname <= get_path_max ()) - rname = strcpy (resolved, rname); + if (resolved != NULL) + { + if (dest - rname <= get_path_max ()) + rname = strcpy (resolved, rname); + else + { + failed = true; + __set_errno (ENAMETOOLONG); + } + } error_nomem: scratch_buffer_free (&extra_buffer); diff --git a/stdlib/tst-realpath-toolong.c b/stdlib/tst-realpath-toolong.c new file mode 100644 index 0000000000..8bed772460 --- /dev/null +++ b/stdlib/tst-realpath-toolong.c @@ -0,0 +1,49 @@ +/* Verify that realpath returns NULL with ENAMETOOLONG if the result exceeds + NAME_MAX. + Copyright The GNU Toolchain Authors. + 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 + <https://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <limits.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <support/check.h> +#include <support/temp_file.h> +#include <sys/types.h> +#include <sys/stat.h> + +#define BASENAME "tst-realpath-toolong." + +int +do_test (void) +{ + char *base = support_create_and_chdir_toolong_temp_directory (BASENAME); + + char buf[PATH_MAX + 1]; + const char *res = realpath (".", buf); + + /* canonicalize.c states that if the real path is >= PATH_MAX, then + realpath returns NULL and sets ENAMETOOLONG. */ + TEST_VERIFY (res == NULL); + TEST_VERIFY (errno == ENAMETOOLONG); + + free (base); + return 0; +} + +#include <support/test-driver.c> -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) 2022-01-19 8:21 [PATCH v2 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 Siddhesh Poyarekar 2022-01-19 8:21 ` [PATCH v2 1/3] support: Add helpers to create paths longer than PATH_MAX Siddhesh Poyarekar 2022-01-19 8:21 ` [PATCH v2 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX (CVE-2021-3998) Siddhesh Poyarekar @ 2022-01-19 8:21 ` Siddhesh Poyarekar 2022-01-20 9:32 ` [PATCH v3 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 Siddhesh Poyarekar 3 siblings, 0 replies; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-19 8:21 UTC (permalink / raw) To: libc-alpha; +Cc: adhemerval.zanella, eggert, Qualys Security Advisory No valid path returned by getcwd would fit into 1 byte, so reject the size early and return NULL with errno set to ERANGE. This change is prompted by CVE-2021-3999, which describes a single byte buffer underflow and overflow when all of the following conditions are met: - The buffer size (i.e. the second argument of getcwd) is 1 byte - The current working directory is too long - '/' is also mounted on the current working directory Sequence of events: - In sysdeps/unix/sysv/linux/getcwd.c, the syscall returns ENAMETOOLONG because the linux kernel checks for name length before it checks buffer size - The code falls back to the generic getcwd in sysdeps/posix - In the generic func, the buf[0] is set to '\0' on line 250 - this while loop on line 262 is bypassed: while (!(thisdev == rootdev && thisino == rootino)) since the rootfs (/) is bind mounted onto the directory and the flow goes on to line 449, where it puts a '/' in the byte before the buffer. - Finally on line 458, it moves 2 bytes (the underflowed byte and the '\0') to the buf[0] and buf[1], resulting in a 1 byte buffer overflow. - buf is returned on line 469 and errno is not set. This resolves BZ #28769. Signed-off-by: Qualys Security Advisory <qsa@qualys.com> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- NEWS | 6 + sysdeps/posix/getcwd.c | 7 + sysdeps/unix/sysv/linux/Makefile | 7 +- .../unix/sysv/linux/tst-getcwd-smallbuff.c | 245 ++++++++++++++++++ 4 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c diff --git a/NEWS b/NEWS index 4c392a445e..07e9eac52d 100644 --- a/NEWS +++ b/NEWS @@ -170,6 +170,12 @@ Security related changes: function could result in a memory leak and potential access of uninitialized memory. Reported by Qualys. + CVE-2021-3999: Passing a buffer of size exactly 1 byte to the getcwd + function may result in an off-by-one buffer underflow and overflow + when the current working directory is longer than PATH_MAX and also + corresponds to the / directory through an unprivileged mount + namespace. Reported by Qualys. + The following bugs are resolved with this release: [The release manager will add the list generated by diff --git a/sysdeps/posix/getcwd.c b/sysdeps/posix/getcwd.c index e147a31a81..9d5787b6f4 100644 --- a/sysdeps/posix/getcwd.c +++ b/sysdeps/posix/getcwd.c @@ -187,6 +187,13 @@ __getcwd_generic (char *buf, size_t size) size_t allocated = size; size_t used; + /* A size of 1 byte is never useful. */ + if (allocated == 1) + { + __set_errno (ERANGE); + return NULL; + } + #if HAVE_MINIMALLY_WORKING_GETCWD /* If AT_FDCWD is not defined, the algorithm below is O(N**2) and this is much slower than the system getcwd (at least on diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index 85fc8cbf75..7ca9350c99 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -346,7 +346,12 @@ sysdep_routines += xstatconv internal_statvfs \ sysdep_headers += bits/fcntl-linux.h -tests += tst-fallocate tst-fallocate64 tst-o_path-locks +tests += \ + tst-fallocate \ + tst-fallocate64 \ + tst-getcwd-smallbuff \ + tst-o_path-locks \ +# tests endif ifeq ($(subdir),elf) diff --git a/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c new file mode 100644 index 0000000000..791dfe4d02 --- /dev/null +++ b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c @@ -0,0 +1,245 @@ +/* Verify that getcwd returns ERANGE for size 1 byte and does not underflow + buffer when the CWD is too long and is also a mount target of /. See bug + #28769 or CVE-2021-3999 for more context. + Copyright The GNU Toolchain Authors. + 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 + <https://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <fcntl.h> +#include <limits.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/mount.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <sys/wait.h> + +#include <sys/socket.h> +#include <sys/un.h> +#include <support/check.h> +#include <support/temp_file.h> +#include <support/xsched.h> +#include <support/xunistd.h> + +#ifndef PATH_MAX +# define PATH_MAX 1024 +#endif + +static char *base; +#define BASENAME "tst-getcwd-smallbuff" +#define MOUNT_NAME "mpoint" +static int sockfd[2]; + +static void +do_cleanup (void) +{ + support_chdir_toolong_temp_directory (base); + TEST_VERIFY_EXIT (rmdir (MOUNT_NAME) == 0); + free (base); +} + +static void +send_fd (const int sock, const int fd) +{ + struct msghdr msg; + union + { + struct cmsghdr hdr; + char buf[CMSG_SPACE (sizeof (int))]; + } cmsgbuf; + struct cmsghdr *cmsg; + struct iovec vec; + char ch = 'A'; + ssize_t n; + + memset (&msg, 0, sizeof (msg)); + memset (&cmsgbuf, 0, sizeof (cmsgbuf)); + msg.msg_control = &cmsgbuf.buf; + msg.msg_controllen = sizeof (cmsgbuf.buf); + + cmsg = CMSG_FIRSTHDR (&msg); + cmsg->cmsg_len = CMSG_LEN (sizeof (int)); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + *(int *) CMSG_DATA (cmsg) = fd; + + vec.iov_base = &ch; + vec.iov_len = 1; + msg.msg_iov = &vec; + msg.msg_iovlen = 1; + + while ((n = sendmsg (sock, &msg, 0)) == -1 && errno == EINTR); + + TEST_VERIFY_EXIT (n == 1); +} + +static int +recv_fd (const int sock) +{ + struct msghdr msg; + union + { + struct cmsghdr hdr; + char buf[CMSG_SPACE(sizeof(int))]; + } cmsgbuf; + struct cmsghdr *cmsg; + struct iovec vec; + ssize_t n; + char ch = '\0'; + int fd = -1; + + memset (&msg, 0, sizeof (msg)); + vec.iov_base = &ch; + vec.iov_len = 1; + msg.msg_iov = &vec; + msg.msg_iovlen = 1; + + memset (&cmsgbuf, 0, sizeof (cmsgbuf)); + msg.msg_control = &cmsgbuf.buf; + msg.msg_controllen = sizeof (cmsgbuf.buf); + + while ((n = recvmsg (sock, &msg, 0)) == -1 && errno == EINTR); + if (n != 1 || ch != 'A') + return -1; + + cmsg = CMSG_FIRSTHDR (&msg); + if (cmsg == NULL) + return -1; + if (cmsg->cmsg_type != SCM_RIGHTS) + return -1; + fd = *(const int *) CMSG_DATA (cmsg); + if (fd < 0) + return -1; + return fd; +} + +static int +child_func (void * const arg) +{ + xclose (sockfd[0]); + const int sock = sockfd[1]; + char ch; + + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); + TEST_VERIFY_EXIT (ch == '1'); + + if (mount ("/", MOUNT_NAME, NULL, MS_BIND | MS_REC, NULL)) + FAIL_EXIT1 ("mount failed: %m\n"); + const int fd = xopen ("mpoint", + O_RDONLY | O_PATH | O_DIRECTORY | O_NOFOLLOW, 0); + + send_fd (sock, fd); + xclose (fd); + + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); + TEST_VERIFY_EXIT (ch == 'a'); + + xclose (sock); + return 0; +} + +static void +update_map (char * const mapping, const char * const map_file) +{ + const size_t map_len = strlen (mapping); + + const int fd = xopen (map_file, O_WRONLY, 0); + xwrite (fd, mapping, map_len); + xclose (fd); +} + +static void +proc_setgroups_write (const long child_pid, const char * const str) +{ + const size_t str_len = strlen(str); + + char setgroups_path[64]; + snprintf (setgroups_path, sizeof (setgroups_path), + "/proc/%ld/setgroups", child_pid); + + const int fd = open (setgroups_path, O_WRONLY); + + if (fd < 0) + { + TEST_VERIFY_EXIT (errno == ENOENT); + FAIL_UNSUPPORTED ("/proc/%ld/setgroups not found\n", child_pid); + } + + xwrite (fd, str, str_len); + xclose(fd); +} + +static char child_stack[1024 * 1024]; + +int +do_test (void) +{ + base = support_create_and_chdir_toolong_temp_directory (BASENAME); + + xmkdir (MOUNT_NAME, S_IRWXU); + atexit (do_cleanup); + + TEST_VERIFY_EXIT (socketpair (AF_UNIX, SOCK_STREAM, 0, sockfd) == 0); + pid_t child_pid = xclone (child_func, NULL, child_stack, + sizeof (child_stack), + CLONE_NEWUSER | CLONE_NEWNS | SIGCHLD); + + xclose (sockfd[1]); + const int sock = sockfd[0]; + + char map_path[64], map_buf[64]; + snprintf (map_path, sizeof (map_path), "/proc/%ld/uid_map", + (long) child_pid); + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getuid()); + update_map (map_buf, map_path); + + proc_setgroups_write ((long) child_pid, "deny"); + snprintf (map_path, sizeof (map_path), "/proc/%ld/gid_map", + (long) child_pid); + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getgid()); + update_map (map_buf, map_path); + + TEST_VERIFY_EXIT (send (sock, "1", 1, MSG_NOSIGNAL) == 1); + const int fd = recv_fd (sock); + TEST_VERIFY_EXIT (fd >= 0); + TEST_VERIFY_EXIT (fchdir (fd) == 0); + + static char buf[2 * 10 + 1]; + memset (buf, 'A', sizeof(buf)); + + /* Finally, call getcwd and check if it resulted in a buffer underflow. */ + char * cwd = getcwd (buf + sizeof(buf) / 2, 1); + TEST_VERIFY (cwd == NULL); + TEST_VERIFY (errno == ERANGE); + + for (int i = 0; i < sizeof (buf); i++) + if (buf[i] != 'A') + { + printf ("buf[%d] = %02x\n", i, (unsigned int) buf[i]); + support_record_failure (); + } + + TEST_VERIFY_EXIT (send (sock, "a", 1, MSG_NOSIGNAL) == 1); + xclose (sock); + TEST_VERIFY_EXIT (xwaitpid (child_pid, NULL, 0) == child_pid); + + return 0; +} + +#define CLEANUP_HANDLER do_cleanup +#include <support/test-driver.c> -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 2022-01-19 8:21 [PATCH v2 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 Siddhesh Poyarekar ` (2 preceding siblings ...) 2022-01-19 8:21 ` [PATCH v2 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) Siddhesh Poyarekar @ 2022-01-20 9:32 ` Siddhesh Poyarekar 2022-01-20 9:32 ` [PATCH v3 1/3] support: Add helpers to create paths longer than PATH_MAX Siddhesh Poyarekar ` (2 more replies) 3 siblings, 3 replies; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-20 9:32 UTC (permalink / raw) To: libc-alpha; +Cc: eggert, adhemerval.zanella, fweimer Add functions to make directory trees with paths longer than PATH_MAX and use them to test fixes for CVE-2021-3998 and CVE-2021-3999. Tested on x86_64 and i686. Changes from v2: - Adjusted test infrastructure to bail out if the filesystem does not support creating directory trees greater than PATH_MAX. Changes from v1: - Try reducing directory name size to meet lower limits of some fuse filesystems - Fixed review comments - Credited Qualys in NEWS - Use x* functions wherever possible - Drop size check in linux getcwd implementation and rely only on the posix one to flag the error - Fix formatting issues I had missed before. Siddhesh Poyarekar (3): support: Add helpers to create paths longer than PATH_MAX realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) NEWS | 10 + stdlib/Makefile | 1 + stdlib/canonicalize.c | 12 +- stdlib/tst-realpath-toolong.c | 49 ++++ support/temp_file.c | 159 +++++++++++- support/temp_file.h | 9 + sysdeps/posix/getcwd.c | 7 + sysdeps/unix/sysv/linux/Makefile | 7 +- .../unix/sysv/linux/tst-getcwd-smallbuff.c | 245 ++++++++++++++++++ 9 files changed, 487 insertions(+), 12 deletions(-) create mode 100644 stdlib/tst-realpath-toolong.c create mode 100644 sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 1/3] support: Add helpers to create paths longer than PATH_MAX 2022-01-20 9:32 ` [PATCH v3 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 Siddhesh Poyarekar @ 2022-01-20 9:32 ` Siddhesh Poyarekar 2022-01-21 16:27 ` Adhemerval Zanella 2022-01-20 9:32 ` [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] Siddhesh Poyarekar 2022-01-20 9:32 ` [PATCH v3 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) Siddhesh Poyarekar 2 siblings, 1 reply; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-20 9:32 UTC (permalink / raw) To: libc-alpha; +Cc: eggert, adhemerval.zanella, fweimer Add new helpers support_create_and_chdir_toolong_temp_directory and support_chdir_toolong_temp_directory to create and descend into directory trees longer than PATH_MAX. Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- support/temp_file.c | 159 +++++++++++++++++++++++++++++++++++++++++--- support/temp_file.h | 9 +++ 2 files changed, 159 insertions(+), 9 deletions(-) diff --git a/support/temp_file.c b/support/temp_file.c index e7bb8aadb9..e41128c2d4 100644 --- a/support/temp_file.c +++ b/support/temp_file.c @@ -1,5 +1,6 @@ /* Temporary file handling for tests. Copyright (C) 1998-2022 Free Software Foundation, Inc. + Copyright The GNU Tools Authors. This file is part of the GNU C Library. The GNU C Library is free software; you can redistribute it and/or @@ -20,15 +21,17 @@ some 32-bit platforms. */ #define _FILE_OFFSET_BITS 64 +#include <support/check.h> #include <support/temp_file.h> #include <support/temp_file-internal.h> #include <support/support.h> +#include <errno.h> #include <paths.h> #include <stdio.h> #include <stdlib.h> #include <string.h> -#include <unistd.h> +#include <xunistd.h> /* List of temporary files. */ static struct temp_name_list @@ -36,14 +39,20 @@ static struct temp_name_list struct temp_name_list *next; char *name; pid_t owner; + bool toolong; } *temp_name_list; /* Location of the temporary files. Set by the test skeleton via support_set_test_dir. The string is not be freed. */ static const char *test_dir = _PATH_TMP; -void -add_temp_file (const char *name) +/* Name of subdirectories in a too long temporary directory tree. */ +static char toolong_subdir[NAME_MAX + 1]; +static bool toolong_initialized; +static size_t toolong_path_max; + +static void +add_temp_file_internal (const char *name, bool toolong) { struct temp_name_list *newp = (struct temp_name_list *) xcalloc (sizeof (*newp), 1); @@ -53,12 +62,19 @@ add_temp_file (const char *name) newp->name = newname; newp->next = temp_name_list; newp->owner = getpid (); + newp->toolong = toolong; temp_name_list = newp; } else free (newp); } +void +add_temp_file (const char *name) +{ + add_temp_file_internal (name, false); +} + int create_temp_file_in_dir (const char *base, const char *dir, char **filename) { @@ -90,8 +106,8 @@ create_temp_file (const char *base, char **filename) return create_temp_file_in_dir (base, test_dir, filename); } -char * -support_create_temp_directory (const char *base) +static char * +create_temp_directory_internal (const char *base, bool toolong) { char *path = xasprintf ("%s/%sXXXXXX", test_dir, base); if (mkdtemp (path) == NULL) @@ -99,16 +115,132 @@ support_create_temp_directory (const char *base) printf ("error: mkdtemp (\"%s\"): %m", path); exit (1); } - add_temp_file (path); + add_temp_file_internal (path, toolong); return path; } -/* Helper functions called by the test skeleton follow. */ +char * +support_create_temp_directory (const char *base) +{ + return create_temp_directory_internal (base, false); +} + +static void +ensure_toolong_initialized (void) +{ + if (!toolong_initialized) + FAIL_EXIT1 ("uninitialized toolong directory tree\n"); +} + +static void +initialize_toolong (const char *base) +{ + long name_max = pathconf (base, _PC_NAME_MAX); + name_max = (name_max < 0 ? 64 + : (name_max < sizeof (toolong_subdir) ? name_max + : sizeof (toolong_subdir) - 1)); + + long path_max = pathconf (base, _PC_PATH_MAX); + path_max = (path_max < 0 ? 1024 + : path_max <= PTRDIFF_MAX ? path_max : PTRDIFF_MAX); + + /* Sanity check to ensure that the test does not create temporary directories + in different filesystems because this API doesn't support it. */ + if (toolong_initialized) + { + if (name_max != strlen (toolong_subdir)) + FAIL_UNSUPPORTED ("name_max: Temporary directories in different" + " filesystems not supported yet\n"); + if (path_max != toolong_path_max) + FAIL_UNSUPPORTED ("path_max: Temporary directories in different" + " filesystems not supported yet\n"); + return; + } + + toolong_path_max = path_max; + + size_t len = name_max; + memset (toolong_subdir, 'X', len); + toolong_initialized = true; +} + +char * +support_create_and_chdir_toolong_temp_directory (const char *basename) +{ + char *base = create_temp_directory_internal (basename, true); + xchdir (base); + + initialize_toolong (base); + + size_t sz = strlen (toolong_subdir); + + /* Create directories and descend into them so that the final path is larger + than PATH_MAX. */ + for (size_t i = 0; i <= toolong_path_max / sz; i++) + { + int ret = mkdir (toolong_subdir, S_IRWXU); + if (ret != 0 && errno == ENAMETOOLONG) + FAIL_UNSUPPORTED ("Filesystem does not support creating too long " + "directory trees\n"); + else if (ret != 0) + FAIL_EXIT1 ("Failed to create directory tree: %m\n"); + xchdir (toolong_subdir); + } + return base; +} void -support_set_test_dir (const char *path) +support_chdir_toolong_temp_directory (const char *base) { - test_dir = path; + ensure_toolong_initialized (); + + xchdir (base); + + size_t sz = strlen (toolong_subdir); + for (size_t i = 0; i <= toolong_path_max / sz; i++) + xchdir (toolong_subdir); +} + +/* Helper functions called by the test skeleton follow. */ + +static void +remove_toolong_subdirs (const char *base) +{ + ensure_toolong_initialized (); + + if (chdir (base) != 0) + { + printf ("warning: toolong cleanup base failed: chdir (\"%s\"): %m\n", + base); + return; + } + + /* Descend. */ + int levels = 0; + size_t sz = strlen (toolong_subdir); + for (levels = 0; levels <= toolong_path_max / sz; levels++) + if (chdir (toolong_subdir) != 0) + { + printf ("warning: toolong cleanup failed: chdir (\"%s\"): %m\n", + toolong_subdir); + break; + } + + /* Ascend and remove. */ + while (--levels >= 0) + { + if (chdir ("..") != 0) + { + printf ("warning: toolong cleanup failed: chdir (\"..\"): %m\n"); + return; + } + if (remove (toolong_subdir) != 0) + { + printf ("warning: could not remove subdirectory: %s: %m\n", + toolong_subdir); + return; + } + } } void @@ -123,6 +255,9 @@ support_delete_temp_files (void) around, to prevent PID reuse.) */ if (temp_name_list->owner == pid) { + if (temp_name_list->toolong) + remove_toolong_subdirs (temp_name_list->name); + if (remove (temp_name_list->name) != 0) printf ("warning: could not remove temporary file: %s: %m\n", temp_name_list->name); @@ -147,3 +282,9 @@ support_print_temp_files (FILE *f) fprintf (f, ")\n"); } } + +void +support_set_test_dir (const char *path) +{ + test_dir = path; +} diff --git a/support/temp_file.h b/support/temp_file.h index 50a443abe4..8459ddda72 100644 --- a/support/temp_file.h +++ b/support/temp_file.h @@ -44,6 +44,15 @@ int create_temp_file_in_dir (const char *base, const char *dir, returns. The caller should free this string. */ char *support_create_temp_directory (const char *base); +/* Create a temporary directory tree that is longer than PATH_MAX and schedule + it for deletion. BASENAME is used as a prefix for the unique directory + name, which the function returns. The caller should free this string. */ +char *support_create_and_chdir_toolong_temp_directory (const char *basename); + +/* Change into the innermost directory of the directory tree BASE, which was + created using support_create_and_chdir_toolong_temp_directory. */ +void support_chdir_toolong_temp_directory (const char *base); + __END_DECLS #endif /* SUPPORT_TEMP_FILE_H */ -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 1/3] support: Add helpers to create paths longer than PATH_MAX 2022-01-20 9:32 ` [PATCH v3 1/3] support: Add helpers to create paths longer than PATH_MAX Siddhesh Poyarekar @ 2022-01-21 16:27 ` Adhemerval Zanella 0 siblings, 0 replies; 36+ messages in thread From: Adhemerval Zanella @ 2022-01-21 16:27 UTC (permalink / raw) To: Siddhesh Poyarekar, libc-alpha; +Cc: eggert, fweimer On 20/01/2022 06:32, Siddhesh Poyarekar wrote: > Add new helpers support_create_and_chdir_toolong_temp_directory and > support_chdir_toolong_temp_directory to create and descend into > directory trees longer than PATH_MAX. > > Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > support/temp_file.c | 159 +++++++++++++++++++++++++++++++++++++++++--- > support/temp_file.h | 9 +++ > 2 files changed, 159 insertions(+), 9 deletions(-) > > diff --git a/support/temp_file.c b/support/temp_file.c > index e7bb8aadb9..e41128c2d4 100644 > --- a/support/temp_file.c > +++ b/support/temp_file.c > @@ -1,5 +1,6 @@ > /* Temporary file handling for tests. > Copyright (C) 1998-2022 Free Software Foundation, Inc. > + Copyright The GNU Tools Authors. > This file is part of the GNU C Library. > > The GNU C Library is free software; you can redistribute it and/or > @@ -20,15 +21,17 @@ > some 32-bit platforms. */ > #define _FILE_OFFSET_BITS 64 > > +#include <support/check.h> > #include <support/temp_file.h> > #include <support/temp_file-internal.h> > #include <support/support.h> > > +#include <errno.h> > #include <paths.h> > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > -#include <unistd.h> > +#include <xunistd.h> > > /* List of temporary files. */ > static struct temp_name_list > @@ -36,14 +39,20 @@ static struct temp_name_list > struct temp_name_list *next; > char *name; > pid_t owner; > + bool toolong; > } *temp_name_list; > > /* Location of the temporary files. Set by the test skeleton via > support_set_test_dir. The string is not be freed. */ > static const char *test_dir = _PATH_TMP; > > -void > -add_temp_file (const char *name) > +/* Name of subdirectories in a too long temporary directory tree. */ > +static char toolong_subdir[NAME_MAX + 1]; > +static bool toolong_initialized; > +static size_t toolong_path_max; > + I think it is suffice for current tests, although it would need to change if any tests requires more than one large path. > +static void > +add_temp_file_internal (const char *name, bool toolong) > { > struct temp_name_list *newp > = (struct temp_name_list *) xcalloc (sizeof (*newp), 1); > @@ -53,12 +62,19 @@ add_temp_file (const char *name) > newp->name = newname; > newp->next = temp_name_list; > newp->owner = getpid (); > + newp->toolong = toolong; > temp_name_list = newp; > } > else > free (newp); > } > > +void > +add_temp_file (const char *name) > +{ > + add_temp_file_internal (name, false); > +} > + > int > create_temp_file_in_dir (const char *base, const char *dir, char **filename) > { > @@ -90,8 +106,8 @@ create_temp_file (const char *base, char **filename) > return create_temp_file_in_dir (base, test_dir, filename); > } > > -char * > -support_create_temp_directory (const char *base) > +static char * > +create_temp_directory_internal (const char *base, bool toolong) > { > char *path = xasprintf ("%s/%sXXXXXX", test_dir, base); > if (mkdtemp (path) == NULL) > @@ -99,16 +115,132 @@ support_create_temp_directory (const char *base) > printf ("error: mkdtemp (\"%s\"): %m", path); > exit (1); > } > - add_temp_file (path); > + add_temp_file_internal (path, toolong); > return path; > } > > -/* Helper functions called by the test skeleton follow. */ > +char * > +support_create_temp_directory (const char *base) > +{ > + return create_temp_directory_internal (base, false); > +} > + > +static void > +ensure_toolong_initialized (void) > +{ > + if (!toolong_initialized) > + FAIL_EXIT1 ("uninitialized toolong directory tree\n"); > +} > + > +static void > +initialize_toolong (const char *base) > +{ > + long name_max = pathconf (base, _PC_NAME_MAX); > + name_max = (name_max < 0 ? 64 > + : (name_max < sizeof (toolong_subdir) ? name_max > + : sizeof (toolong_subdir) - 1)); > + > + long path_max = pathconf (base, _PC_PATH_MAX); > + path_max = (path_max < 0 ? 1024 > + : path_max <= PTRDIFF_MAX ? path_max : PTRDIFF_MAX); > + > + /* Sanity check to ensure that the test does not create temporary directories > + in different filesystems because this API doesn't support it. */ > + if (toolong_initialized) > + { > + if (name_max != strlen (toolong_subdir)) > + FAIL_UNSUPPORTED ("name_max: Temporary directories in different" > + " filesystems not supported yet\n"); > + if (path_max != toolong_path_max) > + FAIL_UNSUPPORTED ("path_max: Temporary directories in different" > + " filesystems not supported yet\n"); > + return; > + } > + > + toolong_path_max = path_max; > + > + size_t len = name_max; > + memset (toolong_subdir, 'X', len); > + toolong_initialized = true; > +} > + Ok. > +char * > +support_create_and_chdir_toolong_temp_directory (const char *basename) > +{> + char *base = create_temp_directory_internal (basename, true); > + xchdir (base); > + > + initialize_toolong (base); > + > + size_t sz = strlen (toolong_subdir); > + > + /* Create directories and descend into them so that the final path is larger > + than PATH_MAX. */ > + for (size_t i = 0; i <= toolong_path_max / sz; i++) > + { > + int ret = mkdir (toolong_subdir, S_IRWXU); > + if (ret != 0 && errno == ENAMETOOLONG) > + FAIL_UNSUPPORTED ("Filesystem does not support creating too long " > + "directory trees\n"); > + else if (ret != 0) > + FAIL_EXIT1 ("Failed to create directory tree: %m\n"); > + xchdir (toolong_subdir); > + } > + return base; > +} > > void > -support_set_test_dir (const char *path) > +support_chdir_toolong_temp_directory (const char *base) > { > - test_dir = path; > + ensure_toolong_initialized (); > + > + xchdir (base); > + > + size_t sz = strlen (toolong_subdir); > + for (size_t i = 0; i <= toolong_path_max / sz; i++) > + xchdir (toolong_subdir); > +} > + > +/* Helper functions called by the test skeleton follow. */ > + > +static void > +remove_toolong_subdirs (const char *base) > +{ > + ensure_toolong_initialized (); > + > + if (chdir (base) != 0) > + { > + printf ("warning: toolong cleanup base failed: chdir (\"%s\"): %m\n", > + base); > + return; > + } I think there is no much we co in such case. > + > + /* Descend. */ > + int levels = 0; > + size_t sz = strlen (toolong_subdir); > + for (levels = 0; levels <= toolong_path_max / sz; levels++) > + if (chdir (toolong_subdir) != 0) > + { > + printf ("warning: toolong cleanup failed: chdir (\"%s\"): %m\n", > + toolong_subdir); > + break; > + } > + > + /* Ascend and remove. */ > + while (--levels >= 0) > + { > + if (chdir ("..") != 0) > + { > + printf ("warning: toolong cleanup failed: chdir (\"..\"): %m\n"); > + return; > + } > + if (remove (toolong_subdir) != 0) > + { > + printf ("warning: could not remove subdirectory: %s: %m\n", > + toolong_subdir); > + return; > + } > + } > } > > void > @@ -123,6 +255,9 @@ support_delete_temp_files (void) > around, to prevent PID reuse.) */ > if (temp_name_list->owner == pid) > { > + if (temp_name_list->toolong) > + remove_toolong_subdirs (temp_name_list->name); > + > if (remove (temp_name_list->name) != 0) > printf ("warning: could not remove temporary file: %s: %m\n", > temp_name_list->name); > @@ -147,3 +282,9 @@ support_print_temp_files (FILE *f) > fprintf (f, ")\n"); > } > } > + > +void > +support_set_test_dir (const char *path) > +{ > + test_dir = path; > +} > diff --git a/support/temp_file.h b/support/temp_file.h > index 50a443abe4..8459ddda72 100644 > --- a/support/temp_file.h > +++ b/support/temp_file.h > @@ -44,6 +44,15 @@ int create_temp_file_in_dir (const char *base, const char *dir, > returns. The caller should free this string. */ > char *support_create_temp_directory (const char *base); > > +/* Create a temporary directory tree that is longer than PATH_MAX and schedule > + it for deletion. BASENAME is used as a prefix for the unique directory > + name, which the function returns. The caller should free this string. */ > +char *support_create_and_chdir_toolong_temp_directory (const char *basename); > + > +/* Change into the innermost directory of the directory tree BASE, which was > + created using support_create_and_chdir_toolong_temp_directory. */ > +void support_chdir_toolong_temp_directory (const char *base); > + > __END_DECLS > > #endif /* SUPPORT_TEMP_FILE_H */ ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] 2022-01-20 9:32 ` [PATCH v3 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 Siddhesh Poyarekar 2022-01-20 9:32 ` [PATCH v3 1/3] support: Add helpers to create paths longer than PATH_MAX Siddhesh Poyarekar @ 2022-01-20 9:32 ` Siddhesh Poyarekar 2022-01-21 23:22 ` Joseph Myers 2022-01-24 13:45 ` [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] Andreas Schwab 2022-01-20 9:32 ` [PATCH v3 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) Siddhesh Poyarekar 2 siblings, 2 replies; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-20 9:32 UTC (permalink / raw) To: libc-alpha; +Cc: eggert, adhemerval.zanella, fweimer realpath returns an allocated string when the result exceeds PATH_MAX, which is unexpected when its second argument is not NULL. This results in the second argument (resolved) being uninitialized and also results in a memory leak since the caller expects resolved to be the same as the returned value. Return NULL and set errno to ENAMETOOLONG if the result exceeds PATH_MAX. This fixes [BZ #28770], which is CVE-2021-3998. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- NEWS | 4 +++ stdlib/Makefile | 1 + stdlib/canonicalize.c | 12 +++++++-- stdlib/tst-realpath-toolong.c | 49 +++++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 stdlib/tst-realpath-toolong.c diff --git a/NEWS b/NEWS index 6ed9fa9787..4c392a445e 100644 --- a/NEWS +++ b/NEWS @@ -166,6 +166,10 @@ Security related changes: CVE-2022-23218: Passing an overlong file name to the svcunix_create legacy function could result in a stack-based buffer overflow. + CVE-2021-3998: Passing a path longer than PATH_MAX to the realpath + function could result in a memory leak and potential access of + uninitialized memory. Reported by Qualys. + The following bugs are resolved with this release: [The release manager will add the list generated by diff --git a/stdlib/Makefile b/stdlib/Makefile index 1e81f98fac..8236741984 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -109,6 +109,7 @@ tests := \ tst-random \ tst-random2 \ tst-realpath \ + tst-realpath-toolong \ tst-secure-getenv \ tst-setcontext \ tst-setcontext2 \ diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c index f36bdf4c76..732dc7ea46 100644 --- a/stdlib/canonicalize.c +++ b/stdlib/canonicalize.c @@ -400,8 +400,16 @@ realpath_stk (const char *name, char *resolved, error: *dest++ = '\0'; - if (resolved != NULL && dest - rname <= get_path_max ()) - rname = strcpy (resolved, rname); + if (resolved != NULL) + { + if (dest - rname <= get_path_max ()) + rname = strcpy (resolved, rname); + else + { + failed = true; + __set_errno (ENAMETOOLONG); + } + } error_nomem: scratch_buffer_free (&extra_buffer); diff --git a/stdlib/tst-realpath-toolong.c b/stdlib/tst-realpath-toolong.c new file mode 100644 index 0000000000..8bed772460 --- /dev/null +++ b/stdlib/tst-realpath-toolong.c @@ -0,0 +1,49 @@ +/* Verify that realpath returns NULL with ENAMETOOLONG if the result exceeds + NAME_MAX. + Copyright The GNU Toolchain Authors. + 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 + <https://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <limits.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <support/check.h> +#include <support/temp_file.h> +#include <sys/types.h> +#include <sys/stat.h> + +#define BASENAME "tst-realpath-toolong." + +int +do_test (void) +{ + char *base = support_create_and_chdir_toolong_temp_directory (BASENAME); + + char buf[PATH_MAX + 1]; + const char *res = realpath (".", buf); + + /* canonicalize.c states that if the real path is >= PATH_MAX, then + realpath returns NULL and sets ENAMETOOLONG. */ + TEST_VERIFY (res == NULL); + TEST_VERIFY (errno == ENAMETOOLONG); + + free (base); + return 0; +} + +#include <support/test-driver.c> -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] 2022-01-20 9:32 ` [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] Siddhesh Poyarekar @ 2022-01-21 23:22 ` Joseph Myers 2022-01-22 3:47 ` Siddhesh Poyarekar 2022-01-22 14:45 ` [PATCH] tst-realpath-toolong: Fix hurd build Siddhesh Poyarekar 2022-01-24 13:45 ` [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] Andreas Schwab 1 sibling, 2 replies; 36+ messages in thread From: Joseph Myers @ 2022-01-21 23:22 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: libc-alpha, fweimer On Thu, 20 Jan 2022, Siddhesh Poyarekar via Libc-alpha wrote: > diff --git a/stdlib/tst-realpath-toolong.c b/stdlib/tst-realpath-toolong.c > + char buf[PATH_MAX + 1]; This has broken the testsuite build for Hurd. tst-realpath-toolong.c: In function 'do_test': tst-realpath-toolong.c:37:12: error: 'PATH_MAX' undeclared (first use in this function) -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] 2022-01-21 23:22 ` Joseph Myers @ 2022-01-22 3:47 ` Siddhesh Poyarekar 2022-01-22 14:45 ` [PATCH] tst-realpath-toolong: Fix hurd build Siddhesh Poyarekar 1 sibling, 0 replies; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-22 3:47 UTC (permalink / raw) To: Joseph Myers; +Cc: libc-alpha, fweimer On 22/01/2022 04:52, Joseph Myers wrote: > On Thu, 20 Jan 2022, Siddhesh Poyarekar via Libc-alpha wrote: > >> diff --git a/stdlib/tst-realpath-toolong.c b/stdlib/tst-realpath-toolong.c > >> + char buf[PATH_MAX + 1]; > > This has broken the testsuite build for Hurd. > > tst-realpath-toolong.c: In function 'do_test': > tst-realpath-toolong.c:37:12: error: 'PATH_MAX' undeclared (first use in this function) > Sorry I missed that, I'll post a fix. I was getting a different build error on hurd which prevented me from seeing this. I'm rebuilding my build-many-glibcs to see if that fixes it. Thanks, Siddhesh ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] tst-realpath-toolong: Fix hurd build 2022-01-21 23:22 ` Joseph Myers 2022-01-22 3:47 ` Siddhesh Poyarekar @ 2022-01-22 14:45 ` Siddhesh Poyarekar 2022-01-23 0:36 ` Samuel Thibault 1 sibling, 1 reply; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-22 14:45 UTC (permalink / raw) To: libc-alpha; +Cc: joseph We don't really need a bigger buffer for realpath since it should fail and return NULL. In the bug too, the buffer itself is not accessed; it is in fact left untouched. Drop the PATH_MAX use and pass a single char address. Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- stdlib/tst-realpath-toolong.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stdlib/tst-realpath-toolong.c b/stdlib/tst-realpath-toolong.c index 8bed772460..ed84787a32 100644 --- a/stdlib/tst-realpath-toolong.c +++ b/stdlib/tst-realpath-toolong.c @@ -34,8 +34,8 @@ do_test (void) { char *base = support_create_and_chdir_toolong_temp_directory (BASENAME); - char buf[PATH_MAX + 1]; - const char *res = realpath (".", buf); + char buf; + const char *res = realpath (".", &buf); /* canonicalize.c states that if the real path is >= PATH_MAX, then realpath returns NULL and sets ENAMETOOLONG. */ -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] tst-realpath-toolong: Fix hurd build 2022-01-22 14:45 ` [PATCH] tst-realpath-toolong: Fix hurd build Siddhesh Poyarekar @ 2022-01-23 0:36 ` Samuel Thibault 2022-01-23 15:19 ` Siddhesh Poyarekar 0 siblings, 1 reply; 36+ messages in thread From: Samuel Thibault @ 2022-01-23 0:36 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: libc-alpha, joseph Siddhesh Poyarekar via Libc-alpha, le sam. 22 janv. 2022 20:15:23 +0530, a ecrit: > We don't really need a bigger buffer for realpath since it should fail > and return NULL. In the bug too, the buffer itself is not accessed; it > is in fact left untouched. Drop the PATH_MAX use and pass a single char > address. ? realpath assumes that the passed buffer is PATH_MAX-long. When PATH_MAX is not defined, calling it with a buffer is essentially undefined. Better just pass NULL. > Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> > --- > stdlib/tst-realpath-toolong.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/stdlib/tst-realpath-toolong.c b/stdlib/tst-realpath-toolong.c > index 8bed772460..ed84787a32 100644 > --- a/stdlib/tst-realpath-toolong.c > +++ b/stdlib/tst-realpath-toolong.c > @@ -34,8 +34,8 @@ do_test (void) > { > char *base = support_create_and_chdir_toolong_temp_directory (BASENAME); > > - char buf[PATH_MAX + 1]; > - const char *res = realpath (".", buf); > + char buf; > + const char *res = realpath (".", &buf); > > /* canonicalize.c states that if the real path is >= PATH_MAX, then > realpath returns NULL and sets ENAMETOOLONG. */ > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] tst-realpath-toolong: Fix hurd build 2022-01-23 0:36 ` Samuel Thibault @ 2022-01-23 15:19 ` Siddhesh Poyarekar 2022-01-23 15:33 ` Samuel Thibault 0 siblings, 1 reply; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-23 15:19 UTC (permalink / raw) To: Samuel Thibault; +Cc: libc-alpha, joseph On 23/01/2022 06:06, Samuel Thibault wrote: > Siddhesh Poyarekar via Libc-alpha, le sam. 22 janv. 2022 20:15:23 +0530, a ecrit: >> We don't really need a bigger buffer for realpath since it should fail >> and return NULL. In the bug too, the buffer itself is not accessed; it >> is in fact left untouched. Drop the PATH_MAX use and pass a single char >> address. > > ? realpath assumes that the passed buffer is PATH_MAX-long. When > PATH_MAX is not defined, calling it with a buffer is essentially > undefined. Better just pass NULL. Passing NULL doesn't reproduce the problem because realpath just allocates enough to accommodate the return, even when it exceeds PATH_MAX. It only applies when a non-NULL buffer is supplied. Would you prefer it if I defined PATH_MAX on hurd then, something like: #ifndef PATH_MAX # define PATH_MAX 1024 #endif or do you prefer a more accurate path_max value using pathconf()? The former will be a simpler fix, the latter will be best served by a get_path_max support function, which will be more elaborate but accurate. I'm happy to do either. Thanks, Siddhesh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] tst-realpath-toolong: Fix hurd build 2022-01-23 15:19 ` Siddhesh Poyarekar @ 2022-01-23 15:33 ` Samuel Thibault 2022-01-24 6:00 ` [committed v2] " Siddhesh Poyarekar 0 siblings, 1 reply; 36+ messages in thread From: Samuel Thibault @ 2022-01-23 15:33 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: libc-alpha, joseph Siddhesh Poyarekar, le dim. 23 janv. 2022 20:49:48 +0530, a ecrit: > On 23/01/2022 06:06, Samuel Thibault wrote: > > Siddhesh Poyarekar via Libc-alpha, le sam. 22 janv. 2022 20:15:23 +0530, a ecrit: > > > We don't really need a bigger buffer for realpath since it should fail > > > and return NULL. In the bug too, the buffer itself is not accessed; it > > > is in fact left untouched. Drop the PATH_MAX use and pass a single char > > > address. > > > > ? realpath assumes that the passed buffer is PATH_MAX-long. When > > PATH_MAX is not defined, calling it with a buffer is essentially > > undefined. Better just pass NULL. > > Passing NULL doesn't reproduce the problem because realpath just allocates > enough to accommodate the return, even when it exceeds PATH_MAX. Ah, ok, sorry I didn't check the whole story. > Would you prefer it if I defined PATH_MAX on hurd then, something like: > > #ifndef PATH_MAX > # define PATH_MAX 1024 > #endif > > or do you prefer a more accurate path_max value using pathconf()? Better use the accurate from pathconf() ; that being said it will return -1 on the ext2fs filesystem, so we'll have to resort to a hardcoded limit in that case anyway. I'm fine with just setting PATH_MAX by hand here. Samuel ^ permalink raw reply [flat|nested] 36+ messages in thread
* [committed v2] tst-realpath-toolong: Fix hurd build 2022-01-23 15:33 ` Samuel Thibault @ 2022-01-24 6:00 ` Siddhesh Poyarekar 0 siblings, 0 replies; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-24 6:00 UTC (permalink / raw) To: libc-alpha; +Cc: Samuel Thibault Thanks, I've committed this after confirming that build-many-glibcs works for i686-gnu. -->8-- Define PATH_MAX to a constant if it isn't already defined, like in hurd. Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- stdlib/tst-realpath-toolong.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/stdlib/tst-realpath-toolong.c b/stdlib/tst-realpath-toolong.c index 8bed772460..4388890294 100644 --- a/stdlib/tst-realpath-toolong.c +++ b/stdlib/tst-realpath-toolong.c @@ -29,6 +29,10 @@ #define BASENAME "tst-realpath-toolong." +#ifndef PATH_MAX +# define PATH_MAX 1024 +#endif + int do_test (void) { -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] 2022-01-20 9:32 ` [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] Siddhesh Poyarekar 2022-01-21 23:22 ` Joseph Myers @ 2022-01-24 13:45 ` Andreas Schwab 2022-01-24 14:25 ` Siddhesh Poyarekar 1 sibling, 1 reply; 36+ messages in thread From: Andreas Schwab @ 2022-01-24 13:45 UTC (permalink / raw) To: Siddhesh Poyarekar via Libc-alpha; +Cc: Siddhesh Poyarekar, fweimer On Jan 20 2022, Siddhesh Poyarekar via Libc-alpha wrote: > diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c > index f36bdf4c76..732dc7ea46 100644 > --- a/stdlib/canonicalize.c > +++ b/stdlib/canonicalize.c > @@ -400,8 +400,16 @@ realpath_stk (const char *name, char *resolved, > > error: > *dest++ = '\0'; > - if (resolved != NULL && dest - rname <= get_path_max ()) > - rname = strcpy (resolved, rname); > + if (resolved != NULL) > + { > + if (dest - rname <= get_path_max ()) > + rname = strcpy (resolved, rname); > + else > + { > + failed = true; > + __set_errno (ENAMETOOLONG); > + } > + } Shouldn't that preserve any preexisting error? I think the result should only be copied if !failed. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] 2022-01-24 13:45 ` [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] Andreas Schwab @ 2022-01-24 14:25 ` Siddhesh Poyarekar 2022-01-24 14:48 ` Andreas Schwab 0 siblings, 1 reply; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-24 14:25 UTC (permalink / raw) To: Andreas Schwab, Siddhesh Poyarekar via Libc-alpha; +Cc: fweimer On 24/01/2022 19:15, Andreas Schwab wrote: > On Jan 20 2022, Siddhesh Poyarekar via Libc-alpha wrote: > >> diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c >> index f36bdf4c76..732dc7ea46 100644 >> --- a/stdlib/canonicalize.c >> +++ b/stdlib/canonicalize.c >> @@ -400,8 +400,16 @@ realpath_stk (const char *name, char *resolved, >> >> error: >> *dest++ = '\0'; >> - if (resolved != NULL && dest - rname <= get_path_max ()) >> - rname = strcpy (resolved, rname); >> + if (resolved != NULL) >> + { >> + if (dest - rname <= get_path_max ()) >> + rname = strcpy (resolved, rname); >> + else >> + { >> + failed = true; >> + __set_errno (ENAMETOOLONG); >> + } >> + } > > Shouldn't that preserve any preexisting error? like this? diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c index 732dc7ea46..6caed9e70e 100644 --- a/stdlib/canonicalize.c +++ b/stdlib/canonicalize.c @@ -404,7 +404,7 @@ error: { if (dest - rname <= get_path_max ()) rname = strcpy (resolved, rname); - else + else if (!failed) { failed = true; __set_errno (ENAMETOOLONG); > I think the result > should only be copied if !failed. > Looks like tests in test-canon.c depend on buf contents having being copied in despite failure. Siddhesh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] 2022-01-24 14:25 ` Siddhesh Poyarekar @ 2022-01-24 14:48 ` Andreas Schwab 2022-01-24 14:59 ` Siddhesh Poyarekar 0 siblings, 1 reply; 36+ messages in thread From: Andreas Schwab @ 2022-01-24 14:48 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: Siddhesh Poyarekar via Libc-alpha, fweimer On Jan 24 2022, Siddhesh Poyarekar wrote: > Looks like tests in test-canon.c depend on buf contents having being > copied in despite failure. That is not supported by POSIX. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] 2022-01-24 14:48 ` Andreas Schwab @ 2022-01-24 14:59 ` Siddhesh Poyarekar 2022-01-24 15:28 ` Andreas Schwab 0 siblings, 1 reply; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-24 14:59 UTC (permalink / raw) To: Andreas Schwab; +Cc: fweimer, Siddhesh Poyarekar via Libc-alpha On 24/01/2022 20:18, Andreas Schwab wrote: > On Jan 24 2022, Siddhesh Poyarekar wrote: > >> Looks like tests in test-canon.c depend on buf contents having being >> copied in despite failure. > > That is not supported by POSIX. > OK, I'll file a separate bug and fix for it then. Is the fix I posted sufficient for your other concern, i.e. not overriding preexisting error? Thanks, Siddhesh ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] 2022-01-24 14:59 ` Siddhesh Poyarekar @ 2022-01-24 15:28 ` Andreas Schwab 2022-01-24 16:08 ` [committed] realpath: Avoid overwriting preexisting error (CVE-2021-3998) Siddhesh Poyarekar 0 siblings, 1 reply; 36+ messages in thread From: Andreas Schwab @ 2022-01-24 15:28 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: fweimer, Siddhesh Poyarekar via Libc-alpha On Jan 24 2022, Siddhesh Poyarekar wrote: > Is the fix I posted sufficient for your other concern, i.e. not > overriding preexisting error? Yes. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 36+ messages in thread
* [committed] realpath: Avoid overwriting preexisting error (CVE-2021-3998) 2022-01-24 15:28 ` Andreas Schwab @ 2022-01-24 16:08 ` Siddhesh Poyarekar 0 siblings, 0 replies; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-24 16:08 UTC (permalink / raw) To: libc-alpha; +Cc: Andreas Schwab Set errno and failure for paths that are too long only if no other error occurred earlier. Related: BZ #28770 Reviewed-by: Andreas Schwab <schwab@linux-m68k.org> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- stdlib/canonicalize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c index 732dc7ea46..6caed9e70e 100644 --- a/stdlib/canonicalize.c +++ b/stdlib/canonicalize.c @@ -404,7 +404,7 @@ error: { if (dest - rname <= get_path_max ()) rname = strcpy (resolved, rname); - else + else if (!failed) { failed = true; __set_errno (ENAMETOOLONG); -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) 2022-01-20 9:32 ` [PATCH v3 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 Siddhesh Poyarekar 2022-01-20 9:32 ` [PATCH v3 1/3] support: Add helpers to create paths longer than PATH_MAX Siddhesh Poyarekar 2022-01-20 9:32 ` [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] Siddhesh Poyarekar @ 2022-01-20 9:32 ` Siddhesh Poyarekar 2022-01-20 13:32 ` Andreas Schwab 2022-01-21 16:41 ` Adhemerval Zanella 2 siblings, 2 replies; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-20 9:32 UTC (permalink / raw) To: libc-alpha; +Cc: eggert, adhemerval.zanella, fweimer, Qualys Security Advisory No valid path returned by getcwd would fit into 1 byte, so reject the size early and return NULL with errno set to ERANGE. This change is prompted by CVE-2021-3999, which describes a single byte buffer underflow and overflow when all of the following conditions are met: - The buffer size (i.e. the second argument of getcwd) is 1 byte - The current working directory is too long - '/' is also mounted on the current working directory Sequence of events: - In sysdeps/unix/sysv/linux/getcwd.c, the syscall returns ENAMETOOLONG because the linux kernel checks for name length before it checks buffer size - The code falls back to the generic getcwd in sysdeps/posix - In the generic func, the buf[0] is set to '\0' on line 250 - this while loop on line 262 is bypassed: while (!(thisdev == rootdev && thisino == rootino)) since the rootfs (/) is bind mounted onto the directory and the flow goes on to line 449, where it puts a '/' in the byte before the buffer. - Finally on line 458, it moves 2 bytes (the underflowed byte and the '\0') to the buf[0] and buf[1], resulting in a 1 byte buffer overflow. - buf is returned on line 469 and errno is not set. This resolves BZ #28769. Signed-off-by: Qualys Security Advisory <qsa@qualys.com> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- NEWS | 6 + sysdeps/posix/getcwd.c | 7 + sysdeps/unix/sysv/linux/Makefile | 7 +- .../unix/sysv/linux/tst-getcwd-smallbuff.c | 245 ++++++++++++++++++ 4 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c diff --git a/NEWS b/NEWS index 4c392a445e..07e9eac52d 100644 --- a/NEWS +++ b/NEWS @@ -170,6 +170,12 @@ Security related changes: function could result in a memory leak and potential access of uninitialized memory. Reported by Qualys. + CVE-2021-3999: Passing a buffer of size exactly 1 byte to the getcwd + function may result in an off-by-one buffer underflow and overflow + when the current working directory is longer than PATH_MAX and also + corresponds to the / directory through an unprivileged mount + namespace. Reported by Qualys. + The following bugs are resolved with this release: [The release manager will add the list generated by diff --git a/sysdeps/posix/getcwd.c b/sysdeps/posix/getcwd.c index e147a31a81..9d5787b6f4 100644 --- a/sysdeps/posix/getcwd.c +++ b/sysdeps/posix/getcwd.c @@ -187,6 +187,13 @@ __getcwd_generic (char *buf, size_t size) size_t allocated = size; size_t used; + /* A size of 1 byte is never useful. */ + if (allocated == 1) + { + __set_errno (ERANGE); + return NULL; + } + #if HAVE_MINIMALLY_WORKING_GETCWD /* If AT_FDCWD is not defined, the algorithm below is O(N**2) and this is much slower than the system getcwd (at least on diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index 85fc8cbf75..7ca9350c99 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -346,7 +346,12 @@ sysdep_routines += xstatconv internal_statvfs \ sysdep_headers += bits/fcntl-linux.h -tests += tst-fallocate tst-fallocate64 tst-o_path-locks +tests += \ + tst-fallocate \ + tst-fallocate64 \ + tst-getcwd-smallbuff \ + tst-o_path-locks \ +# tests endif ifeq ($(subdir),elf) diff --git a/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c new file mode 100644 index 0000000000..791dfe4d02 --- /dev/null +++ b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c @@ -0,0 +1,245 @@ +/* Verify that getcwd returns ERANGE for size 1 byte and does not underflow + buffer when the CWD is too long and is also a mount target of /. See bug + #28769 or CVE-2021-3999 for more context. + Copyright The GNU Toolchain Authors. + 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 + <https://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <fcntl.h> +#include <limits.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/mount.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <sys/wait.h> + +#include <sys/socket.h> +#include <sys/un.h> +#include <support/check.h> +#include <support/temp_file.h> +#include <support/xsched.h> +#include <support/xunistd.h> + +#ifndef PATH_MAX +# define PATH_MAX 1024 +#endif + +static char *base; +#define BASENAME "tst-getcwd-smallbuff" +#define MOUNT_NAME "mpoint" +static int sockfd[2]; + +static void +do_cleanup (void) +{ + support_chdir_toolong_temp_directory (base); + TEST_VERIFY_EXIT (rmdir (MOUNT_NAME) == 0); + free (base); +} + +static void +send_fd (const int sock, const int fd) +{ + struct msghdr msg; + union + { + struct cmsghdr hdr; + char buf[CMSG_SPACE (sizeof (int))]; + } cmsgbuf; + struct cmsghdr *cmsg; + struct iovec vec; + char ch = 'A'; + ssize_t n; + + memset (&msg, 0, sizeof (msg)); + memset (&cmsgbuf, 0, sizeof (cmsgbuf)); + msg.msg_control = &cmsgbuf.buf; + msg.msg_controllen = sizeof (cmsgbuf.buf); + + cmsg = CMSG_FIRSTHDR (&msg); + cmsg->cmsg_len = CMSG_LEN (sizeof (int)); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + *(int *) CMSG_DATA (cmsg) = fd; + + vec.iov_base = &ch; + vec.iov_len = 1; + msg.msg_iov = &vec; + msg.msg_iovlen = 1; + + while ((n = sendmsg (sock, &msg, 0)) == -1 && errno == EINTR); + + TEST_VERIFY_EXIT (n == 1); +} + +static int +recv_fd (const int sock) +{ + struct msghdr msg; + union + { + struct cmsghdr hdr; + char buf[CMSG_SPACE(sizeof(int))]; + } cmsgbuf; + struct cmsghdr *cmsg; + struct iovec vec; + ssize_t n; + char ch = '\0'; + int fd = -1; + + memset (&msg, 0, sizeof (msg)); + vec.iov_base = &ch; + vec.iov_len = 1; + msg.msg_iov = &vec; + msg.msg_iovlen = 1; + + memset (&cmsgbuf, 0, sizeof (cmsgbuf)); + msg.msg_control = &cmsgbuf.buf; + msg.msg_controllen = sizeof (cmsgbuf.buf); + + while ((n = recvmsg (sock, &msg, 0)) == -1 && errno == EINTR); + if (n != 1 || ch != 'A') + return -1; + + cmsg = CMSG_FIRSTHDR (&msg); + if (cmsg == NULL) + return -1; + if (cmsg->cmsg_type != SCM_RIGHTS) + return -1; + fd = *(const int *) CMSG_DATA (cmsg); + if (fd < 0) + return -1; + return fd; +} + +static int +child_func (void * const arg) +{ + xclose (sockfd[0]); + const int sock = sockfd[1]; + char ch; + + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); + TEST_VERIFY_EXIT (ch == '1'); + + if (mount ("/", MOUNT_NAME, NULL, MS_BIND | MS_REC, NULL)) + FAIL_EXIT1 ("mount failed: %m\n"); + const int fd = xopen ("mpoint", + O_RDONLY | O_PATH | O_DIRECTORY | O_NOFOLLOW, 0); + + send_fd (sock, fd); + xclose (fd); + + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); + TEST_VERIFY_EXIT (ch == 'a'); + + xclose (sock); + return 0; +} + +static void +update_map (char * const mapping, const char * const map_file) +{ + const size_t map_len = strlen (mapping); + + const int fd = xopen (map_file, O_WRONLY, 0); + xwrite (fd, mapping, map_len); + xclose (fd); +} + +static void +proc_setgroups_write (const long child_pid, const char * const str) +{ + const size_t str_len = strlen(str); + + char setgroups_path[64]; + snprintf (setgroups_path, sizeof (setgroups_path), + "/proc/%ld/setgroups", child_pid); + + const int fd = open (setgroups_path, O_WRONLY); + + if (fd < 0) + { + TEST_VERIFY_EXIT (errno == ENOENT); + FAIL_UNSUPPORTED ("/proc/%ld/setgroups not found\n", child_pid); + } + + xwrite (fd, str, str_len); + xclose(fd); +} + +static char child_stack[1024 * 1024]; + +int +do_test (void) +{ + base = support_create_and_chdir_toolong_temp_directory (BASENAME); + + xmkdir (MOUNT_NAME, S_IRWXU); + atexit (do_cleanup); + + TEST_VERIFY_EXIT (socketpair (AF_UNIX, SOCK_STREAM, 0, sockfd) == 0); + pid_t child_pid = xclone (child_func, NULL, child_stack, + sizeof (child_stack), + CLONE_NEWUSER | CLONE_NEWNS | SIGCHLD); + + xclose (sockfd[1]); + const int sock = sockfd[0]; + + char map_path[64], map_buf[64]; + snprintf (map_path, sizeof (map_path), "/proc/%ld/uid_map", + (long) child_pid); + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getuid()); + update_map (map_buf, map_path); + + proc_setgroups_write ((long) child_pid, "deny"); + snprintf (map_path, sizeof (map_path), "/proc/%ld/gid_map", + (long) child_pid); + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getgid()); + update_map (map_buf, map_path); + + TEST_VERIFY_EXIT (send (sock, "1", 1, MSG_NOSIGNAL) == 1); + const int fd = recv_fd (sock); + TEST_VERIFY_EXIT (fd >= 0); + TEST_VERIFY_EXIT (fchdir (fd) == 0); + + static char buf[2 * 10 + 1]; + memset (buf, 'A', sizeof(buf)); + + /* Finally, call getcwd and check if it resulted in a buffer underflow. */ + char * cwd = getcwd (buf + sizeof(buf) / 2, 1); + TEST_VERIFY (cwd == NULL); + TEST_VERIFY (errno == ERANGE); + + for (int i = 0; i < sizeof (buf); i++) + if (buf[i] != 'A') + { + printf ("buf[%d] = %02x\n", i, (unsigned int) buf[i]); + support_record_failure (); + } + + TEST_VERIFY_EXIT (send (sock, "a", 1, MSG_NOSIGNAL) == 1); + xclose (sock); + TEST_VERIFY_EXIT (xwaitpid (child_pid, NULL, 0) == child_pid); + + return 0; +} + +#define CLEANUP_HANDLER do_cleanup +#include <support/test-driver.c> -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) 2022-01-20 9:32 ` [PATCH v3 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) Siddhesh Poyarekar @ 2022-01-20 13:32 ` Andreas Schwab 2022-01-21 16:41 ` Adhemerval Zanella 1 sibling, 0 replies; 36+ messages in thread From: Andreas Schwab @ 2022-01-20 13:32 UTC (permalink / raw) To: Siddhesh Poyarekar via Libc-alpha Cc: Siddhesh Poyarekar, fweimer, Qualys Security Advisory Ok. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) 2022-01-20 9:32 ` [PATCH v3 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) Siddhesh Poyarekar 2022-01-20 13:32 ` Andreas Schwab @ 2022-01-21 16:41 ` Adhemerval Zanella 2022-01-21 17:26 ` Siddhesh Poyarekar ` (2 more replies) 1 sibling, 3 replies; 36+ messages in thread From: Adhemerval Zanella @ 2022-01-21 16:41 UTC (permalink / raw) To: Siddhesh Poyarekar, libc-alpha; +Cc: eggert, fweimer, Qualys Security Advisory On 20/01/2022 06:32, Siddhesh Poyarekar wrote: > No valid path returned by getcwd would fit into 1 byte, so reject the > size early and return NULL with errno set to ERANGE. This change is > prompted by CVE-2021-3999, which describes a single byte buffer > underflow and overflow when all of the following conditions are met: > > - The buffer size (i.e. the second argument of getcwd) is 1 byte > - The current working directory is too long > - '/' is also mounted on the current working directory > > Sequence of events: > > - In sysdeps/unix/sysv/linux/getcwd.c, the syscall returns ENAMETOOLONG > because the linux kernel checks for name length before it checks > buffer size > > - The code falls back to the generic getcwd in sysdeps/posix > > - In the generic func, the buf[0] is set to '\0' on line 250 > > - this while loop on line 262 is bypassed: > > while (!(thisdev == rootdev && thisino == rootino)) > > since the rootfs (/) is bind mounted onto the directory and the flow > goes on to line 449, where it puts a '/' in the byte before the > buffer. > > - Finally on line 458, it moves 2 bytes (the underflowed byte and the > '\0') to the buf[0] and buf[1], resulting in a 1 byte buffer overflow. > > - buf is returned on line 469 and errno is not set. > > This resolves BZ #28769. > > Signed-off-by: Qualys Security Advisory <qsa@qualys.com> > Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> Look good with just two fixed below for CMSG_DATA and a couple of comments. Ok with the fixes, the comments would be good but it is a blocker. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > NEWS | 6 + > sysdeps/posix/getcwd.c | 7 + > sysdeps/unix/sysv/linux/Makefile | 7 +- > .../unix/sysv/linux/tst-getcwd-smallbuff.c | 245 ++++++++++++++++++ > 4 files changed, 264 insertions(+), 1 deletion(-) > create mode 100644 sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c > > diff --git a/NEWS b/NEWS > index 4c392a445e..07e9eac52d 100644 > --- a/NEWS > +++ b/NEWS > @@ -170,6 +170,12 @@ Security related changes: > function could result in a memory leak and potential access of > uninitialized memory. Reported by Qualys. > > + CVE-2021-3999: Passing a buffer of size exactly 1 byte to the getcwd > + function may result in an off-by-one buffer underflow and overflow > + when the current working directory is longer than PATH_MAX and also > + corresponds to the / directory through an unprivileged mount > + namespace. Reported by Qualys. > + > The following bugs are resolved with this release: > > [The release manager will add the list generated by > diff --git a/sysdeps/posix/getcwd.c b/sysdeps/posix/getcwd.c > index e147a31a81..9d5787b6f4 100644 > --- a/sysdeps/posix/getcwd.c > +++ b/sysdeps/posix/getcwd.c > @@ -187,6 +187,13 @@ __getcwd_generic (char *buf, size_t size) > size_t allocated = size; > size_t used; > > + /* A size of 1 byte is never useful. */ > + if (allocated == 1) > + { > + __set_errno (ERANGE); > + return NULL; > + } > + > #if HAVE_MINIMALLY_WORKING_GETCWD > /* If AT_FDCWD is not defined, the algorithm below is O(N**2) and > this is much slower than the system getcwd (at least on > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile > index 85fc8cbf75..7ca9350c99 100644 > --- a/sysdeps/unix/sysv/linux/Makefile > +++ b/sysdeps/unix/sysv/linux/Makefile > @@ -346,7 +346,12 @@ sysdep_routines += xstatconv internal_statvfs \ > > sysdep_headers += bits/fcntl-linux.h > > -tests += tst-fallocate tst-fallocate64 tst-o_path-locks > +tests += \ > + tst-fallocate \ > + tst-fallocate64 \ > + tst-getcwd-smallbuff \ > + tst-o_path-locks \ > +# tests > endif > > ifeq ($(subdir),elf) > diff --git a/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c > new file mode 100644 > index 0000000000..791dfe4d02 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c > @@ -0,0 +1,245 @@ > +/* Verify that getcwd returns ERANGE for size 1 byte and does not underflow > + buffer when the CWD is too long and is also a mount target of /. See bug > + #28769 or CVE-2021-3999 for more context. > + Copyright The GNU Toolchain Authors. > + 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 > + <https://www.gnu.org/licenses/>. */ > + > +#include <errno.h> > +#include <fcntl.h> > +#include <limits.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <sys/mount.h> > +#include <sys/stat.h> > +#include <sys/types.h> > +#include <sys/wait.h> > + > +#include <sys/socket.h> > +#include <sys/un.h> > +#include <support/check.h> > +#include <support/temp_file.h> > +#include <support/xsched.h> > +#include <support/xunistd.h> > + > +#ifndef PATH_MAX > +# define PATH_MAX 1024 > +#endif No need since it is a Linux only test and PATH_MAX is always defined. > + > +static char *base; > +#define BASENAME "tst-getcwd-smallbuff" > +#define MOUNT_NAME "mpoint" > +static int sockfd[2]; > + > +static void > +do_cleanup (void) > +{ > + support_chdir_toolong_temp_directory (base); > + TEST_VERIFY_EXIT (rmdir (MOUNT_NAME) == 0); > + free (base); > +} > + > +static void > +send_fd (const int sock, const int fd) > +{ > + struct msghdr msg; > + union > + { > + struct cmsghdr hdr; > + char buf[CMSG_SPACE (sizeof (int))]; > + } cmsgbuf; Maybe zero-initialize both first to avoid the memset below? > + struct cmsghdr *cmsg; > + struct iovec vec; > + char ch = 'A'; > + ssize_t n; > + > + memset (&msg, 0, sizeof (msg)); > + memset (&cmsgbuf, 0, sizeof (cmsgbuf)); > + msg.msg_control = &cmsgbuf.buf; > + msg.msg_controllen = sizeof (cmsgbuf.buf); > + > + cmsg = CMSG_FIRSTHDR (&msg); > + cmsg->cmsg_len = CMSG_LEN (sizeof (int)); > + cmsg->cmsg_level = SOL_SOCKET; > + cmsg->cmsg_type = SCM_RIGHTS; > + *(int *) CMSG_DATA (cmsg) = fd; I think CMSG_DATA does not guarantee the alignment, so I think it would be safe to use memcpy here: memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd)); > + > + vec.iov_base = &ch; > + vec.iov_len = 1; > + msg.msg_iov = &vec; > + msg.msg_iovlen = 1; > + > + while ((n = sendmsg (sock, &msg, 0)) == -1 && errno == EINTR); > + > + TEST_VERIFY_EXIT (n == 1); > +} > + Ok. > +static int > +recv_fd (const int sock) > +{ > + struct msghdr msg; Maybe also zero-initialize here. > + union > + { > + struct cmsghdr hdr; > + char buf[CMSG_SPACE(sizeof(int))]; > + } cmsgbuf; > + struct cmsghdr *cmsg; > + struct iovec vec; > + ssize_t n; > + char ch = '\0'; > + int fd = -1; > + > + memset (&msg, 0, sizeof (msg)); > + vec.iov_base = &ch; > + vec.iov_len = 1; > + msg.msg_iov = &vec; > + msg.msg_iovlen = 1; > + > + memset (&cmsgbuf, 0, sizeof (cmsgbuf)); > + msg.msg_control = &cmsgbuf.buf; > + msg.msg_controllen = sizeof (cmsgbuf.buf); > + > + while ((n = recvmsg (sock, &msg, 0)) == -1 && errno == EINTR); > + if (n != 1 || ch != 'A') > + return -1; > + > + cmsg = CMSG_FIRSTHDR (&msg); > + if (cmsg == NULL) > + return -1; > + if (cmsg->cmsg_type != SCM_RIGHTS) > + return -1; > + fd = *(const int *) CMSG_DATA (cmsg); Same as before, I think you will need to copy to a temporary using memcpy. > + if (fd < 0) > + return -1; > + return fd; > +} > + > +static int > +child_func (void * const arg) > +{ > + xclose (sockfd[0]); > + const int sock = sockfd[1]; > + char ch; > + > + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); > + TEST_VERIFY_EXIT (ch == '1'); > + > + if (mount ("/", MOUNT_NAME, NULL, MS_BIND | MS_REC, NULL)) > + FAIL_EXIT1 ("mount failed: %m\n"); > + const int fd = xopen ("mpoint", > + O_RDONLY | O_PATH | O_DIRECTORY | O_NOFOLLOW, 0); > + > + send_fd (sock, fd); > + xclose (fd); > + > + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); > + TEST_VERIFY_EXIT (ch == 'a'); > + > + xclose (sock); > + return 0; > +} > + > +static void > +update_map (char * const mapping, const char * const map_file) > +{ > + const size_t map_len = strlen (mapping); > + > + const int fd = xopen (map_file, O_WRONLY, 0); > + xwrite (fd, mapping, map_len); > + xclose (fd); > +} > + > +static void > +proc_setgroups_write (const long child_pid, const char * const str) > +{ > + const size_t str_len = strlen(str); > + > + char setgroups_path[64]; Maybe define the size as: /* The path is the form /proc/%ld/setgroups. */ char map_path[sizeof("/proc/setgroups") + INT_STRLEN_BOUND (long int)]; > + snprintf (setgroups_path, sizeof (setgroups_path), > + "/proc/%ld/setgroups", child_pid); > + > + const int fd = open (setgroups_path, O_WRONLY); > + > + if (fd < 0) > + { > + TEST_VERIFY_EXIT (errno == ENOENT); > + FAIL_UNSUPPORTED ("/proc/%ld/setgroups not found\n", child_pid); > + } > + > + xwrite (fd, str, str_len); > + xclose(fd); > +} > + > +static char child_stack[1024 * 1024]; > + > +int > +do_test (void) > +{ > + base = support_create_and_chdir_toolong_temp_directory (BASENAME); > + > + xmkdir (MOUNT_NAME, S_IRWXU); > + atexit (do_cleanup); > + > + TEST_VERIFY_EXIT (socketpair (AF_UNIX, SOCK_STREAM, 0, sockfd) == 0); > + pid_t child_pid = xclone (child_func, NULL, child_stack, > + sizeof (child_stack), > + CLONE_NEWUSER | CLONE_NEWNS | SIGCHLD); > + > + xclose (sockfd[1]); > + const int sock = sockfd[0]; > + > + char map_path[64], map_buf[64]; Same comment as for setgroups_path. > + snprintf (map_path, sizeof (map_path), , > + (long) child_pid); > + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getuid()); > + update_map (map_buf, map_path); > + > + proc_setgroups_write ((long) child_pid, "deny"); > + snprintf (map_path, sizeof (map_path), "/proc/%ld/gid_map", > + (long) child_pid); > + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getgid()); > + update_map (map_buf, map_path); > + > + TEST_VERIFY_EXIT (send (sock, "1", 1, MSG_NOSIGNAL) == 1); > + const int fd = recv_fd (sock); > + TEST_VERIFY_EXIT (fd >= 0); > + TEST_VERIFY_EXIT (fchdir (fd) == 0); > + > + static char buf[2 * 10 + 1]; > + memset (buf, 'A', sizeof(buf)); Space before (. > + > + /* Finally, call getcwd and check if it resulted in a buffer underflow. */ > + char * cwd = getcwd (buf + sizeof(buf) / 2, 1); > + TEST_VERIFY (cwd == NULL); > + TEST_VERIFY (errno == ERANGE); > + > + for (int i = 0; i < sizeof (buf); i++) > + if (buf[i] != 'A') > + { > + printf ("buf[%d] = %02x\n", i, (unsigned int) buf[i]); > + support_record_failure (); > + } > + > + TEST_VERIFY_EXIT (send (sock, "a", 1, MSG_NOSIGNAL) == 1); > + xclose (sock); > + TEST_VERIFY_EXIT (xwaitpid (child_pid, NULL, 0) == child_pid); > + > + return 0; > +} > + > +#define CLEANUP_HANDLER do_cleanup > +#include <support/test-driver.c> ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) 2022-01-21 16:41 ` Adhemerval Zanella @ 2022-01-21 17:26 ` Siddhesh Poyarekar 2022-01-21 17:31 ` Adhemerval Zanella 2022-01-21 17:38 ` [PATCH v4] " Siddhesh Poyarekar 2022-01-21 18:04 ` [PATCH v5] " Siddhesh Poyarekar 2 siblings, 1 reply; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-21 17:26 UTC (permalink / raw) To: Adhemerval Zanella, libc-alpha; +Cc: eggert, fweimer, Qualys Security Advisory On 21/01/2022 22:11, Adhemerval Zanella wrote: > > > On 20/01/2022 06:32, Siddhesh Poyarekar wrote: >> No valid path returned by getcwd would fit into 1 byte, so reject the >> size early and return NULL with errno set to ERANGE. This change is >> prompted by CVE-2021-3999, which describes a single byte buffer >> underflow and overflow when all of the following conditions are met: >> >> - The buffer size (i.e. the second argument of getcwd) is 1 byte >> - The current working directory is too long >> - '/' is also mounted on the current working directory >> >> Sequence of events: >> >> - In sysdeps/unix/sysv/linux/getcwd.c, the syscall returns ENAMETOOLONG >> because the linux kernel checks for name length before it checks >> buffer size >> >> - The code falls back to the generic getcwd in sysdeps/posix >> >> - In the generic func, the buf[0] is set to '\0' on line 250 >> >> - this while loop on line 262 is bypassed: >> >> while (!(thisdev == rootdev && thisino == rootino)) >> >> since the rootfs (/) is bind mounted onto the directory and the flow >> goes on to line 449, where it puts a '/' in the byte before the >> buffer. >> >> - Finally on line 458, it moves 2 bytes (the underflowed byte and the >> '\0') to the buf[0] and buf[1], resulting in a 1 byte buffer overflow. >> >> - buf is returned on line 469 and errno is not set. >> >> This resolves BZ #28769. >> >> Signed-off-by: Qualys Security Advisory <qsa@qualys.com> >> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> > > Look good with just two fixed below for CMSG_DATA and a couple of comments. > Ok with the fixes, the comments would be good but it is a blocker. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > >> --- >> NEWS | 6 + >> sysdeps/posix/getcwd.c | 7 + >> sysdeps/unix/sysv/linux/Makefile | 7 +- >> .../unix/sysv/linux/tst-getcwd-smallbuff.c | 245 ++++++++++++++++++ >> 4 files changed, 264 insertions(+), 1 deletion(-) >> create mode 100644 sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c >> >> diff --git a/NEWS b/NEWS >> index 4c392a445e..07e9eac52d 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -170,6 +170,12 @@ Security related changes: >> function could result in a memory leak and potential access of >> uninitialized memory. Reported by Qualys. >> >> + CVE-2021-3999: Passing a buffer of size exactly 1 byte to the getcwd >> + function may result in an off-by-one buffer underflow and overflow >> + when the current working directory is longer than PATH_MAX and also >> + corresponds to the / directory through an unprivileged mount >> + namespace. Reported by Qualys. >> + >> The following bugs are resolved with this release: >> >> [The release manager will add the list generated by >> diff --git a/sysdeps/posix/getcwd.c b/sysdeps/posix/getcwd.c >> index e147a31a81..9d5787b6f4 100644 >> --- a/sysdeps/posix/getcwd.c >> +++ b/sysdeps/posix/getcwd.c >> @@ -187,6 +187,13 @@ __getcwd_generic (char *buf, size_t size) >> size_t allocated = size; >> size_t used; >> >> + /* A size of 1 byte is never useful. */ >> + if (allocated == 1) >> + { >> + __set_errno (ERANGE); >> + return NULL; >> + } >> + >> #if HAVE_MINIMALLY_WORKING_GETCWD >> /* If AT_FDCWD is not defined, the algorithm below is O(N**2) and >> this is much slower than the system getcwd (at least on >> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile >> index 85fc8cbf75..7ca9350c99 100644 >> --- a/sysdeps/unix/sysv/linux/Makefile >> +++ b/sysdeps/unix/sysv/linux/Makefile >> @@ -346,7 +346,12 @@ sysdep_routines += xstatconv internal_statvfs \ >> >> sysdep_headers += bits/fcntl-linux.h >> >> -tests += tst-fallocate tst-fallocate64 tst-o_path-locks >> +tests += \ >> + tst-fallocate \ >> + tst-fallocate64 \ >> + tst-getcwd-smallbuff \ >> + tst-o_path-locks \ >> +# tests >> endif >> >> ifeq ($(subdir),elf) >> diff --git a/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c >> new file mode 100644 >> index 0000000000..791dfe4d02 >> --- /dev/null >> +++ b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c >> @@ -0,0 +1,245 @@ >> +/* Verify that getcwd returns ERANGE for size 1 byte and does not underflow >> + buffer when the CWD is too long and is also a mount target of /. See bug >> + #28769 or CVE-2021-3999 for more context. >> + Copyright The GNU Toolchain Authors. >> + 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 >> + <https://www.gnu.org/licenses/>. */ >> + >> +#include <errno.h> >> +#include <fcntl.h> >> +#include <limits.h> >> +#include <stdio.h> >> +#include <stdlib.h> >> +#include <string.h> >> +#include <sys/mount.h> >> +#include <sys/stat.h> >> +#include <sys/types.h> >> +#include <sys/wait.h> >> + >> +#include <sys/socket.h> >> +#include <sys/un.h> >> +#include <support/check.h> >> +#include <support/temp_file.h> >> +#include <support/xsched.h> >> +#include <support/xunistd.h> >> + >> +#ifndef PATH_MAX >> +# define PATH_MAX 1024 >> +#endif > > No need since it is a Linux only test and PATH_MAX is always defined. Ahh, I didn't actually need it anyway, leftover from a previous iteration. I'll remove it. > >> + >> +static char *base; >> +#define BASENAME "tst-getcwd-smallbuff" >> +#define MOUNT_NAME "mpoint" >> +static int sockfd[2]; >> + >> +static void >> +do_cleanup (void) >> +{ >> + support_chdir_toolong_temp_directory (base); >> + TEST_VERIFY_EXIT (rmdir (MOUNT_NAME) == 0); >> + free (base); >> +} >> + >> +static void >> +send_fd (const int sock, const int fd) >> +{ >> + struct msghdr msg; >> + union >> + { >> + struct cmsghdr hdr; >> + char buf[CMSG_SPACE (sizeof (int))]; >> + } cmsgbuf; > > Maybe zero-initialize both first to avoid the memset below? OK. > >> + struct cmsghdr *cmsg; >> + struct iovec vec; >> + char ch = 'A'; >> + ssize_t n; >> + >> + memset (&msg, 0, sizeof (msg)); >> + memset (&cmsgbuf, 0, sizeof (cmsgbuf)); >> + msg.msg_control = &cmsgbuf.buf; >> + msg.msg_controllen = sizeof (cmsgbuf.buf); >> + >> + cmsg = CMSG_FIRSTHDR (&msg); >> + cmsg->cmsg_len = CMSG_LEN (sizeof (int)); >> + cmsg->cmsg_level = SOL_SOCKET; >> + cmsg->cmsg_type = SCM_RIGHTS; >> + *(int *) CMSG_DATA (cmsg) = fd; > > I think CMSG_DATA does not guarantee the alignment, so I think it would be > safe to use memcpy here: > > memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd)); OK. > >> + >> + vec.iov_base = &ch; >> + vec.iov_len = 1; >> + msg.msg_iov = &vec; >> + msg.msg_iovlen = 1; >> + >> + while ((n = sendmsg (sock, &msg, 0)) == -1 && errno == EINTR); >> + >> + TEST_VERIFY_EXIT (n == 1); >> +} >> + > > Ok. > >> +static int >> +recv_fd (const int sock) >> +{ >> + struct msghdr msg; > > Maybe also zero-initialize here. > >> + union >> + { >> + struct cmsghdr hdr; >> + char buf[CMSG_SPACE(sizeof(int))]; >> + } cmsgbuf; >> + struct cmsghdr *cmsg; >> + struct iovec vec; >> + ssize_t n; >> + char ch = '\0'; >> + int fd = -1; >> + >> + memset (&msg, 0, sizeof (msg)); >> + vec.iov_base = &ch; >> + vec.iov_len = 1; >> + msg.msg_iov = &vec; >> + msg.msg_iovlen = 1; >> + >> + memset (&cmsgbuf, 0, sizeof (cmsgbuf)); >> + msg.msg_control = &cmsgbuf.buf; >> + msg.msg_controllen = sizeof (cmsgbuf.buf); >> + >> + while ((n = recvmsg (sock, &msg, 0)) == -1 && errno == EINTR); >> + if (n != 1 || ch != 'A') >> + return -1; >> + >> + cmsg = CMSG_FIRSTHDR (&msg); >> + if (cmsg == NULL) >> + return -1; >> + if (cmsg->cmsg_type != SCM_RIGHTS) >> + return -1; >> + fd = *(const int *) CMSG_DATA (cmsg); > > Same as before, I think you will need to copy to a temporary using memcpy. Why not just: memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd)); i.e., is a temporary necessary? > >> + if (fd < 0) >> + return -1; >> + return fd; >> +} >> + >> +static int >> +child_func (void * const arg) >> +{ >> + xclose (sockfd[0]); >> + const int sock = sockfd[1]; >> + char ch; >> + >> + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); >> + TEST_VERIFY_EXIT (ch == '1'); >> + >> + if (mount ("/", MOUNT_NAME, NULL, MS_BIND | MS_REC, NULL)) >> + FAIL_EXIT1 ("mount failed: %m\n"); >> + const int fd = xopen ("mpoint", >> + O_RDONLY | O_PATH | O_DIRECTORY | O_NOFOLLOW, 0); >> + >> + send_fd (sock, fd); >> + xclose (fd); >> + >> + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); >> + TEST_VERIFY_EXIT (ch == 'a'); >> + >> + xclose (sock); >> + return 0; >> +} >> + >> +static void >> +update_map (char * const mapping, const char * const map_file) >> +{ >> + const size_t map_len = strlen (mapping); >> + >> + const int fd = xopen (map_file, O_WRONLY, 0); >> + xwrite (fd, mapping, map_len); >> + xclose (fd); >> +} >> + >> +static void >> +proc_setgroups_write (const long child_pid, const char * const str) >> +{ >> + const size_t str_len = strlen(str); >> + >> + char setgroups_path[64]; > > > Maybe define the size as: > > /* The path is the form /proc/%ld/setgroups. */ > char map_path[sizeof("/proc/setgroups") + INT_STRLEN_BOUND (long int)]; > OK. >> + snprintf (setgroups_path, sizeof (setgroups_path), >> + "/proc/%ld/setgroups", child_pid); >> + >> + const int fd = open (setgroups_path, O_WRONLY); >> + >> + if (fd < 0) >> + { >> + TEST_VERIFY_EXIT (errno == ENOENT); >> + FAIL_UNSUPPORTED ("/proc/%ld/setgroups not found\n", child_pid); >> + } >> + >> + xwrite (fd, str, str_len); >> + xclose(fd); >> +} >> + >> +static char child_stack[1024 * 1024]; >> + >> +int >> +do_test (void) >> +{ >> + base = support_create_and_chdir_toolong_temp_directory (BASENAME); >> + >> + xmkdir (MOUNT_NAME, S_IRWXU); >> + atexit (do_cleanup); >> + >> + TEST_VERIFY_EXIT (socketpair (AF_UNIX, SOCK_STREAM, 0, sockfd) == 0); >> + pid_t child_pid = xclone (child_func, NULL, child_stack, >> + sizeof (child_stack), >> + CLONE_NEWUSER | CLONE_NEWNS | SIGCHLD); >> + >> + xclose (sockfd[1]); >> + const int sock = sockfd[0]; >> + >> + char map_path[64], map_buf[64]; > > Same comment as for setgroups_path. > >> + snprintf (map_path, sizeof (map_path), , >> + (long) child_pid); >> + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getuid()); >> + update_map (map_buf, map_path); >> + >> + proc_setgroups_write ((long) child_pid, "deny"); >> + snprintf (map_path, sizeof (map_path), "/proc/%ld/gid_map", >> + (long) child_pid); >> + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getgid()); >> + update_map (map_buf, map_path); >> + >> + TEST_VERIFY_EXIT (send (sock, "1", 1, MSG_NOSIGNAL) == 1); >> + const int fd = recv_fd (sock); >> + TEST_VERIFY_EXIT (fd >= 0); >> + TEST_VERIFY_EXIT (fchdir (fd) == 0); >> + >> + static char buf[2 * 10 + 1]; >> + memset (buf, 'A', sizeof(buf)); > > > Space before (. > >> + >> + /* Finally, call getcwd and check if it resulted in a buffer underflow. */ >> + char * cwd = getcwd (buf + sizeof(buf) / 2, 1); >> + TEST_VERIFY (cwd == NULL); >> + TEST_VERIFY (errno == ERANGE); >> + >> + for (int i = 0; i < sizeof (buf); i++) >> + if (buf[i] != 'A') >> + { >> + printf ("buf[%d] = %02x\n", i, (unsigned int) buf[i]); >> + support_record_failure (); >> + } >> + >> + TEST_VERIFY_EXIT (send (sock, "a", 1, MSG_NOSIGNAL) == 1); >> + xclose (sock); >> + TEST_VERIFY_EXIT (xwaitpid (child_pid, NULL, 0) == child_pid); >> + >> + return 0; >> +} >> + >> +#define CLEANUP_HANDLER do_cleanup >> +#include <support/test-driver.c> > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) 2022-01-21 17:26 ` Siddhesh Poyarekar @ 2022-01-21 17:31 ` Adhemerval Zanella 0 siblings, 0 replies; 36+ messages in thread From: Adhemerval Zanella @ 2022-01-21 17:31 UTC (permalink / raw) To: Siddhesh Poyarekar, libc-alpha; +Cc: eggert, fweimer, Qualys Security Advisory On 21/01/2022 14:26, Siddhesh Poyarekar wrote: > On 21/01/2022 22:11, Adhemerval Zanella wrote: >> >> >> On 20/01/2022 06:32, Siddhesh Poyarekar wrote: >>> + cmsg = CMSG_FIRSTHDR (&msg); >>> + if (cmsg == NULL) >>> + return -1; >>> + if (cmsg->cmsg_type != SCM_RIGHTS) >>> + return -1; >>> + fd = *(const int *) CMSG_DATA (cmsg); >> >> Same as before, I think you will need to copy to a temporary using memcpy. > > Why not just: > > memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd)); > > i.e., is a temporary necessary? Not really, the above works just fine. ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v4] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) 2022-01-21 16:41 ` Adhemerval Zanella 2022-01-21 17:26 ` Siddhesh Poyarekar @ 2022-01-21 17:38 ` Siddhesh Poyarekar 2022-01-21 17:59 ` Andreas Schwab 2022-01-21 18:04 ` [PATCH v5] " Siddhesh Poyarekar 2 siblings, 1 reply; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-21 17:38 UTC (permalink / raw) To: libc-alpha; +Cc: Adhemerval Zanella, Qualys Security Advisory Fixed up, I'll push this if there are no further issues. Thanks, Siddhesh -->8-- No valid path returned by getcwd would fit into 1 byte, so reject the size early and return NULL with errno set to ERANGE. This change is prompted by CVE-2021-3999, which describes a single byte buffer underflow and overflow when all of the following conditions are met: - The buffer size (i.e. the second argument of getcwd) is 1 byte - The current working directory is too long - '/' is also mounted on the current working directory Sequence of events: - In sysdeps/unix/sysv/linux/getcwd.c, the syscall returns ENAMETOOLONG because the linux kernel checks for name length before it checks buffer size - The code falls back to the generic getcwd in sysdeps/posix - In the generic func, the buf[0] is set to '\0' on line 250 - this while loop on line 262 is bypassed: while (!(thisdev == rootdev && thisino == rootino)) since the rootfs (/) is bind mounted onto the directory and the flow goes on to line 449, where it puts a '/' in the byte before the buffer. - Finally on line 458, it moves 2 bytes (the underflowed byte and the '\0') to the buf[0] and buf[1], resulting in a 1 byte buffer overflow. - buf is returned on line 469 and errno is not set. This resolves BZ #28769. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> Signed-off-by: Qualys Security Advisory <qsa@qualys.com> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- NEWS | 6 + sysdeps/posix/getcwd.c | 7 + sysdeps/unix/sysv/linux/Makefile | 7 +- .../unix/sysv/linux/tst-getcwd-smallbuff.c | 243 ++++++++++++++++++ 4 files changed, 262 insertions(+), 1 deletion(-) create mode 100644 sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c diff --git a/NEWS b/NEWS index 4c392a445e..07e9eac52d 100644 --- a/NEWS +++ b/NEWS @@ -170,6 +170,12 @@ Security related changes: function could result in a memory leak and potential access of uninitialized memory. Reported by Qualys. + CVE-2021-3999: Passing a buffer of size exactly 1 byte to the getcwd + function may result in an off-by-one buffer underflow and overflow + when the current working directory is longer than PATH_MAX and also + corresponds to the / directory through an unprivileged mount + namespace. Reported by Qualys. + The following bugs are resolved with this release: [The release manager will add the list generated by diff --git a/sysdeps/posix/getcwd.c b/sysdeps/posix/getcwd.c index e147a31a81..9d5787b6f4 100644 --- a/sysdeps/posix/getcwd.c +++ b/sysdeps/posix/getcwd.c @@ -187,6 +187,13 @@ __getcwd_generic (char *buf, size_t size) size_t allocated = size; size_t used; + /* A size of 1 byte is never useful. */ + if (allocated == 1) + { + __set_errno (ERANGE); + return NULL; + } + #if HAVE_MINIMALLY_WORKING_GETCWD /* If AT_FDCWD is not defined, the algorithm below is O(N**2) and this is much slower than the system getcwd (at least on diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index 85fc8cbf75..7ca9350c99 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -346,7 +346,12 @@ sysdep_routines += xstatconv internal_statvfs \ sysdep_headers += bits/fcntl-linux.h -tests += tst-fallocate tst-fallocate64 tst-o_path-locks +tests += \ + tst-fallocate \ + tst-fallocate64 \ + tst-getcwd-smallbuff \ + tst-o_path-locks \ +# tests endif ifeq ($(subdir),elf) diff --git a/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c new file mode 100644 index 0000000000..212f9b94f3 --- /dev/null +++ b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c @@ -0,0 +1,243 @@ +/* Verify that getcwd returns ERANGE for size 1 byte and does not underflow + buffer when the CWD is too long and is also a mount target of /. See bug + #28769 or CVE-2021-3999 for more context. + Copyright The GNU Toolchain Authors. + 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 + <https://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <fcntl.h> +#include <intprops.h> +#include <limits.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/mount.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <sys/wait.h> + +#include <sys/socket.h> +#include <sys/un.h> +#include <support/check.h> +#include <support/temp_file.h> +#include <support/xsched.h> +#include <support/xunistd.h> + +static char *base; +#define BASENAME "tst-getcwd-smallbuff" +#define MOUNT_NAME "mpoint" +static int sockfd[2]; + +static void +do_cleanup (void) +{ + support_chdir_toolong_temp_directory (base); + TEST_VERIFY_EXIT (rmdir (MOUNT_NAME) == 0); + free (base); +} + +static void +send_fd (const int sock, const int fd) +{ + struct msghdr msg = {0}; + union + { + struct cmsghdr hdr; + char buf[CMSG_SPACE (sizeof (int))]; + } cmsgbuf = {0}; + struct cmsghdr *cmsg; + struct iovec vec; + char ch = 'A'; + ssize_t n; + + msg.msg_control = &cmsgbuf.buf; + msg.msg_controllen = sizeof (cmsgbuf.buf); + + cmsg = CMSG_FIRSTHDR (&msg); + cmsg->cmsg_len = CMSG_LEN (sizeof (int)); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd)); + + vec.iov_base = &ch; + vec.iov_len = 1; + msg.msg_iov = &vec; + msg.msg_iovlen = 1; + + while ((n = sendmsg (sock, &msg, 0)) == -1 && errno == EINTR); + + TEST_VERIFY_EXIT (n == 1); +} + +static int +recv_fd (const int sock) +{ + struct msghdr msg; + union + { + struct cmsghdr hdr; + char buf[CMSG_SPACE(sizeof(int))]; + } cmsgbuf; + struct cmsghdr *cmsg; + struct iovec vec; + ssize_t n; + char ch = '\0'; + int fd = -1; + + memset (&msg, 0, sizeof (msg)); + vec.iov_base = &ch; + vec.iov_len = 1; + msg.msg_iov = &vec; + msg.msg_iovlen = 1; + + memset (&cmsgbuf, 0, sizeof (cmsgbuf)); + msg.msg_control = &cmsgbuf.buf; + msg.msg_controllen = sizeof (cmsgbuf.buf); + + while ((n = recvmsg (sock, &msg, 0)) == -1 && errno == EINTR); + if (n != 1 || ch != 'A') + return -1; + + cmsg = CMSG_FIRSTHDR (&msg); + if (cmsg == NULL) + return -1; + if (cmsg->cmsg_type != SCM_RIGHTS) + return -1; + memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd)); + if (fd < 0) + return -1; + return fd; +} + +static int +child_func (void * const arg) +{ + xclose (sockfd[0]); + const int sock = sockfd[1]; + char ch; + + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); + TEST_VERIFY_EXIT (ch == '1'); + + if (mount ("/", MOUNT_NAME, NULL, MS_BIND | MS_REC, NULL)) + FAIL_EXIT1 ("mount failed: %m\n"); + const int fd = xopen ("mpoint", + O_RDONLY | O_PATH | O_DIRECTORY | O_NOFOLLOW, 0); + + send_fd (sock, fd); + xclose (fd); + + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); + TEST_VERIFY_EXIT (ch == 'a'); + + xclose (sock); + return 0; +} + +static void +update_map (char * const mapping, const char * const map_file) +{ + const size_t map_len = strlen (mapping); + + const int fd = xopen (map_file, O_WRONLY, 0); + xwrite (fd, mapping, map_len); + xclose (fd); +} + +static void +proc_setgroups_write (const long child_pid, const char * const str) +{ + const size_t str_len = strlen(str); + + char setgroups_path[sizeof ("/proc//setgroups") + INT_STRLEN_BOUND (long)]; + + snprintf (setgroups_path, sizeof (setgroups_path), + "/proc/%ld/setgroups", child_pid); + + const int fd = open (setgroups_path, O_WRONLY); + + if (fd < 0) + { + TEST_VERIFY_EXIT (errno == ENOENT); + FAIL_UNSUPPORTED ("/proc/%ld/setgroups not found\n", child_pid); + } + + xwrite (fd, str, str_len); + xclose(fd); +} + +static char child_stack[1024 * 1024]; + +int +do_test (void) +{ + base = support_create_and_chdir_toolong_temp_directory (BASENAME); + + xmkdir (MOUNT_NAME, S_IRWXU); + atexit (do_cleanup); + + TEST_VERIFY_EXIT (socketpair (AF_UNIX, SOCK_STREAM, 0, sockfd) == 0); + pid_t child_pid = xclone (child_func, NULL, child_stack, + sizeof (child_stack), + CLONE_NEWUSER | CLONE_NEWNS | SIGCHLD); + + xclose (sockfd[1]); + const int sock = sockfd[0]; + + char map_path[sizeof ("/proc//uid_map") + INT_STRLEN_BOUND (long)]; + char map_buf[sizeof ("0 1") + INT_STRLEN_BOUND (long)]; + + snprintf (map_path, sizeof (map_path), "/proc/%ld/uid_map", + (long) child_pid); + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getuid()); + update_map (map_buf, map_path); + + proc_setgroups_write ((long) child_pid, "deny"); + snprintf (map_path, sizeof (map_path), "/proc/%ld/gid_map", + (long) child_pid); + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getgid()); + update_map (map_buf, map_path); + + TEST_VERIFY_EXIT (send (sock, "1", 1, MSG_NOSIGNAL) == 1); + const int fd = recv_fd (sock); + TEST_VERIFY_EXIT (fd >= 0); + TEST_VERIFY_EXIT (fchdir (fd) == 0); + + static char buf[2 * 10 + 1]; + memset (buf, 'A', sizeof (buf)); + + /* Finally, call getcwd and check if it resulted in a buffer underflow. */ + char * cwd = getcwd (buf + sizeof (buf) / 2, 1); + TEST_VERIFY (cwd == NULL); + TEST_VERIFY (errno == ERANGE); + + for (int i = 0; i < sizeof (buf); i++) + if (buf[i] != 'A') + { + printf ("buf[%d] = %02x\n", i, (unsigned int) buf[i]); + support_record_failure (); + } + + TEST_VERIFY_EXIT (send (sock, "a", 1, MSG_NOSIGNAL) == 1); + xclose (sock); + TEST_VERIFY_EXIT (xwaitpid (child_pid, NULL, 0) == child_pid); + + return 0; +} + +#define CLEANUP_HANDLER do_cleanup +#include <support/test-driver.c> -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) 2022-01-21 17:38 ` [PATCH v4] " Siddhesh Poyarekar @ 2022-01-21 17:59 ` Andreas Schwab 2022-01-21 18:02 ` Siddhesh Poyarekar 0 siblings, 1 reply; 36+ messages in thread From: Andreas Schwab @ 2022-01-21 17:59 UTC (permalink / raw) To: Siddhesh Poyarekar via Libc-alpha Cc: Siddhesh Poyarekar, Qualys Security Advisory On Jan 21 2022, Siddhesh Poyarekar via Libc-alpha wrote: > +static int > +recv_fd (const int sock) > +{ > + struct msghdr msg; > + union > + { > + struct cmsghdr hdr; > + char buf[CMSG_SPACE(sizeof(int))]; > + } cmsgbuf; Zero-init like in send_fd. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) 2022-01-21 17:59 ` Andreas Schwab @ 2022-01-21 18:02 ` Siddhesh Poyarekar 0 siblings, 0 replies; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-21 18:02 UTC (permalink / raw) To: Andreas Schwab, Siddhesh Poyarekar via Libc-alpha Cc: Qualys Security Advisory On 21/01/2022 23:29, Andreas Schwab wrote: > On Jan 21 2022, Siddhesh Poyarekar via Libc-alpha wrote: > >> +static int >> +recv_fd (const int sock) >> +{ >> + struct msghdr msg; >> + union >> + { >> + struct cmsghdr hdr; >> + char buf[CMSG_SPACE(sizeof(int))]; >> + } cmsgbuf; > > Zero-init like in send_fd. > Thanks, I missed that. Sending v5. Siddhesh ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v5] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) 2022-01-21 16:41 ` Adhemerval Zanella 2022-01-21 17:26 ` Siddhesh Poyarekar 2022-01-21 17:38 ` [PATCH v4] " Siddhesh Poyarekar @ 2022-01-21 18:04 ` Siddhesh Poyarekar 2022-01-24 6:03 ` Siddhesh Poyarekar 2 siblings, 1 reply; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-21 18:04 UTC (permalink / raw) To: libc-alpha; +Cc: Andreas Schwab, Adhemerval Zanella, Qualys Security Advisory No valid path returned by getcwd would fit into 1 byte, so reject the size early and return NULL with errno set to ERANGE. This change is prompted by CVE-2021-3999, which describes a single byte buffer underflow and overflow when all of the following conditions are met: - The buffer size (i.e. the second argument of getcwd) is 1 byte - The current working directory is too long - '/' is also mounted on the current working directory Sequence of events: - In sysdeps/unix/sysv/linux/getcwd.c, the syscall returns ENAMETOOLONG because the linux kernel checks for name length before it checks buffer size - The code falls back to the generic getcwd in sysdeps/posix - In the generic func, the buf[0] is set to '\0' on line 250 - this while loop on line 262 is bypassed: while (!(thisdev == rootdev && thisino == rootino)) since the rootfs (/) is bind mounted onto the directory and the flow goes on to line 449, where it puts a '/' in the byte before the buffer. - Finally on line 458, it moves 2 bytes (the underflowed byte and the '\0') to the buf[0] and buf[1], resulting in a 1 byte buffer overflow. - buf is returned on line 469 and errno is not set. This resolves BZ #28769. Reviewed-by: Andreas Schwab <schwab@linux-m68k.org> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> Signed-off-by: Qualys Security Advisory <qsa@qualys.com> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- NEWS | 6 + sysdeps/posix/getcwd.c | 7 + sysdeps/unix/sysv/linux/Makefile | 7 +- .../unix/sysv/linux/tst-getcwd-smallbuff.c | 241 ++++++++++++++++++ 4 files changed, 260 insertions(+), 1 deletion(-) create mode 100644 sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c diff --git a/NEWS b/NEWS index 4c392a445e..07e9eac52d 100644 --- a/NEWS +++ b/NEWS @@ -170,6 +170,12 @@ Security related changes: function could result in a memory leak and potential access of uninitialized memory. Reported by Qualys. + CVE-2021-3999: Passing a buffer of size exactly 1 byte to the getcwd + function may result in an off-by-one buffer underflow and overflow + when the current working directory is longer than PATH_MAX and also + corresponds to the / directory through an unprivileged mount + namespace. Reported by Qualys. + The following bugs are resolved with this release: [The release manager will add the list generated by diff --git a/sysdeps/posix/getcwd.c b/sysdeps/posix/getcwd.c index e147a31a81..9d5787b6f4 100644 --- a/sysdeps/posix/getcwd.c +++ b/sysdeps/posix/getcwd.c @@ -187,6 +187,13 @@ __getcwd_generic (char *buf, size_t size) size_t allocated = size; size_t used; + /* A size of 1 byte is never useful. */ + if (allocated == 1) + { + __set_errno (ERANGE); + return NULL; + } + #if HAVE_MINIMALLY_WORKING_GETCWD /* If AT_FDCWD is not defined, the algorithm below is O(N**2) and this is much slower than the system getcwd (at least on diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index 85fc8cbf75..7ca9350c99 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -346,7 +346,12 @@ sysdep_routines += xstatconv internal_statvfs \ sysdep_headers += bits/fcntl-linux.h -tests += tst-fallocate tst-fallocate64 tst-o_path-locks +tests += \ + tst-fallocate \ + tst-fallocate64 \ + tst-getcwd-smallbuff \ + tst-o_path-locks \ +# tests endif ifeq ($(subdir),elf) diff --git a/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c new file mode 100644 index 0000000000..d460d6e766 --- /dev/null +++ b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c @@ -0,0 +1,241 @@ +/* Verify that getcwd returns ERANGE for size 1 byte and does not underflow + buffer when the CWD is too long and is also a mount target of /. See bug + #28769 or CVE-2021-3999 for more context. + Copyright The GNU Toolchain Authors. + 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 + <https://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <fcntl.h> +#include <intprops.h> +#include <limits.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/mount.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <sys/wait.h> + +#include <sys/socket.h> +#include <sys/un.h> +#include <support/check.h> +#include <support/temp_file.h> +#include <support/xsched.h> +#include <support/xunistd.h> + +static char *base; +#define BASENAME "tst-getcwd-smallbuff" +#define MOUNT_NAME "mpoint" +static int sockfd[2]; + +static void +do_cleanup (void) +{ + support_chdir_toolong_temp_directory (base); + TEST_VERIFY_EXIT (rmdir (MOUNT_NAME) == 0); + free (base); +} + +static void +send_fd (const int sock, const int fd) +{ + struct msghdr msg = {0}; + union + { + struct cmsghdr hdr; + char buf[CMSG_SPACE (sizeof (int))]; + } cmsgbuf = {0}; + struct cmsghdr *cmsg; + struct iovec vec; + char ch = 'A'; + ssize_t n; + + msg.msg_control = &cmsgbuf.buf; + msg.msg_controllen = sizeof (cmsgbuf.buf); + + cmsg = CMSG_FIRSTHDR (&msg); + cmsg->cmsg_len = CMSG_LEN (sizeof (int)); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd)); + + vec.iov_base = &ch; + vec.iov_len = 1; + msg.msg_iov = &vec; + msg.msg_iovlen = 1; + + while ((n = sendmsg (sock, &msg, 0)) == -1 && errno == EINTR); + + TEST_VERIFY_EXIT (n == 1); +} + +static int +recv_fd (const int sock) +{ + struct msghdr msg = {0}; + union + { + struct cmsghdr hdr; + char buf[CMSG_SPACE(sizeof(int))]; + } cmsgbuf = {0}; + struct cmsghdr *cmsg; + struct iovec vec; + ssize_t n; + char ch = '\0'; + int fd = -1; + + vec.iov_base = &ch; + vec.iov_len = 1; + msg.msg_iov = &vec; + msg.msg_iovlen = 1; + + msg.msg_control = &cmsgbuf.buf; + msg.msg_controllen = sizeof (cmsgbuf.buf); + + while ((n = recvmsg (sock, &msg, 0)) == -1 && errno == EINTR); + if (n != 1 || ch != 'A') + return -1; + + cmsg = CMSG_FIRSTHDR (&msg); + if (cmsg == NULL) + return -1; + if (cmsg->cmsg_type != SCM_RIGHTS) + return -1; + memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd)); + if (fd < 0) + return -1; + return fd; +} + +static int +child_func (void * const arg) +{ + xclose (sockfd[0]); + const int sock = sockfd[1]; + char ch; + + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); + TEST_VERIFY_EXIT (ch == '1'); + + if (mount ("/", MOUNT_NAME, NULL, MS_BIND | MS_REC, NULL)) + FAIL_EXIT1 ("mount failed: %m\n"); + const int fd = xopen ("mpoint", + O_RDONLY | O_PATH | O_DIRECTORY | O_NOFOLLOW, 0); + + send_fd (sock, fd); + xclose (fd); + + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); + TEST_VERIFY_EXIT (ch == 'a'); + + xclose (sock); + return 0; +} + +static void +update_map (char * const mapping, const char * const map_file) +{ + const size_t map_len = strlen (mapping); + + const int fd = xopen (map_file, O_WRONLY, 0); + xwrite (fd, mapping, map_len); + xclose (fd); +} + +static void +proc_setgroups_write (const long child_pid, const char * const str) +{ + const size_t str_len = strlen(str); + + char setgroups_path[sizeof ("/proc//setgroups") + INT_STRLEN_BOUND (long)]; + + snprintf (setgroups_path, sizeof (setgroups_path), + "/proc/%ld/setgroups", child_pid); + + const int fd = open (setgroups_path, O_WRONLY); + + if (fd < 0) + { + TEST_VERIFY_EXIT (errno == ENOENT); + FAIL_UNSUPPORTED ("/proc/%ld/setgroups not found\n", child_pid); + } + + xwrite (fd, str, str_len); + xclose(fd); +} + +static char child_stack[1024 * 1024]; + +int +do_test (void) +{ + base = support_create_and_chdir_toolong_temp_directory (BASENAME); + + xmkdir (MOUNT_NAME, S_IRWXU); + atexit (do_cleanup); + + TEST_VERIFY_EXIT (socketpair (AF_UNIX, SOCK_STREAM, 0, sockfd) == 0); + pid_t child_pid = xclone (child_func, NULL, child_stack, + sizeof (child_stack), + CLONE_NEWUSER | CLONE_NEWNS | SIGCHLD); + + xclose (sockfd[1]); + const int sock = sockfd[0]; + + char map_path[sizeof ("/proc//uid_map") + INT_STRLEN_BOUND (long)]; + char map_buf[sizeof ("0 1") + INT_STRLEN_BOUND (long)]; + + snprintf (map_path, sizeof (map_path), "/proc/%ld/uid_map", + (long) child_pid); + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getuid()); + update_map (map_buf, map_path); + + proc_setgroups_write ((long) child_pid, "deny"); + snprintf (map_path, sizeof (map_path), "/proc/%ld/gid_map", + (long) child_pid); + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getgid()); + update_map (map_buf, map_path); + + TEST_VERIFY_EXIT (send (sock, "1", 1, MSG_NOSIGNAL) == 1); + const int fd = recv_fd (sock); + TEST_VERIFY_EXIT (fd >= 0); + TEST_VERIFY_EXIT (fchdir (fd) == 0); + + static char buf[2 * 10 + 1]; + memset (buf, 'A', sizeof (buf)); + + /* Finally, call getcwd and check if it resulted in a buffer underflow. */ + char * cwd = getcwd (buf + sizeof (buf) / 2, 1); + TEST_VERIFY (cwd == NULL); + TEST_VERIFY (errno == ERANGE); + + for (int i = 0; i < sizeof (buf); i++) + if (buf[i] != 'A') + { + printf ("buf[%d] = %02x\n", i, (unsigned int) buf[i]); + support_record_failure (); + } + + TEST_VERIFY_EXIT (send (sock, "a", 1, MSG_NOSIGNAL) == 1); + xclose (sock); + TEST_VERIFY_EXIT (xwaitpid (child_pid, NULL, 0) == child_pid); + + return 0; +} + +#define CLEANUP_HANDLER do_cleanup +#include <support/test-driver.c> -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v5] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) 2022-01-21 18:04 ` [PATCH v5] " Siddhesh Poyarekar @ 2022-01-24 6:03 ` Siddhesh Poyarekar 0 siblings, 0 replies; 36+ messages in thread From: Siddhesh Poyarekar @ 2022-01-24 6:03 UTC (permalink / raw) To: libc-alpha; +Cc: Qualys Security Advisory, Andreas Schwab On 21/01/2022 23:34, Siddhesh Poyarekar via Libc-alpha wrote: > No valid path returned by getcwd would fit into 1 byte, so reject the > size early and return NULL with errno set to ERANGE. This change is > prompted by CVE-2021-3999, which describes a single byte buffer > underflow and overflow when all of the following conditions are met: > > - The buffer size (i.e. the second argument of getcwd) is 1 byte > - The current working directory is too long > - '/' is also mounted on the current working directory > > Sequence of events: > > - In sysdeps/unix/sysv/linux/getcwd.c, the syscall returns ENAMETOOLONG > because the linux kernel checks for name length before it checks > buffer size > > - The code falls back to the generic getcwd in sysdeps/posix > > - In the generic func, the buf[0] is set to '\0' on line 250 > > - this while loop on line 262 is bypassed: > > while (!(thisdev == rootdev && thisino == rootino)) > > since the rootfs (/) is bind mounted onto the directory and the flow > goes on to line 449, where it puts a '/' in the byte before the > buffer. > > - Finally on line 458, it moves 2 bytes (the underflowed byte and the > '\0') to the buf[0] and buf[1], resulting in a 1 byte buffer overflow. > > - buf is returned on line 469 and errno is not set. > > This resolves BZ #28769. > > Reviewed-by: Andreas Schwab <schwab@linux-m68k.org> > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > Signed-off-by: Qualys Security Advisory <qsa@qualys.com> > Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> Pushed now since updates were trivial and Adhemerval and Andreas had already acked the rest. Thanks, Siddhesh > --- > NEWS | 6 + > sysdeps/posix/getcwd.c | 7 + > sysdeps/unix/sysv/linux/Makefile | 7 +- > .../unix/sysv/linux/tst-getcwd-smallbuff.c | 241 ++++++++++++++++++ > 4 files changed, 260 insertions(+), 1 deletion(-) > create mode 100644 sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c > > diff --git a/NEWS b/NEWS > index 4c392a445e..07e9eac52d 100644 > --- a/NEWS > +++ b/NEWS > @@ -170,6 +170,12 @@ Security related changes: > function could result in a memory leak and potential access of > uninitialized memory. Reported by Qualys. > > + CVE-2021-3999: Passing a buffer of size exactly 1 byte to the getcwd > + function may result in an off-by-one buffer underflow and overflow > + when the current working directory is longer than PATH_MAX and also > + corresponds to the / directory through an unprivileged mount > + namespace. Reported by Qualys. > + > The following bugs are resolved with this release: > > [The release manager will add the list generated by > diff --git a/sysdeps/posix/getcwd.c b/sysdeps/posix/getcwd.c > index e147a31a81..9d5787b6f4 100644 > --- a/sysdeps/posix/getcwd.c > +++ b/sysdeps/posix/getcwd.c > @@ -187,6 +187,13 @@ __getcwd_generic (char *buf, size_t size) > size_t allocated = size; > size_t used; > > + /* A size of 1 byte is never useful. */ > + if (allocated == 1) > + { > + __set_errno (ERANGE); > + return NULL; > + } > + > #if HAVE_MINIMALLY_WORKING_GETCWD > /* If AT_FDCWD is not defined, the algorithm below is O(N**2) and > this is much slower than the system getcwd (at least on > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile > index 85fc8cbf75..7ca9350c99 100644 > --- a/sysdeps/unix/sysv/linux/Makefile > +++ b/sysdeps/unix/sysv/linux/Makefile > @@ -346,7 +346,12 @@ sysdep_routines += xstatconv internal_statvfs \ > > sysdep_headers += bits/fcntl-linux.h > > -tests += tst-fallocate tst-fallocate64 tst-o_path-locks > +tests += \ > + tst-fallocate \ > + tst-fallocate64 \ > + tst-getcwd-smallbuff \ > + tst-o_path-locks \ > +# tests > endif > > ifeq ($(subdir),elf) > diff --git a/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c > new file mode 100644 > index 0000000000..d460d6e766 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c > @@ -0,0 +1,241 @@ > +/* Verify that getcwd returns ERANGE for size 1 byte and does not underflow > + buffer when the CWD is too long and is also a mount target of /. See bug > + #28769 or CVE-2021-3999 for more context. > + Copyright The GNU Toolchain Authors. > + 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 > + <https://www.gnu.org/licenses/>. */ > + > +#include <errno.h> > +#include <fcntl.h> > +#include <intprops.h> > +#include <limits.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <sys/mount.h> > +#include <sys/stat.h> > +#include <sys/types.h> > +#include <sys/wait.h> > + > +#include <sys/socket.h> > +#include <sys/un.h> > +#include <support/check.h> > +#include <support/temp_file.h> > +#include <support/xsched.h> > +#include <support/xunistd.h> > + > +static char *base; > +#define BASENAME "tst-getcwd-smallbuff" > +#define MOUNT_NAME "mpoint" > +static int sockfd[2]; > + > +static void > +do_cleanup (void) > +{ > + support_chdir_toolong_temp_directory (base); > + TEST_VERIFY_EXIT (rmdir (MOUNT_NAME) == 0); > + free (base); > +} > + > +static void > +send_fd (const int sock, const int fd) > +{ > + struct msghdr msg = {0}; > + union > + { > + struct cmsghdr hdr; > + char buf[CMSG_SPACE (sizeof (int))]; > + } cmsgbuf = {0}; > + struct cmsghdr *cmsg; > + struct iovec vec; > + char ch = 'A'; > + ssize_t n; > + > + msg.msg_control = &cmsgbuf.buf; > + msg.msg_controllen = sizeof (cmsgbuf.buf); > + > + cmsg = CMSG_FIRSTHDR (&msg); > + cmsg->cmsg_len = CMSG_LEN (sizeof (int)); > + cmsg->cmsg_level = SOL_SOCKET; > + cmsg->cmsg_type = SCM_RIGHTS; > + memcpy (CMSG_DATA (cmsg), &fd, sizeof (fd)); > + > + vec.iov_base = &ch; > + vec.iov_len = 1; > + msg.msg_iov = &vec; > + msg.msg_iovlen = 1; > + > + while ((n = sendmsg (sock, &msg, 0)) == -1 && errno == EINTR); > + > + TEST_VERIFY_EXIT (n == 1); > +} > + > +static int > +recv_fd (const int sock) > +{ > + struct msghdr msg = {0}; > + union > + { > + struct cmsghdr hdr; > + char buf[CMSG_SPACE(sizeof(int))]; > + } cmsgbuf = {0}; > + struct cmsghdr *cmsg; > + struct iovec vec; > + ssize_t n; > + char ch = '\0'; > + int fd = -1; > + > + vec.iov_base = &ch; > + vec.iov_len = 1; > + msg.msg_iov = &vec; > + msg.msg_iovlen = 1; > + > + msg.msg_control = &cmsgbuf.buf; > + msg.msg_controllen = sizeof (cmsgbuf.buf); > + > + while ((n = recvmsg (sock, &msg, 0)) == -1 && errno == EINTR); > + if (n != 1 || ch != 'A') > + return -1; > + > + cmsg = CMSG_FIRSTHDR (&msg); > + if (cmsg == NULL) > + return -1; > + if (cmsg->cmsg_type != SCM_RIGHTS) > + return -1; > + memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd)); > + if (fd < 0) > + return -1; > + return fd; > +} > + > +static int > +child_func (void * const arg) > +{ > + xclose (sockfd[0]); > + const int sock = sockfd[1]; > + char ch; > + > + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); > + TEST_VERIFY_EXIT (ch == '1'); > + > + if (mount ("/", MOUNT_NAME, NULL, MS_BIND | MS_REC, NULL)) > + FAIL_EXIT1 ("mount failed: %m\n"); > + const int fd = xopen ("mpoint", > + O_RDONLY | O_PATH | O_DIRECTORY | O_NOFOLLOW, 0); > + > + send_fd (sock, fd); > + xclose (fd); > + > + TEST_VERIFY_EXIT (read (sock, &ch, 1) == 1); > + TEST_VERIFY_EXIT (ch == 'a'); > + > + xclose (sock); > + return 0; > +} > + > +static void > +update_map (char * const mapping, const char * const map_file) > +{ > + const size_t map_len = strlen (mapping); > + > + const int fd = xopen (map_file, O_WRONLY, 0); > + xwrite (fd, mapping, map_len); > + xclose (fd); > +} > + > +static void > +proc_setgroups_write (const long child_pid, const char * const str) > +{ > + const size_t str_len = strlen(str); > + > + char setgroups_path[sizeof ("/proc//setgroups") + INT_STRLEN_BOUND (long)]; > + > + snprintf (setgroups_path, sizeof (setgroups_path), > + "/proc/%ld/setgroups", child_pid); > + > + const int fd = open (setgroups_path, O_WRONLY); > + > + if (fd < 0) > + { > + TEST_VERIFY_EXIT (errno == ENOENT); > + FAIL_UNSUPPORTED ("/proc/%ld/setgroups not found\n", child_pid); > + } > + > + xwrite (fd, str, str_len); > + xclose(fd); > +} > + > +static char child_stack[1024 * 1024]; > + > +int > +do_test (void) > +{ > + base = support_create_and_chdir_toolong_temp_directory (BASENAME); > + > + xmkdir (MOUNT_NAME, S_IRWXU); > + atexit (do_cleanup); > + > + TEST_VERIFY_EXIT (socketpair (AF_UNIX, SOCK_STREAM, 0, sockfd) == 0); > + pid_t child_pid = xclone (child_func, NULL, child_stack, > + sizeof (child_stack), > + CLONE_NEWUSER | CLONE_NEWNS | SIGCHLD); > + > + xclose (sockfd[1]); > + const int sock = sockfd[0]; > + > + char map_path[sizeof ("/proc//uid_map") + INT_STRLEN_BOUND (long)]; > + char map_buf[sizeof ("0 1") + INT_STRLEN_BOUND (long)]; > + > + snprintf (map_path, sizeof (map_path), "/proc/%ld/uid_map", > + (long) child_pid); > + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getuid()); > + update_map (map_buf, map_path); > + > + proc_setgroups_write ((long) child_pid, "deny"); > + snprintf (map_path, sizeof (map_path), "/proc/%ld/gid_map", > + (long) child_pid); > + snprintf (map_buf, sizeof (map_buf), "0 %ld 1", (long) getgid()); > + update_map (map_buf, map_path); > + > + TEST_VERIFY_EXIT (send (sock, "1", 1, MSG_NOSIGNAL) == 1); > + const int fd = recv_fd (sock); > + TEST_VERIFY_EXIT (fd >= 0); > + TEST_VERIFY_EXIT (fchdir (fd) == 0); > + > + static char buf[2 * 10 + 1]; > + memset (buf, 'A', sizeof (buf)); > + > + /* Finally, call getcwd and check if it resulted in a buffer underflow. */ > + char * cwd = getcwd (buf + sizeof (buf) / 2, 1); > + TEST_VERIFY (cwd == NULL); > + TEST_VERIFY (errno == ERANGE); > + > + for (int i = 0; i < sizeof (buf); i++) > + if (buf[i] != 'A') > + { > + printf ("buf[%d] = %02x\n", i, (unsigned int) buf[i]); > + support_record_failure (); > + } > + > + TEST_VERIFY_EXIT (send (sock, "a", 1, MSG_NOSIGNAL) == 1); > + xclose (sock); > + TEST_VERIFY_EXIT (xwaitpid (child_pid, NULL, 0) == child_pid); > + > + return 0; > +} > + > +#define CLEANUP_HANDLER do_cleanup > +#include <support/test-driver.c> ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2022-01-24 16:18 UTC | newest] Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-19 8:21 [PATCH v2 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 Siddhesh Poyarekar 2022-01-19 8:21 ` [PATCH v2 1/3] support: Add helpers to create paths longer than PATH_MAX Siddhesh Poyarekar 2022-01-19 10:13 ` Siddhesh Poyarekar 2022-01-20 14:47 ` Cristian Rodríguez 2022-01-20 14:56 ` Florian Weimer 2022-01-20 15:17 ` Cristian Rodríguez 2022-01-20 14:57 ` Siddhesh Poyarekar 2022-01-19 8:21 ` [PATCH v2 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX (CVE-2021-3998) Siddhesh Poyarekar 2022-01-19 8:21 ` [PATCH v2 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) Siddhesh Poyarekar 2022-01-20 9:32 ` [PATCH v3 0/3] Fixes for CVE-2021-3998 and CVE-2021-3999 Siddhesh Poyarekar 2022-01-20 9:32 ` [PATCH v3 1/3] support: Add helpers to create paths longer than PATH_MAX Siddhesh Poyarekar 2022-01-21 16:27 ` Adhemerval Zanella 2022-01-20 9:32 ` [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] Siddhesh Poyarekar 2022-01-21 23:22 ` Joseph Myers 2022-01-22 3:47 ` Siddhesh Poyarekar 2022-01-22 14:45 ` [PATCH] tst-realpath-toolong: Fix hurd build Siddhesh Poyarekar 2022-01-23 0:36 ` Samuel Thibault 2022-01-23 15:19 ` Siddhesh Poyarekar 2022-01-23 15:33 ` Samuel Thibault 2022-01-24 6:00 ` [committed v2] " Siddhesh Poyarekar 2022-01-24 13:45 ` [PATCH v3 2/3] realpath: Set errno to ENAMETOOLONG for result larger than PATH_MAX [BZ #28770] Andreas Schwab 2022-01-24 14:25 ` Siddhesh Poyarekar 2022-01-24 14:48 ` Andreas Schwab 2022-01-24 14:59 ` Siddhesh Poyarekar 2022-01-24 15:28 ` Andreas Schwab 2022-01-24 16:08 ` [committed] realpath: Avoid overwriting preexisting error (CVE-2021-3998) Siddhesh Poyarekar 2022-01-20 9:32 ` [PATCH v3 3/3] getcwd: Set errno to ERANGE for size == 1 (CVE-2021-3999) Siddhesh Poyarekar 2022-01-20 13:32 ` Andreas Schwab 2022-01-21 16:41 ` Adhemerval Zanella 2022-01-21 17:26 ` Siddhesh Poyarekar 2022-01-21 17:31 ` Adhemerval Zanella 2022-01-21 17:38 ` [PATCH v4] " Siddhesh Poyarekar 2022-01-21 17:59 ` Andreas Schwab 2022-01-21 18:02 ` Siddhesh Poyarekar 2022-01-21 18:04 ` [PATCH v5] " Siddhesh Poyarekar 2022-01-24 6:03 ` Siddhesh Poyarekar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).