Re: Potential DoS attack on PF due to infinite loop

On Tue, Jul 11, 2017 at 12:29:33PM -0700, Jingmin Zhou wrote:
> The problem is at line 224. When a LB rule is configured to have 65535 as
> the high port, and uint16 variable tmp reaches it, ++(tmp) will wrap around
> and hence potentially enter into an infinite loop. Of course, it only
> happens when high is configured to 65535.

I think this should detect all overflow problems.

ok?

bluhm

Index: net/pf_lb.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_lb.c,v
retrieving revision 1.60
diff -u -p -r1.60 pf_lb.c
--- net/pf_lb.c	23 Apr 2017 11:37:11 -0000	1.60
+++ net/pf_lb.c	12 Jul 2017 12:11:35 -0000
@@ -211,7 +211,7 @@ pf_get_sport(struct pf_pdesc *pd, struct
 				return (0);
 			}
 		} else {
-			u_int16_t tmp;
+			u_int32_t tmp;
 
 			if (low > high) {
 				tmp = low;
@@ -221,7 +221,7 @@ pf_get_sport(struct pf_pdesc *pd, struct
 			/* low < high */
 			cut = arc4random_uniform(1 + high - low) + low;
 			/* low <= cut <= high */
-			for (tmp = cut; tmp <= high; ++(tmp)) {
+			for (tmp = cut; tmp <= high && tmp <= 0xffff; ++tmp) {
 				key.port[sidx] = htons(tmp);
 				if (pf_find_state_all(&key, dir, NULL) ==
 				    NULL && !in_baddynamic(tmp, pd->proto)) {
@@ -229,7 +229,8 @@ pf_get_sport(struct pf_pdesc *pd, struct
 					return (0);
 				}
 			}
-			for (tmp = cut - 1; tmp >= low; --(tmp)) {
+			tmp = cut;
+			for (tmp -= 1; tmp >= low && tmp <= 0xffff; --tmp) {
 				key.port[sidx] = htons(tmp);
 				if (pf_find_state_all(&key, dir, NULL) ==
 				    NULL && !in_baddynamic(tmp, pd->proto)) {