objdump, nm: sprintf sanitizer null destination pointer

Message ID ZMuSoc3Euo895oCb@squeak.grove.modra.org
State Accepted
Headers
Series objdump, nm: sprintf sanitizer null destination pointer |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Alan Modra Aug. 3, 2023, 11:42 a.m. UTC
  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.
  

Patch

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;