From 760fb390fd4ce506abd401e8a75fc0a510b82d48 Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Thu, 3 Aug 2023 07:29:58 +0930 Subject: [PATCH] objdump, nm: sprintf sanitizer null destination pointer Seen on Ubuntu 23.04 x86_64-linux using gcc-12.2 and gcc-12.3 with CFLAGS="-m32 -g -O2 -fsanitize=address,undefined". CC objdump.o In file included from /usr/include/stdio.h:906, from /home/alan/src/binutils-gdb/binutils/sysdep.h:24, from /home/alan/src/binutils-gdb/binutils/objdump.c:51: In function 'sprintf', inlined from 'display_utf8' at /home/alan/src/binutils-gdb/binutils/objdump.c:621:14, inlined from 'sanitize_string.part.0' at /home/alan/src/binutils-gdb/binutils/objdump.c:742:11: /usr/include/bits/stdio2.h:30:10: error: null destination pointer [-Werror=format-overflow=] 30 | return __builtin___sprintf_chk (__s, __USE_FORTIFY_LEVEL - 1, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 31 | __glibc_objsize (__s), __fmt, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 32 | __va_arg_pack ()); | ~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors The warning is bogus of course. xmalloc is guaranteed to return non-NULL, but apparently this isn't seen in display_utf6. The same doesn't happen with -m64, maybe due to inlining differences, I haven't investigated fully. Easily avoided as we hardly need to use sprintf for a single char, or a two char string. * objdump.c (display_utf8): Avoid bogus sprintf sanitizer warning. Use hex ESC to switch back to default colour. (sanitize_string): Comment. Bump buffer size by one. Fix overlong line. * nm.c (display_utf8, sanitize_string): As above. --- binutils/nm.c | 29 +++++++++++++++++------------ binutils/objdump.c | 25 +++++++++++++++---------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/binutils/nm.c b/binutils/nm.c index f96cfa31cb9..e4c8036df1b 100644 --- a/binutils/nm.c +++ b/binutils/nm.c @@ -541,13 +541,14 @@ display_utf8 (const unsigned char * in, char * out, unsigned int * consumed) case unicode_invalid: case unicode_hex: - out += sprintf (out, "%c", unicode_display == unicode_hex ? '<' : '{'); - out += sprintf (out, "0x"); + *out++ = unicode_display == unicode_hex ? '<' : '{'; + *out++ = '0'; + *out++ = 'x'; for (j = 0; j < nchars; j++) out += sprintf (out, "%02x", in [j]); - out += sprintf (out, "%c", unicode_display == unicode_hex ? '>' : '}'); + *out++ = unicode_display == unicode_hex ? '>' : '}'; break; - + case unicode_highlight: if (isatty (1)) out += sprintf (out, "\x1B[31;47m"); /* Red. */ @@ -557,7 +558,7 @@ display_utf8 (const unsigned char * in, char * out, unsigned int * consumed) { case 2: out += sprintf (out, "\\u%02x%02x", - ((in[0] & 0x1c) >> 2), + ((in[0] & 0x1c) >> 2), ((in[0] & 0x03) << 6) | (in[1] & 0x3f)); break; @@ -579,7 +580,7 @@ display_utf8 (const unsigned char * in, char * out, unsigned int * consumed) } if (unicode_display == unicode_highlight && isatty (1)) - out += sprintf (out, "\033[0m"); /* Default colour. */ + out += sprintf (out, "\x1B[0m"); /* Default colour. */ break; default: @@ -633,11 +634,15 @@ convert_utf8 (const char * in) /* Copy the input, translating as needed. */ in = original; - if (buffer_len < (strlen (in) * 9)) + /* For 2 char unicode, max out is 12 (colour escapes) + 6, ie. 9 per in + For hex, max out is 8 for 2 char unicode, ie. 4 per in. + 3 and 4 char unicode produce less output for input. */ + size_t max_needed = strlen (in) * 9 + 1; + if (buffer_len < max_needed) { - free ((void *) buffer); - buffer_len = strlen (in) * 9; - buffer = xmalloc (buffer_len + 1); + buffer_len = max_needed; + free (buffer); + buffer = xmalloc (buffer_len); } out = buffer; @@ -657,8 +662,8 @@ convert_utf8 (const char * in) { unsigned int num_consumed; - out += display_utf8 ((const unsigned char *)(in - 1), out, & num_consumed); - in += num_consumed - 1; + out += display_utf8 ((const unsigned char *) --in, out, &num_consumed); + in += num_consumed; } else *out++ = c; diff --git a/binutils/objdump.c b/binutils/objdump.c index a35982ea969..fb0db5d4fe8 100644 --- a/binutils/objdump.c +++ b/binutils/objdump.c @@ -617,11 +617,12 @@ display_utf8 (const unsigned char * in, char * out, unsigned int * consumed) case unicode_invalid: case unicode_hex: - out += sprintf (out, "%c", unicode_display == unicode_hex ? '<' : '{'); - out += sprintf (out, "0x"); + *out++ = unicode_display == unicode_hex ? '<' : '{'; + *out++ = '0'; + *out++ = 'x'; for (j = 0; j < nchars; j++) out += sprintf (out, "%02x", in [j]); - out += sprintf (out, "%c", unicode_display == unicode_hex ? '>' : '}'); + *out++ = unicode_display == unicode_hex ? '>' : '}'; break; case unicode_highlight: @@ -655,7 +656,7 @@ display_utf8 (const unsigned char * in, char * out, unsigned int * consumed) } if (unicode_display == unicode_highlight && isatty (1)) - out += sprintf (out, "\033[0m"); /* Default colour. */ + out += sprintf (out, "\x1B[0m"); /* Default colour. */ break; default: @@ -711,11 +712,15 @@ sanitize_string (const char * in) /* Copy the input, translating as needed. */ in = original; - if (buffer_len < (strlen (in) * 9)) + /* For 2 char unicode, max out is 12 (colour escapes) + 6, ie. 9 per in + For hex, max out is 8 for 2 char unicode, ie. 4 per in. + 3 and 4 char unicode produce less output for input. */ + size_t max_needed = strlen (in) * 9 + 1; + if (buffer_len < max_needed) { - free ((void *) buffer); - buffer_len = strlen (in) * 9; - buffer = xmalloc (buffer_len + 1); + buffer_len = max_needed; + free (buffer); + buffer = xmalloc (buffer_len); } out = buffer; @@ -735,8 +740,8 @@ sanitize_string (const char * in) { unsigned int num_consumed; - out += display_utf8 ((const unsigned char *)(in - 1), out, & num_consumed); - in += num_consumed - 1; + out += display_utf8 ((const unsigned char *) --in, out, &num_consumed); + in += num_consumed; } else *out++ = c; -- 2.30.2