traviscross / mtr

Official repository for mtr, a network diagnostic tool
http://www.bitwizard.nl/mtr/
GNU General Public License v2.0
2.6k stars 334 forks source link

Running 'mtr -r www.google.com -c 1' leads to segmentation fault #469

Closed robert-scheck closed 1 year ago

robert-scheck commented 1 year ago

Using MTR 0.95, running mtr -r www.google.com -c 1 leads to a segmentation fault.

There is a downstream report at https://bugzilla.redhat.com/show_bug.cgi?id=2188394 which I am able to reproduce using a container:

[root@ac1b9ae63297 ~]# gdb --args mtr -4 -r www.google.com -c 1
GNU gdb (Fedora Linux) 13.1-4.fc39
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from mtr...
Reading symbols from /usr/lib/debug/usr/sbin/mtr-0.95-4.fc38.x86_64.debug...
(gdb) run
Starting program: /usr/sbin/mtr -4 -r www.google.com -c 1
warning: Error disabling address space randomization: Function not implemented
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Missing separate debuginfo for /lib64/libgcc_s.so.1.
The debuginfo package for this file is probably broken.
[Detaching after fork from child process 235]
[Detaching after fork from child process 236]
Start: 2023-04-21T20:43:49+0000
*** buffer overflow detected ***: terminated

Program received signal SIGABRT, Aborted.
__pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
44            return INTERNAL_SYSCALL_ERROR_P (ret) ? INTERNAL_SYSCALL_ERRNO (ret) : 0;
(gdb) bt
#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
#1  0x00007f5e94714313 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
#2  0x00007f5e946c31ce in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007f5e946ab87f in __GI_abort () at abort.c:79
#4  0x00007f5e946ac60f in __libc_message (fmt=fmt@entry=0x7f5e948272c5 "*** %s ***: terminated\n") at ../sysdeps/posix/libc_fatal.c:150
#5  0x00007f5e947a7949 in __GI___fortify_fail (msg=msg@entry=0x7f5e948272ac "buffer overflow detected") at fortify_fail.c:24
#6  0x00007f5e947a7304 in __GI___chk_fail () at chk_fail.c:28
#7  0x00007f5e947a8ae5 in ___snprintf_chk (s=s@entry=0x7ffeef1f5591 "      ", maxlen=maxlen@entry=1024, flag=flag@entry=2, slen=slen@entry=991, format=format@entry=0x7ffeef1f5420 "%6s") at snprintf_chk.c:29
#8  0x000055c1a2b3ee44 in snprintf (__fmt=0x7ffeef1f5420 "%6s", __n=1024, __s=0x7ffeef1f5591 "      ") at /usr/include/bits/stdio2.h:54
#9  report_close (ctl=ctl@entry=0x7ffeef1f6440) at ui/report.c:143
#10 0x000055c1a2b4038d in display_close (ctl=ctl@entry=0x7ffeef1f6440) at ui/display.c:127
#11 0x000055c1a2b3994f in main (argc=<optimized out>, argv=<optimized out>) at ui/mtr.c:828
(gdb) 
robert-scheck commented 1 year ago

Apologies, this is already fixed by commit 5908af4c19188cb17b62f23368b6ef462831a0cb

rewolff commented 1 year ago

Oh, that fortify stuff crashes your program when there is a POSSIBLE buffer overflow, not when there really IS a buffer overflow ? We were printing "about 100, max 200" characters into a 1000 char buffer without using snprintf. Then some compiler complained and someone quickly changed it to snprintf without doing the math for the buffer length correctly. So statically it still fits with PLENTY of margin.

grbeneke commented 2 weeks ago

Is it possible to tag a release with this fix? The version packaged with Ubuntu 24.04 LTS still has this bug.