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!
CollinFrom 45d5900c9ee8d03554f28d698fc8533c7dcb321b Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.fu...@gmail.com>
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
