Fix path length overflow in realpath (BZ#22786)
Commit Message
Greetings,
Attached is a trivial fix, and a test case.
Thanks,
2018-04-09 Paul Pluzhnikov <ppluzhnikov@google.com>
[BZ #22786]
* stdlib/canonicalize.c (__realpath): Fix overflow in path length
computation.
* stdlib/Makefile (test-bz22786): New test.
* stdlib/test-bz22786.c: New test.
Comments
On Mon, 9 Apr 2018, Paul Pluzhnikov wrote:
> +# suppress warnings about allocation size.
> +CFLAGS-test-bz22786.c += $(+gcc-nowarn)
I see no current uses of $(+gcc-nowarn) in the source tree. Rather than
resurrecting this variable, we should remove it. Warnings should be
disabled as locally as possible in the sources - meaning using DIAG_* with
appropriate detailed comments around the specific statements generating
warnings, if possible, and failing that, with specific -Wno-* options in
the makefile rather than the extremely blunt -w which would also hide all
valid warnings, not just the one warning you intend to avoid on the one
bit of code for which you intend to avoid it.
On Mon, Apr 09, 2018 at 11:01:50PM +0000, Paul Pluzhnikov wrote:
> Greetings,
>
> Attached is a trivial fix, and a test case.
Paul, could you format commit messages in a git friendly way
(the first line is a terse summary, the second line is empty,
the third line starts a descriptive text) like others do, please?
On Wed, May 9, 2018 at 4:11 AM Dmitry V. Levin <ldv@altlinux.org> wrote:
> Paul, could you format commit messages in a git friendly way
> (the first line is a terse summary, the second line is empty,
> the third line starts a descriptive text) like others do, please?
Will do. Sorry about that.
@@ -84,7 +84,7 @@ tests := tst-strtol tst-strtod testmb testrand testsort testdiv \
tst-cxa_atexit tst-on_exit test-atexit-race \
test-at_quick_exit-race test-cxa_atexit-race \
test-on_exit-race test-dlclose-exit-race \
- tst-makecontext-align
+ tst-makecontext-align test-bz22786
tests-internal := tst-strtod1i tst-strtod3 tst-strtod4 tst-strtod5i \
tst-tls-atexit tst-tls-atexit-nodelete
@@ -156,6 +156,9 @@ CFLAGS-tst-qsort.c += $(stack-align-test-flags)
CFLAGS-tst-makecontext.c += -funwind-tables
CFLAGS-tst-makecontext2.c += $(stack-align-test-flags)
+# suppress warnings about allocation size.
+CFLAGS-test-bz22786.c += $(+gcc-nowarn)
+
# Run a test on the header files we use.
tests-special += $(objpfx)isomac.out
@@ -181,7 +181,7 @@ __realpath (const char *name, char *resolved)
extra_buf = __alloca (path_max);
len = strlen (end);
- if ((long int) (n + len) >= path_max)
+ if (path_max - n <= len)
{
__set_errno (ENAMETOOLONG);
goto error;
new file mode 100644
@@ -0,0 +1,80 @@
+/* Bug 22786: test for stack overflow in realpath.
+ Copyright (C) 2017-2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* This file must be run from within a directory called "stdlib". */
+
+#include <errno.h>
+#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+static int
+do_test (void)
+{
+ const char dir[] = "bz22786";
+ const char lnk[] = "bz22786/symlink";
+
+ rmdir (dir);
+ if (mkdir (dir, 0755) != 0 && errno != EEXIST)
+ {
+ printf ("mkdir %s: %m\n", dir);
+ return EXIT_FAILURE;
+ }
+ if (symlink (".", lnk) != 0 && errno != EEXIST)
+ {
+ printf ("symlink (%s, %s): %m\n", dir, lnk);
+ return EXIT_FAILURE;
+ }
+
+ const size_t path_len = (size_t) INT_MAX + 1;
+ char *path = malloc(path_len);
+
+ if (path == NULL)
+ {
+ printf ("malloc (%zu): %m\n", path_len);
+ return EXIT_FAILURE;
+ }
+
+ /* Construct very long path = "bz22786/symlink/aaaa....." */
+ char *p = mempcpy (path, lnk, sizeof (lnk) - 1);
+ *(p++) = '/';
+ memset (p, 'a', path_len - (path - p) - 2);
+ p[path_len - (path - p) - 1] = '\0';
+
+ /* This call crashes before the fix for bz22786 on 32-bit platforms. */
+ p = realpath (path, NULL);
+
+ if (p != NULL || errno != ENAMETOOLONG)
+ {
+ printf ("realpath: %s (%m)", p);
+ return EXIT_FAILURE;
+ }
+
+ /* Cleanup. */
+ unlink (lnk);
+ rmdir (dir);
+
+ return 0;
+}
+
+#define TEST_FUNCTION do_test
+#include <support/test-driver.c>