svn commit: r345366 - head/sys/netpfil/pf

[Kristof Provost]

Author: kp
Date: Thu Mar 21 08:09:52 2019
New Revision: 345366
URL: https://svnweb.freebsd.org/changeset/base/345366

Log:
  pf: Ensure that IP addresses match in ICMP error packets
  
  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.
  
  Reported by:	Nicolas Collignon, Corentin Bayet, Eloi Vanderbeken, Luca Moro at Synacktiv
  Obtained from:	OpenBSD
  Security:	CVE-2019-5598

Modified:
  head/sys/netpfil/pf/pf.c

Modified: head/sys/netpfil/pf/pf.c
==============================================================================
--- head/sys/netpfil/pf/pf.c	Thu Mar 21 06:47:23 2019	(r345365)
+++ head/sys/netpfil/pf/pf.c	Thu Mar 21 08:09:52 2019	(r345366)
@@ -4594,7 +4594,7 @@ pf_test_state_icmp(struct pf_state **state, int direct
 {
 	struct pf_addr  *saddr = pd->src, *daddr = pd->dst;
 	u_int16_t	 icmpid = 0, *icmpsum;
-	u_int8_t	 icmptype;
+	u_int8_t	 icmptype, icmpcode;
 	int		 state_icmp = 0;
 	struct pf_state_key_cmp key;
 
@@ -4603,6 +4603,7 @@ pf_test_state_icmp(struct pf_state **state, int direct
 #ifdef INET
 	case IPPROTO_ICMP:
 		icmptype = pd->hdr.icmp->icmp_type;
+		icmpcode = pd->hdr.icmp->icmp_code;
 		icmpid = pd->hdr.icmp->icmp_id;
 		icmpsum = &pd->hdr.icmp->icmp_cksum;
 
@@ -4617,6 +4618,7 @@ pf_test_state_icmp(struct pf_state **state, int direct
 #ifdef INET6
 	case IPPROTO_ICMPV6:
 		icmptype = pd->hdr.icmp6->icmp6_type;
+		icmpcode = pd->hdr.icmp6->icmp6_code;
 		icmpid = pd->hdr.icmp6->icmp6_id;
 		icmpsum = &pd->hdr.icmp6->icmp6_cksum;
 
@@ -4815,6 +4817,23 @@ pf_test_state_icmp(struct pf_state **state, int direct
 #endif /* INET6 */
 		}
 
+		if (PF_ANEQ(pd->dst, pd2.src, pd->af)) {
+			if (V_pf_status.debug >= PF_DEBUG_MISC) {
+				printf("pf: BAD ICMP %d:%d outer dst: ",
+				    icmptype, icmpcode);
+				pf_print_host(pd->src, 0, pd->af);
+				printf(" -> ");
+				pf_print_host(pd->dst, 0, pd->af);
+				printf(" inner src: ");
+				pf_print_host(pd2.src, 0, pd2.af);
+				printf(" -> ");
+				pf_print_host(pd2.dst, 0, pd2.af);
+				printf("\n");
+			}
+			REASON_SET(reason, PFRES_BADSTATE);
+			return (PF_DROP);
+		}
+
 		switch (pd2.proto) {
 		case IPPROTO_TCP: {
 			struct tcphdr		 th;
@@ -4871,7 +4890,7 @@ pf_test_state_icmp(struct pf_state **state, int direct
 			    !SEQ_GEQ(seq, src->seqlo - (dst->max_win << dws)))) {
 				if (V_pf_status.debug >= PF_DEBUG_MISC) {
 					printf("pf: BAD ICMP %d:%d ",
-					    icmptype, pd->hdr.icmp->icmp_code);
+					    icmptype, icmpcode);
 					pf_print_host(pd->src, 0, pd->af);
 					printf(" -> ");
 					pf_print_host(pd->dst, 0, pd->af);
@@ -4884,7 +4903,7 @@ pf_test_state_icmp(struct pf_state **state, int direct
 			} else {
 				if (V_pf_status.debug >= PF_DEBUG_MISC) {
 					printf("pf: OK ICMP %d:%d ",
-					    icmptype, pd->hdr.icmp->icmp_code);
+					    icmptype, icmpcode);
 					pf_print_host(pd->src, 0, pd->af);
 					printf(" -> ");
 					pf_print_host(pd->dst, 0, pd->af);