Page MenuHomeFreeBSD

ping(8): Improve validation of -g, -G and -h parameters.
ClosedPublic

Authored by markj on Jul 11 2020, 5:11 PM.

Details

Summary
  • Check for potential overflow from strtol().
  • Make sure sweepmax isn't larger than the maximum packet size.
  • Check against sweepmax before incrementing the packet size, not after.

Inspired by PR 239978.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj requested review of this revision.Jul 11 2020, 5:11 PM
markj created this revision.
yuripv added inline comments.
sbin/ping/ping.c
342 ↗(On Diff #74313)

All of these would benefit of using strtonum(3) instead.

markj added inline comments.
sbin/ping/ping.c
342 ↗(On Diff #74313)

Oh nice, I didn't know about that function.

For the record, the patch also fixes 239974 and 239975.

sbin/ping/ping.c
250 ↗(On Diff #79514)

Do you really need to change the type here? All strtonum() use cases are limited by LONG_MAX or smaller values.

982 ↗(On Diff #79514)

You don't need a whitespace before ";" here. Overall, this change looks unrelated.

sbin/ping/ping.c
250 ↗(On Diff #79514)

I don't see any reason why not though - leaving it defined as long will trigger compiler warnings, static analyzer warnings, etc..

982 ↗(On Diff #79514)

I didn't modify that line, but I can fix it.

This is a bug related to the validation of -g/-G/-h. It could be split out in a separate diff, I don't have strong feelings either way.

This revision was not accepted when it landed; it landed in state Needs Review.Tue, Nov 24, 5:12 PM
This revision was automatically updated to reflect the committed changes.