Page MenuHomeFreeBSD

ping: Fix integer underflow resuling in a ping -R segfault
ClosedPublic

Authored by cy on Feb 23 2023, 6:50 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sep 24 2024, 7:28 AM
Unknown Object (File)
Aug 21 2024, 3:25 PM
Unknown Object (File)
Aug 21 2024, 3:25 PM
Unknown Object (File)
Aug 21 2024, 3:25 PM
Unknown Object (File)
Aug 21 2024, 2:45 PM
Unknown Object (File)
Aug 15 2024, 3:12 AM
Unknown Object (File)
Aug 14 2024, 7:16 PM
Unknown Object (File)
Aug 8 2024, 4:24 PM
Subscribers

Details

Summary

ping -R (F_RROUTE) will loop at ping.c:1381 until it segfaults:

slippy$ ping -R 192.168.0.101
PING 192.168.0.101 (192.168.0.101): 56 data bytes
64 bytes from 192.168.0.101: icmp_seq=0 ttl=63 time=1.081 ms
RR: 192.168.0.1

192.168.0.101
192.168.0.101
10.1.1.254
10.1.1.91

unknown option bb
unknown option 32
unknown option 6
...
unknown option 96
unknown option 2d
Segmentation fault

The reason for this is while looping through loose source routing and
strict source routing, hlen will become negative, terminating the loop.
However, when hlen is unsigned, it wraps around becoming a large number
causing the loop to continue virtually forever until hlen is by chance
smaller than an IP header.

Fixes: 46d7b45a267b

This should be MFCed to 13.2 prior to release.

Test Plan

Tested and fixed locally.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

cy requested review of this revision.Feb 23 2023, 6:50 AM

Note: another alternate solution for D38470 and D38593.

In D38744#881716, @jlduran_gmail.com wrote:

Note: another alternate solution for D38470 and D38593.

This approach is certainly more simple than the other two approaches.

cy retitled this revision from ping: Fix ping -R segfault to ping: Fix integer overflow resuling in a ping -R segfault.Feb 23 2023, 2:40 PM
cy edited the test plan for this revision. (Show Details)
cy retitled this revision from ping: Fix integer overflow resuling in a ping -R segfault to ping: Fix integer underflow resuling in a ping -R segfault.Feb 23 2023, 3:05 PM
cy edited the summary of this revision. (Show Details)

I have also tested this revision, and it is passing all my tests.
I also agree that this is the simpler approach, since it essentially reverts hlen to being a signed integer again.
If this revision gets accepted, I'll update mine to just submit the tests.

Hopefully this kind of thing won't happen any more once we have a better ping test suite.

This revision is now accepted and ready to land.Feb 24 2023, 2:33 PM