ping: do a better job checking what we receive from the net

[maxim]

Summary

• zero out a buffer for the incoming icmp message that we are about to parse
• for ICMP_MASKREPLY and ICMP_TSTAMPREPLY responses check their length and warn and reject them if they are truncated

Without these checks a part of stack allocated struct icmp icp could
be printed out which seems low severity since we already dropped root
privileges by the time icp is allocated.

MFC after: 1 month
Found with: Claude Code Sonnet 5

[glebius]

The call to pr_pack() is preceded by < 0 check of return of recvmsg() and then with another == 0 check. Thus, the cc is always positive. I would suggest to change prototype of pr_pack to unsigned type of cc. This will allow to operate with unsigned types for all lengths in the function and remove existing (ssize_t) casts as well as avoid adding new such casts.

sbin/ping/ping.c

1122

New code should use memset(). Or you can declare it as struct icmp icp = {} leaving optimization to compiler.

[maxim]

In D59555#1366699, @glebius wrote:

The call to pr_pack() is preceded by < 0 check of return of recvmsg() and then with another == 0 check. Thus, the cc is always positive. I would suggest to change prototype of pr_pack to unsigned type of cc. This will allow to operate with unsigned types for all lengths in the function and remove existing (ssize_t) casts as well as avoid adding new such casts.

This work has a very narrow focus on a specific task: hardening against truncated/mangled responses to specific ICMP queries. While changing the pr_pack() prototype is probably a good idea per se, it seems that we don't want to mix unrelated changes into one patch.

sbin/ping/ping.c

1122

ping.c uses bzero(3) everywhere so I just want to keep my changes consistent with the style of the rest of the code. A sweep s/bzero/memset/ change is probably feasible but should be made separately and is out of scope of this specific patch.

[glebius]

I don't object to the change, but IMHO better to not be afraid of improving the old code.

[maxim]

In D59555#1367069, @glebius wrote:

I don't object to the change, but IMHO better to not be afraid of improving the old code.

I am not! :-)

Thanks for the review!