Recently we discovered a potential bug in pf_lb.c. It is in the latest code that we retrieved from here:
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c?annotate=1.60 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. Thanks, Jingmin 146: int1.13 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.13> henning 147: pf_get_sport(struct pf_pdesc *pd, struct pf_rule *r,1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 148: struct pf_addr *naddr, u_int16_t *nport, u_int16_t low, u_int16_t high,1.13 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.13> henning 149: struct pf_src_node **sn)1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 150: { 151: struct pf_state_key_cmp key; 152: struct pf_addr init_addr; 153: u_int16_t cut;1.54 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.54> bluhm 154: int dir = (pd->dir == PF_IN) ? PF_OUT : PF_IN; 155: int sidx = pd->sidx; 156: int didx = pd->didx;1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 157: 158: bzero(&init_addr, sizeof(init_addr));1.19 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.19> claudio 159: if (pf_map_addr(pd->naf, r, &pd->nsaddr, naddr, &init_addr, sn, &r->nat,1.9 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.9> henning 160: PF_SN_NAT))1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 161: return (1); 162:1.35 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.35> bluhm 163: if (pd->proto == IPPROTO_ICMP) { 164: if (pd->ndport == htons(ICMP_ECHO)) {1.4 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.4> mcbride 165: low = 1; 166: high = 65535; 167: } else 168: return (0); /* Don't try to modify non-echo ICMP */1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 169: }1.35 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.35> bluhm 170: #ifdef INET6 171: if (pd->proto == IPPROTO_ICMPV6) { 172: if (pd->ndport == htons(ICMP6_ECHO_REQUEST)) { 173: low = 1; 174: high = 65535; 175: } else 176: return (0); /* Don't try to modify non-echo ICMP */ 177: } 178: #endif /* INET6 */1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 179: 180: do {1.19 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.19> claudio 181: key.af = pd->naf;1.13 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.13> henning 182: key.proto = pd->proto; 183: key.rdomain = pd->rdomain;1.54 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.54> bluhm 184: PF_ACPY(&key.addr[didx], &pd->ndaddr, key.af); 185: PF_ACPY(&key.addr[sidx], naddr, key.af); 186: key.port[didx] = pd->ndport;1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 187: 188: /* 189: * port search; start random, step; 190: * similar 2 portloop in in_pcbbind 191: */1.13 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.13> henning 192: if (!(pd->proto == IPPROTO_TCP || pd->proto == IPPROTO_UDP ||1.30 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.30> mikeb 193: pd->proto == IPPROTO_ICMP || pd->proto == IPPROTO_ICMPV6)) {1.12 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.12> sthen 194: /* XXX bug: icmp states dont use the id on both 195: * XXX sides (traceroute -I through nat) */1.54 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.54> bluhm 196: key.port[sidx] = pd->nsport; 197: if (pf_find_state_all(&key, dir, NULL) == NULL) {1.13 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.13> henning 198: *nport = pd->nsport;1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 199: return (0);1.12 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.12> sthen 200: }1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 201: } else if (low == 0 && high == 0) {1.54 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.54> bluhm 202: key.port[sidx] = pd->nsport; 203: if (pf_find_state_all(&key, dir, NULL) == NULL) {1.13 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.13> henning 204: *nport = pd->nsport;1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 205: return (0);1.12 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.12> sthen 206: }1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 207: } else if (low == high) {1.54 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.54> bluhm 208: key.port[sidx] = htons(low); 209: if (pf_find_state_all(&key, dir, NULL) == NULL) {1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 210: *nport = htons(low); 211: return (0); 212: } 213: } else { 214: u_int16_t tmp; 215: 216: if (low > high) { 217: tmp = low; 218: low = high; 219: high = tmp; 220: } 221: /* low < high */ 222: cut = arc4random_uniform(1 + high - low) + low; 223: /* low <= cut <= high */ 224: for (tmp = cut; tmp <= high; ++(tmp)) {1.54 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.54> bluhm 225: key.port[sidx] = htons(tmp); 226: if (pf_find_state_all(&key, dir, NULL) ==1.13 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.13> henning 227: NULL && !in_baddynamic(tmp, pd->proto)) {1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 228: *nport = htons(tmp); 229: return (0); 230: } 231: } 232: for (tmp = cut - 1; tmp >= low; --(tmp)) {1.54 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.54> bluhm 233: key.port[sidx] = htons(tmp); 234: if (pf_find_state_all(&key, dir, NULL) ==1.13 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.13> henning 235: NULL && !in_baddynamic(tmp, pd->proto)) {1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 236: *nport = htons(tmp); 237: return (0); 238: } 239: } 240: } 241:1.6 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.6> henning 242: switch (r->nat.opts & PF_POOL_TYPEMASK) {1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 243: case PF_POOL_RANDOM: 244: case PF_POOL_ROUNDROBIN:1.15 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.15> zinke 245: case PF_POOL_LEASTSTATES:1.29 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.29> mikeb 246: /* 247: * pick a different source address since we're out 248: * of free port choices for the current one. 249: */1.19 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.19> claudio 250: if (pf_map_addr(pd->naf, r, &pd->nsaddr, naddr,1.13 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.13> henning 251: &init_addr, sn, &r->nat, PF_SN_NAT))1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 252: return (1); 253: break; 254: case PF_POOL_NONE: 255: case PF_POOL_SRCHASH: 256: case PF_POOL_BITMASK: 257: default: 258: return (1); 259: }1.19 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.19> claudio 260: } while (! PF_AEQ(&init_addr, naddr, pd->naf) );1.1 <https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pf_lb.c#rev1.1> pyr 261: return (1); /* none available */ 262: }
