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 - 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
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
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