diff options
| author | Alexander Bluhm <bluhm@cvs.openbsd.org> | 2019-03-20 20:07:29 +0000 |
|---|---|---|
| committer | Alexander Bluhm <bluhm@cvs.openbsd.org> | 2019-03-20 20:07:29 +0000 |
| commit | ceaa56f81bdfd1b57594eaf9b9973fe9a1d390cc (patch) | |
| tree | 45885ee4d3727654a521d0b0524f2a43dec672c1 | |
| parent | d78036c7a2473520d65a7069d1cc5152aa160c19 (diff) | |
States in pf(4) let ICMP and ICMP6 packets pass if they have a
packet in their payload that matches an exiting connection. It was
not checked whether the outer ICMP packet has the same destination
IP as the source IP of the inner protocol packet. Enforce that
these addresses match, to prevent ICMP packets that do not make
sense.
Issue found by Nicolas Collignon, Corentin Bayet, Eloi Vanderbeken,
Luca Moro at Synacktiv.com
OK sashan@
| -rw-r--r-- | sys/net/pf.c | 28 |
1 files changed, 24 insertions, 4 deletions
diff --git a/sys/net/pf.c b/sys/net/pf.c index bc3a318b2d1..9e454e5c941 100644 --- a/sys/net/pf.c +++ b/sys/net/pf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf.c,v 1.1080 2018/12/17 09:11:10 claudio Exp $ */ +/* $OpenBSD: pf.c,v 1.1081 2019/03/20 20:07:28 bluhm Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -4978,7 +4978,7 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, u_short *reason) { u_int16_t virtual_id, virtual_type; - u_int8_t icmptype; + u_int8_t icmptype, icmpcode; int icmp_dir, iidx, ret, copyback = 0; struct pf_state_key_cmp key; @@ -4986,10 +4986,12 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, switch (pd->proto) { case IPPROTO_ICMP: icmptype = pd->hdr.icmp.icmp_type; + icmpcode = pd->hdr.icmp.icmp_code; break; #ifdef INET6 case IPPROTO_ICMPV6: icmptype = pd->hdr.icmp6.icmp6_type; + icmpcode = pd->hdr.icmp6.icmp6_code; break; #endif /* INET6 */ default: @@ -5167,6 +5169,24 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, unhandled_af(pd->af); } + if (PF_ANEQ(pd->dst, pd2.src, pd->af)) { + if (pf_status.debug >= LOG_NOTICE) { + log(LOG_NOTICE, + "pf: BAD ICMP %d:%d outer dst: ", + icmptype, icmpcode); + pf_print_host(pd->src, 0, pd->af); + addlog(" -> "); + pf_print_host(pd->dst, 0, pd->af); + addlog(" inner src: "); + pf_print_host(pd2.src, 0, pd2.af); + addlog(" -> "); + pf_print_host(pd2.dst, 0, pd2.af); + addlog("\n"); + } + REASON_SET(reason, PFRES_BADSTATE); + return (PF_DROP); + } + switch (pd2.proto) { case IPPROTO_TCP: { struct tcphdr *th = &pd2.hdr.tcp; @@ -5235,7 +5255,7 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, if (pf_status.debug >= LOG_NOTICE) { log(LOG_NOTICE, "pf: BAD ICMP %d:%d ", - icmptype, pd->hdr.icmp.icmp_code); + icmptype, icmpcode); pf_print_host(pd->src, 0, pd->af); addlog(" -> "); pf_print_host(pd->dst, 0, pd->af); @@ -5249,7 +5269,7 @@ pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state, if (pf_status.debug >= LOG_DEBUG) { log(LOG_DEBUG, "pf: OK ICMP %d:%d ", - icmptype, pd->hdr.icmp.icmp_code); + icmptype, icmpcode); pf_print_host(pd->src, 0, pd->af); addlog(" -> "); pf_print_host(pd->dst, 0, pd->af); |