Page MenuHomeFreeBSD

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

Authored by markj on Jul 11 2020, 5:11 PM.
Tags
None
Referenced Files
F133096343: D25622.id79514.diff
Wed, Oct 22, 10:50 PM
F133096342: D25622.id79522.diff
Wed, Oct 22, 10:50 PM
F133096336: D25622.id.diff
Wed, Oct 22, 10:50 PM
F133096334: D25622.id79936.diff
Wed, Oct 22, 10:50 PM
F133096333: D25622.id74313.diff
Wed, Oct 22, 10:50 PM
F133060273: D25622.diff
Wed, Oct 22, 2:06 PM
Unknown Object (File)
Wed, Oct 15, 6:46 AM
Unknown Object (File)
Sun, Oct 12, 11:54 PM
Subscribers

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

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 34782
Build 31831: arc lint + arc unit

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
350–351

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

markj added inline comments.
sbin/ping/ping.c
350–351

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

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

sbin/ping/ping.c
250

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

982

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

sbin/ping/ping.c
250

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

982

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.Nov 24 2020, 5:12 PM
This revision was automatically updated to reflect the committed changes.