Skip to content

Commit 37d17dc

Browse files
committed
net: fix OOB reads and unbounded growth in IPv6 net-channel classifier and DHCPv6 client
Security hardening for the two network-reachable memory-safety issues in the new IPv6/DHCPv6 code (found in audit): 1. classify_ipv6_tcp() OOB read (core/net_channel.cc). ip6_lasthdr_nofrag only guarantees the base IPv6 header + extension headers fit; it did NOT guarantee a full TCP header is present and contiguous, yet the code then dereferenced tcp_hdr (th_flags at +13, ports at +0..3) on the lock-free RX fast path. A crafted IPv6/TCP frame (short, or with extension headers pushing the L4 offset toward the end, or split across a mergeable-buffer mbuf chain via GRO/jumbo) could read out of bounds in the driver softirq. Require nxt_off + sizeof(tcphdr) <= mh_len before touching the TCP header; otherwise fall through to the slow path (tcp_input), which pulls up. 2. DHCPv6 parse() OOB read across an mbuf chain (bsd/porting/dhcp6.cc). The option walk bounded off against pkthdr.len (whole chain) while reading via mtod (first mbuf only); udp6_input makes only the UDP header contiguous. A malicious on-link DHCPv6 server (DHCPv6 is unauthenticated; the client is auto-started on a Managed-flag RA) sending a >4 KiB reply on a jumbo/GRO link becomes an mbuf chain and the walk reads past the first mbuf. Clamp blen to the contiguous first-mbuf length (a well-formed reply is far smaller than one RX buffer). 3. DHCPv6 unbounded state growth (DoS). process_packet() parsed every packet even when BOUND, and parse() appended to _dns/_server_duid, so an on-link attacker who observed our SOLICIT xid could spray REPLYs and grow those without bound. Ignore packets outside SOLICITING/REQUESTING, and reset the per-reply learned state only after the xid matches (so a spoofed packet cannot wipe good state and repeated replies do not accumulate). Verified: IPv6 loopback + dual-stack TCP smoke still passes.
1 parent 1ef2c3a commit 37d17dc

2 files changed

Lines changed: 39 additions & 1 deletion

File tree

bsd/porting/dhcp6.cc

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,18 @@ bool dhcp6_interface_state::parse(struct mbuf* m, msg_type& out_type)
272272
if (payload_off <= 0)
273273
payload_off = sizeof(struct ip6_hdr) + sizeof(struct udphdr);
274274
u8* p = mtod(m, u8*) + payload_off;
275-
int blen = m->M_dat.MH.MH_pkthdr.len - payload_off;
275+
// udp6_input only makes the UDP header contiguous, not the DHCPv6 body.
276+
// pkthdr.len covers the whole mbuf chain, but p points into the FIRST
277+
// mbuf, so bound the option walk by the contiguous first-mbuf length, not
278+
// the chain total - otherwise a >4 KiB reply (jumbo / host GRO -> mbuf
279+
// chain from a malicious on-link DHCPv6 server) would make the walk read
280+
// past this mbuf. A well-formed DHCPv6 reply is far smaller than one RX
281+
// buffer, so clamping to the contiguous region loses nothing legitimate.
282+
int chain_len = m->M_dat.MH.MH_pkthdr.len - payload_off;
283+
int contig_len = (int)m->m_hdr.mh_len - payload_off;
284+
int blen = (chain_len < contig_len) ? chain_len : contig_len;
285+
if (blen < 0)
286+
blen = 0;
276287
if (blen < 4)
277288
return false;
278289

@@ -281,6 +292,12 @@ bool dhcp6_interface_state::parse(struct mbuf* m, msg_type& out_type)
281292
if (xid != _xid)
282293
return false; // not our transaction
283294

295+
// Only after confirming this reply is for our transaction do we reset the
296+
// per-reply learned state, so a non-matching (e.g. spoofed) packet cannot
297+
// wipe good state, and repeated matching replies do not accumulate.
298+
_dns.clear();
299+
_have_addr = false;
300+
284301
int off = 4;
285302
while (off + 4 <= blen) {
286303
u16 code = get16(p + off);
@@ -358,6 +375,14 @@ void dhcp6_interface_state::bind_address()
358375

359376
void dhcp6_interface_state::process_packet(struct mbuf* m)
360377
{
378+
// Only process replies while we are actually soliciting/requesting. Once
379+
// bound (or before we start), ignore unsolicited packets: otherwise an
380+
// on-link attacker who observed our SOLICIT's transaction id could spray
381+
// crafted REPLYs, each of which parse() would append to _dns/_server_duid,
382+
// growing them without bound (memory DoS).
383+
if (_state != DH6_SOLICITING && _state != DH6_REQUESTING) {
384+
return;
385+
}
361386
msg_type t;
362387
if (!parse(m, t))
363388
return;

core/net_channel.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,19 @@ net_channel* classifier::classify_ipv6_tcp(mbuf* m, ip6_hdr *ip_hdr, size_t ip_l
285285
return nullptr;
286286
}
287287

288+
// ip6_lasthdr_nofrag only guarantees the base IPv6 header (and any
289+
// extension headers) fit; it does NOT guarantee a full TCP header is
290+
// present and contiguous in this mbuf. A crafted frame (short, or with
291+
// extension headers pushing nxt_off toward the end, or split across an
292+
// mbuf chain) could otherwise make the tcp_hdr dereferences below read out
293+
// of bounds on the lock-free RX fast path. Require the TCP header to lie
294+
// wholly within the contiguous first mbuf before touching it; anything
295+
// else falls through to the slow path (tcp_input), which pulls up.
296+
if (nxt_off < 0 ||
297+
(size_t)nxt_off + sizeof(struct tcphdr) > (size_t)m->m_hdr.mh_len) {
298+
return nullptr;
299+
}
300+
288301
auto tcp_hdr = reinterpret_cast<tcphdr*>(start + nxt_off);
289302
if (tcp_hdr->th_flags & (TH_SYN | TH_FIN | TH_RST)) {
290303
return nullptr;

0 commit comments

Comments
 (0)