Bug 258048 - 'ping' option parsing broken after merging in 'ping6' code.
Summary: 'ping' option parsing broken after merging in 'ping6' code.
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Alan Somers
Depends on:
Reported: 2021-08-25 20:46 UTC by ghuckriede
Modified: 2021-08-30 16:52 UTC (History)
3 users (show)

See Also:

Potential Fix (3.79 KB, patch)
2021-08-25 20:46 UTC, ghuckriede
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ghuckriede 2021-08-25 20:46:59 UTC
Created attachment 227435 [details]
Potential Fix

When ping6 was merged into ping, the option parsing was broken.

During the first pass of the options, to determine ipv4 or ipv6, only options "46" are provided to getopt().  This causes getopt() to match any '4' or '6', because it believes these are the only valid options.

Below are some examples of how the first pass into getopt() is interpreted:
"-c6" will match 'c' as invalid (which is ignored) and '6' as ipv6 (instead of a count of 6)
"-s46" will match 's' as invalid (which is ignored), '4' as ipv4, and '6' as ipv6 (instead of a size of 46).

I've provided a diff that moves the option string creation to main.h and uses that for all getopt() calls.

The diff also includes the following additional fixes:
- Reorder the PING6ADDOPTS options alphabetically like the PING4ADDOPTS.
- Add missing space after the "[-m ttl]" option the in usage() text.
- Swap the "aA" with "Aa" in the ping 6 usage() text to match PING6ADDOPTS.
- Add some #ifdef IPSEC around some unused declarations.

Steps to Reproduce:
ping -s46
ping -s46 ::1
ping -4 -c6
ping -6 -c4 ::1       
ping -4 -t6
ping -6 -t4 ::1       
ping -6 -t4 ::1
ping -6 -z4 -c1 ::1
ping -s6 -c1
ping -s4 -c1 ::1

Actual Results:
All the "valid" command above return the following errors.
"ping: -4 and -6 cannot be used simultaneously"
"ping: IPv6 requested but IPv4 target address provided"

Expected Results:
ping should not interpret '4' and '6' in the values of other options as "options".

Build Date & Hardware: 
git HEAD @ 9f7a81b133c715f649136dcd0ad004e4180c56c9

Additional Information:
Comment 1 Alan Somers freebsd_committer 2021-08-26 21:56:20 UTC
Thanks for the bug report and the patch.  One more thing, would you be able to add any regression tests to sbin/ping/tests/ping_test.sh ?
Comment 2 ghuckriede 2021-08-30 13:58:01 UTC
I've never looked at those regression tests.  It may be faster to have someone familiar with them do it, but I'll have a look and see.