From ed6a68bac7cd056abda9008019c71b167f0362dc Mon Sep 17 00:00:00 2001 From: Adhemerval Zanella Date: Fri, 14 Mar 2025 16:09:57 -0300 Subject: 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 --- stdio-common/vfprintf-internal.c | 9 ++++----- stdio-common/vfprintf-process-arg.c | 28 ++++++++++++++++++---------- 2 files changed, 22 insertions(+), 15 deletions(-) (limited to 'stdio-common') diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c index aa9708bff5..fa41e1b242 100644 --- a/stdio-common/vfprintf-internal.c +++ b/stdio-common/vfprintf-internal.c @@ -576,7 +576,8 @@ static const uint8_t jump_table[] = /* Handle positional format specifiers. */ static void printf_positional (struct Xprintf_buffer *buf, - const CHAR_T *format, int readonly_format, + const CHAR_T *format, + enum readonly_error_type readonly_format, va_list ap, va_list *ap_savep, int nspecs_done, const UCHAR_T *lead_str_end, CHAR_T *work_buffer, int save_errno, @@ -626,9 +627,7 @@ Xprintf_buffer (struct Xprintf_buffer *buf, const CHAR_T *format, /* For the %m format we may need the current `errno' value. */ int save_errno = errno; - /* 1 if format is in read-only memory, -1 if it is in writable memory, - 0 if unknown. */ - int readonly_format = 0; + enum readonly_error_type readonly_format = readonly_noerror; /* Initialize local variables. */ grouping = (const char *) -1; @@ -1045,7 +1044,7 @@ do_positional: static void printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format, - int readonly_format, + enum readonly_error_type readonly_format, va_list ap, va_list *ap_savep, int nspecs_done, const UCHAR_T *lead_str_end, CHAR_T *work_buffer, int save_errno, diff --git a/stdio-common/vfprintf-process-arg.c b/stdio-common/vfprintf-process-arg.c index 8d20493766..90b5e61ceb 100644 --- a/stdio-common/vfprintf-process-arg.c +++ b/stdio-common/vfprintf-process-arg.c @@ -324,16 +324,24 @@ LABEL (form_pointer): LABEL (form_number): if ((mode_flags & PRINTF_FORTIFY) != 0) { - if (! readonly_format) - { - extern int __readonly_area (const void *, size_t) - attribute_hidden; - readonly_format - = __readonly_area (format, ((STR_LEN (format) + 1) - * sizeof (CHAR_T))); - } - if (readonly_format < 0) - __libc_fatal ("*** %n in writable segment detected ***\n"); + if (readonly_format == readonly_noerror) + readonly_format = __readonly_area (format, ((STR_LEN (format) + 1) + * sizeof (CHAR_T))); + switch (readonly_format) + { + case readonly_area_writable: + __libc_fatal ("*** %n in writable segments detected ***\n"); + /* The format is not within ELF segments and opening /proc/self/maps + failed because there are too many files. */ + case readonly_procfs_open_fail: + __libc_fatal ("*** procfs could not open ***\n"); + /* The /proc/self/maps can not be opened either because it is not + available or the process does not have the right permission. Since + it should not be attacker-controlled we can avoid failure. */ + case readonly_procfs_inaccessible: + case readonly_noerror: + break; + } } /* Answer the count of characters written. */ void *ptrptr = process_arg_pointer (); -- cgit v1.2.3