Created attachment 206711 [details] proposed patch ping(8) crashes with Segmentation Fault due to Out-0f-Bound Write of size 1, causing global-buffer-overflow * /usr/src/sbin/ping/ping.c:945 Compiled with address sanitizer option "-fsanitize=address" on clang/gcc to verify the reproducibility with detailed log info. [Steps to reproduce] * compile: make CC=clang CFLAGS="-fsanitize=address -O0" * /usr/obj/usr/src/amd64.amd64/sbin/ping/ping -G 1 -h 65452 localhost without compiling with sanitizer, it crashes when: * ping -G 1 -h 69517 localhost root@freebsd:/usr/src/sbin/ping # ping -G 1 -h 69517 localhost PING localhost (127.0.0.1): (0 ... 1) data bytes 8 bytes from 127.0.0.1: icmp_seq=0 ttl=64 Segmentation fault Below command will crash at any test case: (tested with already available ping with system) * command: ping -G -h 123456 localhost Sanitizer log as follows: ==4641==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000bad87f at pc 0x0000002e6d49 bp 0x7ffffffed7b0 sp 0x7ffffffed7a8 WRITE of size 1 at 0x000000bad87f thread T0 #0 0x2e6d48 in main /usr/src/sbin/ping/ping.c:945:15 #1 0x23a10e in _start /usr/src/lib/csu/amd64/crt1.c:76:7 0x000000bad87f is located 0 bytes to the right of global variable 'outpackhdr' defined in '/usr/src/sbin/ping/ping.c:172:15' (0xb9d880) of size 65535 SUMMARY: AddressSanitizer: global-buffer-overflow /usr/src/sbin/ping/ping.c:945:15 in main Shadow bytes around the buggy address: 0x400000175ab0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x400000175ac0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x400000175ad0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x400000175ae0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x400000175af0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x400000175b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00[07] 0x400000175b10: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 0x400000175b20: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 0x400000175b30: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 0x400000175b40: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 0x400000175b50: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Shadow gap: cc ==4641==ABORTING [Inferior 1 (process 4641) exited with code 01 Note: root privilege required.
I took a somewhat different approach: https://reviews.freebsd.org/D25622
A commit references this bug: Author: markj Date: Tue Nov 24 17:12:40 UTC 2020 New revision: 367988 URL: https://svnweb.freebsd.org/changeset/base/367988 Log: ping(8): Improve parameter validation - Use strtonum(3) to simplify bounds checking of numeric parameters. - Fix bounds checking when filling out packet data in "sweep" mode. PR: 239974, 239977, 239978 Reported by: Neeraj <neerajpal09@gmail.com> MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D25622 Changes: head/sbin/ping/ping.c
A commit references this bug: Author: markj Date: Tue Dec 1 15:09:03 UTC 2020 New revision: 368231 URL: https://svnweb.freebsd.org/changeset/base/368231 Log: MFC r367988: ping(8): Improve parameter validation PR: 239974, 239977, 239978 Changes: _U stable/12/ stable/12/sbin/ping/ping.c
Thanks for the reports.