Bug 239974

Summary: ping(8) crashes with SIGSEGV - Out-of-Bounds Write of size 1 (global-buffer-overflow)
Product: Base System Reporter: Neeraj <neerajpal09>
Component: binAssignee: Mark Johnston <markj>
Status: Closed FIXED    
Severity: Affects Many People CC: markj, neerajpal09
Priority: --- Keywords: crash, patch, security
Version: CURRENT   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
proposed patch none

Description Neeraj 2019-08-19 21:24:19 UTC
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.
Comment 1 Mark Johnston freebsd_committer freebsd_triage 2020-07-11 18:11:37 UTC
I took a somewhat different approach: https://reviews.freebsd.org/D25622
Comment 2 commit-hook freebsd_committer freebsd_triage 2020-11-24 17:13:19 UTC
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
Comment 3 commit-hook freebsd_committer freebsd_triage 2020-12-01 15:09:06 UTC
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
Comment 4 Mark Johnston freebsd_committer freebsd_triage 2020-12-01 15:10:14 UTC
Thanks for the reports.