ifconfig: Fix ASAN 'dynamic-stack-buffer-overflow' in formatting.

When building Inetutils with address sanitizer enabled, I noticed that
the test suite catches a 'dynamic-stack-buffer-overflow'. Here is the
steps to reproduce:

  # Ignore memory leaks that are handled by end of process.
  $ export ASAN_OPTIONS=detect_leaks=0
  $ uname -a
  Linux fedora 6.8.8-300.fc40.x86_64 #1 SMP PREEMPT_DYNAMIC Sat Apr 27 17:53:31 UTC 2024 x86_64 GNU/Linux
  $ gcc --version
  gcc (GCC) 14.0.1 20240411 (Red Hat 14.0.1-0)
  $ ./bootstrap
  $ ./configure CFLAGS="-fsanitize=address -ggdb -O0"
  $ make -j 16 check
  FAIL: ifconfig.sh

Originally I thought this was limited to 'ifconfig -s' (or --short),
but it seems to occur with uses of --format too. All of the failures
look something like this:

  $ ./ifconfig/ifconfig --format=osf -i lo
  lo: flags=49<UP,LOOPBACK,RUNNING>
  	inet 127.0.0.1 netmask ff000000 ipmtu 65536
  =================================================================
  ==112905==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7fff68ed3fc0 at pc 0x00000040db6b bp 0x7fff68ed3f90 sp 0x7fff68ed3f88
  WRITE of size 8 at 0x7fff68ed3fc0 thread T0
      #0 0x40db6a in print_interfaceX /home/collin/.local/src/inetutils/ifconfig/printif.c:1137
      #1 0x409a73 in select_arg /home/collin/.local/src/inetutils/ifconfig/printif.c:267
  [...]

I think it should be easily reproduced, but I can send more logs if
requested. Here is the offending section of code in
ifconfig/printif.c, comments added to the bad write on line 1137 and
allocation on line 1129:

================================================
	    {
	      int argc = 0;
	      char **argv;
	      argv = alloca (strlen (q) / 2); /* Line 1129 */

	      while (*p == '{')
		{
		  p++;
		  form->format = p;
		  print_interfaceX (form, 1);
		  q = form->format;
		  argv[argc] = xmalloc (q - p + 1); /* Line 1137 */
		  memcpy (argv[argc], p, q - p);
		  argv[argc][q - p] = '\0';
		  if (*q == '}')
		    q++;
		  p = q;
		  argc++;
		}

	      format_handler (id, form, argc, argv);

	      /* Clean up.  */
	      form->format = p;
	      while (--argc >= 0)
		free (argv[argc]);
	    }
================================================

I'm not super familiar with this code, but it looks like this is
parsing the formats from ifconfig/options.c and passing information in
the 'argv' array for printing.

The alloca looks incorrect but I couldn't get it to crash when messing
with compiler flags. Changing the 'alloca' to 'malloc' and using
valgrind shows many 'use of uninitialized values', 'invalid writes',
etc.

    $ valgrind ./ifconfig/ifconfig --format=osf
    [...]
    ==127549== ERROR SUMMARY: 127 errors from 31 contexts (suppressed: 0 from 0)

So I am confident it is a bug. I've applied the attached patch which
seems to fix the issue. This is based on a quick glance of the code so
I would appreciate others looking it over. Thanks!

Collin

From 45d5900c9ee8d03554f28d698fc8533c7dcb321b Mon Sep 17 00:00:00 2001
From: Collin Funk <[email protected]>
Date: Mon, 6 May 2024 17:57:11 -0700
Subject: [PATCH] ifconfig: Fix ASAN 'dynamic-stack-buffer-overflow' in
 formatting.

* ifconfig/printif.c (print_interfaceX): Allocate the argv array on
the heap as the format string is being processed. The previous
'alloca' leads to invalid writes detected by ASAN and Valgrind when
using the --format and --short options.
---
 ifconfig/printif.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/ifconfig/printif.c b/ifconfig/printif.c
index 4f2fdc1a..f68547bd 100644
--- a/ifconfig/printif.c
+++ b/ifconfig/printif.c
@@ -1125,8 +1125,7 @@ print_interfaceX (format_data_t form, int quiet)
 	  else
 	    {
 	      int argc = 0;
-	      char **argv;
-	      argv = alloca (strlen (q) / 2);
+	      char **argv = NULL;
 
 	      while (*p == '{')
 		{
@@ -1134,6 +1133,7 @@ print_interfaceX (format_data_t form, int quiet)
 		  form->format = p;
 		  print_interfaceX (form, 1);
 		  q = form->format;
+                  argv = xrealloc (argv, (argc + 1) * sizeof (char *));
 		  argv[argc] = xmalloc (q - p + 1);
 		  memcpy (argv[argc], p, q - p);
 		  argv[argc][q - p] = '\0';
@@ -1144,11 +1144,14 @@ print_interfaceX (format_data_t form, int quiet)
 		}
 
 	      format_handler (id, form, argc, argv);
-
-	      /* Clean up.  */
-	      form->format = p;
-	      while (--argc >= 0)
-		free (argv[argc]);
+              if (argv != NULL)
+                {
+                  /* Clean up.  */
+                  while (--argc >= 0)
+                    free (argv[argc]);
+                  free (argv);
+                }
+              form->format = p;
 	    }
 	}
     }
-- 
2.45.0