Page MenuHomeFreeBSD

Make TCP options parsing stricter
ClosedPublic

Authored by ae on Dec 10 2019, 12:43 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Mar 23, 1:57 PM
Unknown Object (File)
Dec 23 2023, 1:26 AM
Unknown Object (File)
Dec 20 2023, 10:14 AM
Unknown Object (File)
Sep 5 2023, 10:59 AM
Unknown Object (File)
Aug 27 2023, 8:47 PM
Unknown Object (File)
Jul 8 2023, 12:48 AM
Unknown Object (File)
Mar 21 2023, 8:35 PM
Unknown Object (File)
Mar 4 2023, 10:46 AM
Subscribers

Details

Summary

Rework tcpopts_parse() to be more strict. Add length checks for specific TCP options. The behavior is copied from tcp_input.c.
The main purpose of the change is avoiding of possible out of mbuf's data access.

Reported by: Maxime Villard

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 28059
Build 26207: arc lint + arc unit

Event Timeline

melifaro added inline comments.
sys/netpfil/ipfw/ip_fw2.c
333

Would help if we add a comment suggesting that the parsing logic mimics one from tcp_dooptions().

334

Since we're talking about safeness: can we change the signature to be const struct tcphdr *tcp?

338

Would it be possible to rename x to something more resembling the remaining byte count?

For example, tcp_dooptions() uses cnt for the similar purpose.

This revision is now accepted and ready to land.Dec 10 2019, 1:14 PM

Looks reasonable, with good suggestions from @melifaro

ae marked 3 inline comments as done.

Commited in rS355712.