Re: Potential DoS attack on PF due to infinite loop

2017-07-16 Thread Alexander Bluhm
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 -  1.60
+++ net/pf_lb.c 12 Jul 2017 12:11:35 -
@@ -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 <= 0x; ++tmp) {
key.port[sidx] = htons(tmp);
if (pf_find_state_all(, 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 <= 0x; --tmp) {
key.port[sidx] = htons(tmp);
if (pf_find_state_all(, dir, NULL) ==
NULL && !in_baddynamic(tmp, pd->proto)) {


Re: Potential DoS attack on PF due to infinite loop

2017-07-16 Thread Alexandr Nedvedicky
On Wed, Jul 12, 2017 at 02:40:58PM +0200, Alexander Bluhm wrote:
> 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?

looks good to me.

OK sashan
> 
> 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 -  1.60
> +++ net/pf_lb.c   12 Jul 2017 12:11:35 -
> @@ -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 <= 0x; ++tmp) {
>   key.port[sidx] = htons(tmp);
>   if (pf_find_state_all(, 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 <= 0x; --tmp) {
>   key.port[sidx] = htons(tmp);
>   if (pf_find_state_all(, dir, NULL) ==
>   NULL && !in_baddynamic(tmp, pd->proto)) {
> 


Re: Potential DoS attack on PF due to infinite loop

2017-07-16 Thread Alexander Bluhm
On Tue, Jul 11, 2017 at 12:29:33PM -0700, Jingmin Zhou wrote:
> Recently we discovered a potential bug in pf_lb.c.

I have commited a fix.  Thanks for the report and analysis.

bluhm