diff options
| author | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2025-03-14 16:09:57 -0300 |
|---|---|---|
| committer | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2025-03-21 15:46:48 -0300 |
| commit | ed6a68bac7cd056abda9008019c71b167f0362dc (patch) | |
| tree | 7ceb7f6403f423e4773724c518e537e13f140a3d /debug | |
| parent | 1894e219dc530d7074085e95ffe3c1e66cebc072 (diff) | |
| download | glibc-ed6a68bac7cd056abda9008019c71b167f0362dc.tar.xz glibc-ed6a68bac7cd056abda9008019c71b167f0362dc.zip | |
debug: Improve '%n' fortify detection (BZ 30932)
The 7bb8045ec0 path made the '%n' fortify check ignore EMFILE errors
while trying to open /proc/self/maps, and this added a security
issue where EMFILE can be attacker-controlled thus making it
ineffective for some cases.
The EMFILE failure is reinstated but with a different error
message. Also, to improve the false positive of the hardening for
the cases where no new files can be opened, the
_dl_readonly_area now uses _dl_find_object to check if the
memory area is within a writable ELF segment. The procfs method is
still used as fallback.
Checked on x86_64-linux-gnu and i686-linux-gnu.
Reviewed-by: Arjun Shankar <arjun@redhat.com>
Diffstat (limited to 'debug')
| -rw-r--r-- | debug/Makefile | 12 | ||||
| -rw-r--r-- | debug/readonly-area.c | 23 | ||||
| -rw-r--r-- | debug/tst-sprintf-fortify-rdonly-dlopen.c | 1 | ||||
| -rw-r--r-- | debug/tst-sprintf-fortify-rdonly-mod.c | 56 | ||||
| -rw-r--r-- | debug/tst-sprintf-fortify-rdonly.c | 158 |
5 files changed, 221 insertions, 29 deletions
diff --git a/debug/Makefile b/debug/Makefile index 905f2bf7e0..2484580cd2 100644 --- a/debug/Makefile +++ b/debug/Makefile @@ -75,6 +75,7 @@ routines = \ readlink_chk \ readlinkat_chk \ readonly-area \ + readonly-area-fallback \ realpath_chk \ recv_chk \ recvfrom_chk \ @@ -180,9 +181,15 @@ CPPFLAGS-tst-longjmp_chk3.c += $(no-fortify-source) -D_FORTIFY_SOURCE=1 CPPFLAGS-tst-realpath-chk.c += $(no-fortify-source) -D_FORTIFY_SOURCE=2 CPPFLAGS-tst-chk-cancel.c += $(no-fortify-source) -D_FORTIFY_SOURCE=2 CFLAGS-tst-sprintf-fortify-rdonly.c += $(no-fortify-source) -D_FORTIFY_SOURCE=2 +CFLAGS-tst-sprintf-fortify-rdonly-mod.c += $(no-fortify-source) -D_FORTIFY_SOURCE=2 +CFLAGS-tst-sprintf-fortify-rdonly-dlopen.c += $(no-fortify-source) -D_FORTIFY_SOURCE=2 CFLAGS-tst-fortify-syslog.c += $(no-fortify-source) -D_FORTIFY_SOURCE=2 CFLAGS-tst-fortify-wide.c += $(no-fortify-source) -D_FORTIFY_SOURCE=2 +$(objpfx)tst-sprintf-fortify-rdonly: \ + $(objpfx)tst-sprintf-fortify-rdonly-mod.so \ + $(objpfx)tst-sprintf-fortify-rdonly-dlopen.so + # _FORTIFY_SOURCE tests. # Auto-generate tests for _FORTIFY_SOURCE for different levels, compilers and # preprocessor conditions based on tst-fortify.c. @@ -303,6 +310,11 @@ tests-container += \ tst-fortify-syslog \ # tests-container +modules-names += \ + tst-sprintf-fortify-rdonly-dlopen \ + tst-sprintf-fortify-rdonly-mod \ + # modules-names + ifeq ($(have-ssp),yes) tests += tst-ssp-1 endif diff --git a/debug/readonly-area.c b/debug/readonly-area.c index 04b437ed8d..4311b8214a 100644 --- a/debug/readonly-area.c +++ b/debug/readonly-area.c @@ -16,18 +16,19 @@ <https://www.gnu.org/licenses/>. */ #include <stdlib.h> +#include <ldsodefs.h> -/* Return 1 if the whole area PTR .. PTR+SIZE is not writable. - Return -1 if it is writable. */ - -int +enum readonly_error_type __readonly_area (const void *ptr, size_t size) { - /* We cannot determine in general whether memory is writable or not. - This must be handled in a system-dependent manner. to not - unconditionally break code we need to return here a positive - answer. This disables this security measure but that is the - price people have to pay for using systems without a real - implementation of this interface. */ - return 1; + switch (GLRO(dl_readonly_area (ptr, size))) + { + case dl_readonly_area_rdonly: + return readonly_noerror; + case dl_readonly_area_writable: + return readonly_area_writable; + default: + break; + } + return __readonly_area_fallback (ptr, size); } diff --git a/debug/tst-sprintf-fortify-rdonly-dlopen.c b/debug/tst-sprintf-fortify-rdonly-dlopen.c new file mode 100644 index 0000000000..7da3f51f16 --- /dev/null +++ b/debug/tst-sprintf-fortify-rdonly-dlopen.c @@ -0,0 +1 @@ +#include "tst-sprintf-fortify-rdonly-mod.c" diff --git a/debug/tst-sprintf-fortify-rdonly-mod.c b/debug/tst-sprintf-fortify-rdonly-mod.c new file mode 100644 index 0000000000..3655f27b32 --- /dev/null +++ b/debug/tst-sprintf-fortify-rdonly-mod.c @@ -0,0 +1,56 @@ +/* Testcase for BZ 30932. + Copyright (C) 2025 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 + <https://www.gnu.org/licenses/>. */ + +#include <stdio.h> +#include <string.h> +#include <stdlib.h> + +static const char *str2 = "F"; +static char writeable_format[10] = "%s"; +static char relro_format[10] __attribute__ ((section (".data.rel.ro"))) = + "%s%n%s%n"; + +void +init_writable (void) +{ + strcpy (writeable_format + 2, "%n%s%n"); +} + +int +sprintf_writable (int *n1, int *n2) +{ + char buf[128]; + return sprintf (buf, writeable_format, str2, n1, str2, n2); +} + +int +sprintf_relro (int *n1, int *n2) +{ + char buf[128]; + return sprintf (buf, relro_format, str2, n1, str2, n2); +} + +int +sprintf_writable_malloc (int *n1, int *n2) +{ + char buf[128]; + char *buf2_malloc = strdup (writeable_format); + if (buf2_malloc == NULL) + abort (); + return sprintf (buf, buf2_malloc, str2, n1, str2, n2); +} diff --git a/debug/tst-sprintf-fortify-rdonly.c b/debug/tst-sprintf-fortify-rdonly.c index ba5a791020..fafc8340ea 100644 --- a/debug/tst-sprintf-fortify-rdonly.c +++ b/debug/tst-sprintf-fortify-rdonly.c @@ -27,16 +27,64 @@ #include <support/check.h> #include <support/support.h> #include <support/temp_file.h> +#include <support/xdlfcn.h> -jmp_buf chk_fail_buf; -bool chk_fail_ok; +static sigjmp_buf chk_fail_buf; +static volatile int ret; +static bool chk_fail_ok; -const char *str2 = "F"; -char buf2[10] = "%s"; +static void +handler (int sig) +{ + if (chk_fail_ok) + { + chk_fail_ok = false; + longjmp (chk_fail_buf, 1); + } + else + _exit (127); +} + +#define FORTIFY_FAIL \ + do { printf ("Failure on line %d\n", __LINE__); ret = 1; } while (0) +#define CHK_FAIL_START \ + chk_fail_ok = true; \ + if (! sigsetjmp (chk_fail_buf, 1)) \ + { +#define CHK_FAIL_END \ + chk_fail_ok = false; \ + FORTIFY_FAIL; \ + } + +static const char *str2 = "F"; +static char writeable_format[10] = "%s"; +static char relro_format[10] __attribute__ ((section (".data.rel.ro"))) = + "%s%n%s%n"; + +extern void init_writable (void); +extern int sprintf_writable (int *, int *); +extern int sprintf_relro (int *, int *); +extern int sprintf_writable_malloc (int *, int *); + +#define str(__x) # __x +void (*init_writable_dlopen)(void); +int (*sprintf_writable_dlopen)(int *, int *); +int (*sprintf_rdonly_dlopen)(int *, int *); +int (*sprintf_writable_malloc_dlopen)(int *, int *); static int do_test (void) { + set_fortify_handler (handler); + + { + void *h = xdlopen ("tst-sprintf-fortify-rdonly-dlopen.so", RTLD_NOW); + init_writable_dlopen = xdlsym (h, str(init_writable)); + sprintf_writable_dlopen = xdlsym (h, str(sprintf_writable)); + sprintf_rdonly_dlopen = xdlsym (h, str(sprintf_relro)); + sprintf_writable_malloc_dlopen = xdlsym (h, str(sprintf_writable_malloc)); + } + struct rlimit rl; int max_fd = 24; @@ -63,20 +111,94 @@ do_test (void) } TEST_VERIFY_EXIT (nfiles != 0); - /* When the format string is writable and contains %n, - with -D_FORTIFY_SOURCE=2 it causes __chk_fail. However, if libc can not - open procfs to check if the input format string in within a writable - memory segment, the fortify version can not perform the check. */ - char buf[128]; - int n1; - int n2; - - strcpy (buf2 + 2, "%n%s%n"); - if (sprintf (buf, buf2, str2, &n1, str2, &n2) != 2 - || n1 != 1 || n2 != 2) - FAIL_EXIT1 ("sprintf failed: %s %d %d", buf, n1, n2); - - return 0; + strcpy (writeable_format + 2, "%n%s%n"); + init_writable (); + init_writable_dlopen (); + + /* writeable_format is at a writable part of .bss segment, so libc should be + able to check it without resorting to procfs. */ + { + char buf[128]; + int n1; + int n2; + CHK_FAIL_START + sprintf (buf, writeable_format, str2, &n1, str2, &n2); + CHK_FAIL_END + } + + /* Same as before, but from an library. */ + { + int n1; + int n2; + CHK_FAIL_START + sprintf_writable (&n1, &n2); + CHK_FAIL_END + } + + { + int n1; + int n2; + CHK_FAIL_START + sprintf_writable_dlopen (&n1, &n2); + CHK_FAIL_END + } + + /* relro_format is at a readonly part of .bss segment, so '%n' in format input + should not trigger a fortify failure. */ + { + char buf[128]; + int n1; + int n2; + if (sprintf (buf, relro_format, str2, &n1, str2, &n2) != 2 + || n1 != 1 || n2 != 2) + FAIL_EXIT1 ("sprintf failed: %s %d %d", buf, n1, n2); + } + + /* Same as before, but from an library. */ + { + int n1; + int n2; + if (sprintf_relro (&n1, &n2) != 2 || n1 != 1 || n2 != 2) + FAIL_EXIT1 ("sprintf failed: %d %d", n1, n2); + } + + { + int n1; + int n2; + if (sprintf_rdonly_dlopen (&n1, &n2) != 2 || n1 != 1 || n2 != 2) + FAIL_EXIT1 ("sprintf failed: %d %d", n1, n2); + } + + /* However if the format string is placed on a writable memory not covered + by ELF segments, libc needs to resort to procfs. */ + { + char buf[128]; + int n1; + int n2; + char *buf2_malloc = xstrdup (writeable_format); + CHK_FAIL_START + sprintf (buf, buf2_malloc, str2, &n1, str2, &n2); + CHK_FAIL_END + } + + /* Same as before, but from an library. */ + { + int n1; + int n2; + CHK_FAIL_START + sprintf_writable_malloc (&n1, &n2); + CHK_FAIL_END + } + + { + int n1; + int n2; + CHK_FAIL_START + sprintf_writable_malloc_dlopen (&n1, &n2); + CHK_FAIL_END + } + + return ret; } #include <support/test-driver.c> |
