⬢ DragonFlyBSD Kernel Audit
DF-3263 / fix.diff
← back to finding ↓ download raw
Fix DF-3263 (RDR reversed port range -> divide-by-zero panic)

Root cause: neither pfctl (usr.sbin/pfctl/parse.y parseport() keeps a > b)
nor the kernel validate that a translation pool port range is ordered, or
that the range is non-degenerate.  pf_get_translation() computes

    (ntohs(dport) - ntohs(r->dst.port[0])) %
        (r->rpool.proxy_port[1] - r->rpool.proxy_port[0] + 1)

and a rule loaded as "port 5001:5000" makes the modulus exactly zero:
integer divide-by-zero -> kernel panic on the first matching packet.

Two layers of fix:

1. pf_ioctl.c: reject reversed/degenerate proxy port ranges at rule-load
   time (DIOCADDRULE and DIOCCHANGERULE both go through this validation).
2. pf.c: defensive clamp in the RDR branch so a rule that slipped through
   (e.g. hand-crafted ioctl, or pfsync-imported ruleset) cannot trap.

--- a/sys/net/pf/pf_ioctl.c
+++ b/sys/net/pf/pf_ioctl.c
@@ -1497,6 +1497,15 @@ pfioctl(struct dev_ioctl_args *ap)
 			pf_mv_pool(&pf_pabuf, &newrule->rpool.list);
 			if (((((newrule->action == PF_NAT) ||
 			    (newrule->action == PF_RDR) ||
@@
 			    !newrule->anchor)) &&
 			    (TAILQ_FIRST(&newrule->rpool.list) == NULL))
 				error = EINVAL;
+			if ((newrule->action == PF_NAT ||
+			     newrule->action == PF_RDR ||
+			     newrule->action == PF_BINAT) &&
+			    newrule->rpool.proxy_port[1] != 0 &&
+			    newrule->rpool.proxy_port[1] <
+			    newrule->rpool.proxy_port[0]) {
+				error = EINVAL;
+			}

--- a/sys/net/pf/pf.c
+++ b/sys/net/pf/pf.c
@@ -3300,10 +3300,17 @@ pf_get_translation(...)
 			if ((r->rpool.opts & PF_POOL_TYPEMASK) ==
 			    PF_POOL_BITMASK)
 				PF_POOLMASK(naddr, naddr,
 				    &r->rpool.cur->addr.v.a.mask, daddr,
 				    pd->af);
 
 			if (r->rpool.proxy_port[1]) {
 				u_int32_t	tmp_nport;
+				u_int32_t	prange;
+
+				prange = r->rpool.proxy_port[1] -
+				    r->rpool.proxy_port[0] + 1;
+				if (prange == 0 || prange > 65536)
+					return (NULL);	/* degenerate range */
 
 				tmp_nport = ((ntohs(dport) -
 				    ntohs(r->dst.port[0])) %
-				    (r->rpool.proxy_port[1] -
-				    r->rpool.proxy_port[0] + 1)) +
+				    prange) +
 				    r->rpool.proxy_port[0];