Paths

Table of Contentst

Differential D25622

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

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

Tags

None

Referenced Files

	F154258086: D25622.id79936.diff
	Mon, Apr 27, 11:00 AM

	F154253667: D25622.id79522.diff
	Mon, Apr 27, 10:26 AM

	F154250033: D25622.id79514.diff
	Mon, Apr 27, 9:55 AM

	Unknown Object (File)
	Thu, Apr 23, 7:45 AM

	Unknown Object (File)
	Mon, Apr 6, 7:19 AM

	Unknown Object (File)
	Mon, Apr 6, 4:43 AM

	Unknown Object (File)
	Mar 28 2026, 7:36 AM

	Unknown Object (File)
	Mar 24 2026, 6:25 PM

Subscribers

Details

Reviewers

eugen_grosbein.net

Commits

rS367988: ping(8): Improve parameter validation

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 32244
Build 29734: arc lint + arc unit

Event Timeline

markj requested review of this revision.Jul 11 2020, 5:11 PM

markj created this revision.

Harbormaster completed remote builds in B32244: Diff 74313.Jul 11 2020, 5:11 PM

markj added a reviewer: eugen_grosbein.net.Jul 11 2020, 5:11 PM

yuripv added a subscriber: yuripv.Jul 11 2020, 6:23 PM

yuripv added inline comments.

sbin/ping/ping.c
342	All of these would benefit of using strtonum(3) instead.

markj planned changes to this revision.Jul 11 2020, 6:41 PM

markj added inline comments.

sbin/ping/ping.c
342	Oh nice, I didn't know about that function.

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

Convert to strtonum().

Harbormaster completed remote builds in B34782: Diff 79514.Nov 13 2020, 6:51 PM

maxim added inline comments.Nov 13 2020, 8:07 PM

sbin/ping/ping.c
248	Do you really need to change the type here? All strtonum() use cases are limited by LONG_MAX or smaller values.
971–973	You don't need a whitespace before ";" here. Overall, this change looks unrelated.

markj added inline comments.Nov 13 2020, 8:18 PM

sbin/ping/ping.c
248	I don't see any reason why not though - leaving it defined as `long` will trigger compiler warnings, static analyzer warnings, etc..
971–973	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.

Fix a nearby style bug.

Harbormaster completed remote builds in B34787: Diff 79522.Nov 13 2020, 8:21 PM

This revision was not accepted when it landed; it landed in state Needs Review.Nov 24 2020, 5:12 PM

Closed by commit rS367988: ping(8): Improve parameter validation (authored by markj). · Explain Why

This revision was automatically updated to reflect the committed changes.

markj added a commit: rS367988: ping(8): Improve parameter validation.

Herald added a subscriber: imp. · View Herald TranscriptNov 24 2020, 5:12 PM

Revision Contents
Changeset List

Path

Size

sbin/

ping/

25 lines

Diff 74313

sbin/ping/ping.c

Loading...