pf: add missing IPv6 length check
ClosedPublic
Actions

Authored by kp on Tue, Jul 15, 9:52 AM.

Details

Reviewers

Group Reviewers

network
pfsense

Commits

rGcb43e97aea11: pf: add missing IPv6 length check

Summary

We failed to verify that the packet was long enough for the provided IPv6 packet
length. This could result in us walking off the end of the mbuf and panicing.

PR: 288224
Reported by: Robert Morris <rtm@lcs.mit.edu>
Sponsored by: Rubicon Communications, LLC ("Netgate")

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 65452
Build 62335: arc lint + arc unit

Event Timeline

kp created this revision.Tue, Jul 15, 9:52 AM

Herald added subscribers: vegeta_tuxpowered.net, glebius, melifaro and 2 others. · View Herald TranscriptTue, Jul 15, 9:52 AM

kp requested review of this revision.Tue, Jul 15, 9:52 AM

Harbormaster completed remote builds in B65452: Diff 158534.Tue, Jul 15, 9:52 AM

emaste added a subscriber: emaste.Tue, Jul 15, 6:44 PM

emaste added inline comments.

sys/netpfil/pf/pf.c
10174	Should we move this check earlier as well? (I.e., is it possible to craft a corrupt jumbogram that would crash in pf_walk_header6 because it passed the `pd->m->m_pkthdr.len < sizeof(struct ip6_hdr) + ntohs(h->ip6_plen)` test?

kp added inline comments.Tue, Jul 15, 7:03 PM

sys/netpfil/pf/pf.c
10174	There wouldn't be any harm in it, and because it's an unconditional drop it should probably be done as early as possible, but I don't think it's an issue. pf_walk_header6() does all if its mbuf accesses via pf_pull_hdr(), which checks that we're within the mbuf bounds.