Date: Fri, 16 May 2003 16:50:53 +0300 From: Peter Pentchev <roam@ringlet.net> To: ipfw@FreeBSD.org Subject: ipfw2 buffer overruns Message-ID: <20030516135052.GA13482@straylight.oblivion.bg>
next in thread | raw e-mail | index | archive | help
--Yylu36WmvOXNoKYn Content-Type: multipart/mixed; boundary="Dxnq1zWXvFF0Q93v" Content-Disposition: inline --Dxnq1zWXvFF0Q93v Content-Type: text/plain; charset=windows-1251 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, A friend of mine, Kiril Todorov (CC'd), recently came across some quite strange ipfw2 behavior on -STABLE: when given a specific rule to add, ipfw would hang. A bit of digging into src/sbin/ipfw/ipfw2.c revealed a couple of internal buffers - actbuf[], cmdbuf[], rulebuf[] - with a set length of 255, and a couple of pointers traversing those buffers which were never actually checked for running over the end. Thus, it was trivial to construct a long enough 'ipfw add' command that would eventually overrun the buffer, with much confusion ensuing. Attached is a sample rule that causes this, and a patch which performs a couple of length checks and refuses to add the rule if a buffer overrun is detected. This is not the most elegant solution, and in a couple of the checks the damage is already done, but still... The patch is against -STABLE; it applies to -CURRENT with just a couple of offset lines, and the resulting source compiles; I do not currently have a functional -CURRENT machine to test it on, though. It works on -STABLE, correctly diagnosing the oversized rule, and some other tests I threw at it in a hurry. Still, this is the first time I'm touching the ipfw code, so there is a very high probability that this is not the right way, or not even close to the right direction; feel free to point it out :) G'luck, Peter --=20 Peter Pentchev roam@ringlet.net roam@sbnd.net roam@FreeBSD.org PGP key: http://people.FreeBSD.org/~roam/roam.key.asc Key fingerprint FDBA FD79 C26F 3C51 C95E DF9E ED18 B68D 1619 4553 You have, of course, just begun reading the sentence that you have just fin= ished reading. --Dxnq1zWXvFF0Q93v Content-Type: text/plain; charset=windows-1251 Content-Disposition: attachment; filename="rule.big" Content-Transfer-Encoding: quoted-printable add 10 skipto 1000 all from { 139.92.144.0/24 or 139.92.170.0/24 or 139.92.= 51.0/24 or 152.158.113.0/24 or 157.125.223.0/24 or 192.92.129.0/24 or 193.1= 08.24.0/24 or 193.108.32.0/23 or 193.109.54.0/23 or 193.110.159.0/24 or 193= =2E110.216.0/21 or 193.110.82.0/24 or 193.111.194.0/23 or 193.111.89.0/24 o= r 193.178.152.0/23 or 193.178.166.0/24 or 193.178.222.0/24 or 193.193.162.0= /22 or 193.193.164.0/24 or 193.193.182.0/24 or 193.194.140.0/23 or 193.194.= 141.0/24 or 193.194.156.0/24 or 193.200.0.0/16 or 193.201.114.0/24 or 193.2= 01.172.0/24 or 193.201.240.0/22 or 193.254.29.0/24 or 193.41.182.0/23 or 19= 3.41.188.0/22 or 193.41.206.0/24 or 193.41.64.0/22 or 193.68.0.0/19 or 193.= 68.128.0/17 or 193.68.96.0/19 or 194.12.224.0/19 or 194.141.0.0/16 or 194.1= 45.63.0/24 or 194.153.145.0/24 or 195.138.128.0/19 or 195.212.63.0/24 or 19= 5.230.0.0/19 or 195.234.236.0/22 or 195.24.32.0/19 or 195.34.96.0/19 or 195= =2E72.112.0/24 or 195.75.71.0/24 or 195.96.224.0/19 or 209.239.78.0/23 or 2= 12.116.128.0/19 or 212.122.160.0/19 or 212.124.64.0/19 or 212.21.128.0/19 o= r 212.36.0.0/19 or 212.39.64.0/19 or 212.50.0.0/19 or 212.5.128.0/19 or 212= =2E56.0/19 or 212.7.192.0/19 or 212.72.192.0/19 or 212.91.160.0/19 or 212.9= 5.160.0/19 or 213.130.64.0/19 or 213.137.32.0/19 or 213.145.96.0/19 or 213.= 16.32.0/19 or 213.169.32.0/19 or 213.174.0.0/19 or 213.191.192.0/19 or 213.= 208.10.0/23 or 213.222.32.0/19 or 213.226.0.0/18 or 213.91.128.0/17 or 217.= 10.240.0/20 or 217.18.240.0/20 or 217.145.160.0/20 or 217.197.128.0/20 or 2= 17.79.32.0/19 or 217.79.64.0/20 or 217.9.224.0/20 or 217.75.128.0/20 or 10.= 0.0.0/8 or 172.16.0.0/12 } to any --Dxnq1zWXvFF0Q93v Content-Type: text/plain; charset=windows-1251 Content-Disposition: attachment; filename="ipfw2-rel4.patch" Content-Transfer-Encoding: quoted-printable Index: src/sbin/ipfw/ipfw2.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/ncvs/src/sbin/ipfw/ipfw2.c,v retrieving revision 1.4.2.12 diff -u -r1.4.2.12 ipfw2.c --- src/sbin/ipfw/ipfw2.c 14 Apr 2003 12:41:37 -0000 1.4.2.12 +++ src/sbin/ipfw/ipfw2.c 16 May 2003 13:40:25 -0000 @@ -2481,6 +2481,14 @@ return NULL; } =20 +static void +check_length(ipfw_insn *cmd, size_t add, void *ptr, size_t size) +{ + + if ((uintptr_t)(cmd + add) > (uintptr_t)((char *)ptr + size)) + errx(EX_DATAERR, "Rule too long!"); +} + /* * Parse arguments and assemble the microinstructions which make up a rule. * Rules are added into the 'rulebuf' and then copied in the correct order @@ -2562,6 +2570,7 @@ NEED1("missing action"); i =3D match_token(rule_actions, *av); ac--; av++; + check_length(action, 1, actbuf, sizeof(actbuf)); /* superfluous.. */ action->len =3D 1; /* default */ switch(i) { case TOK_CHECKSTATE: @@ -2602,6 +2611,7 @@ case TOK_QUEUE: case TOK_PIPE: action->len =3D F_INSN_SIZE(ipfw_insn_pipe); + check_length(action, action->len, actbuf, sizeof(actbuf)); case TOK_SKIPTO: if (i =3D=3D TOK_QUEUE) action->opcode =3D O_QUEUE; @@ -2639,6 +2649,7 @@ =20 action->opcode =3D O_FORWARD_IP; action->len =3D F_INSN_SIZE(ipfw_insn_sa); + check_length(action, action->len, actbuf, sizeof(actbuf)); =20 p->sa.sin_len =3D sizeof(struct sockaddr_in); p->sa.sin_family =3D AF_INET; @@ -2665,6 +2676,7 @@ default: errx(EX_DATAERR, "invalid action %s\n", av[-1]); } + check_length(action, F_LEN(action), actbuf, sizeof(actbuf)); action =3D next_cmd(action); =20 /* @@ -2687,6 +2699,7 @@ errx(EX_DATAERR, "logamount must be positive"); ac--; av++; } + check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf)); cmd =3D next_cmd(cmd); } =20 @@ -2751,12 +2764,16 @@ !strncmp(*av, "mac", strlen(*av))) { ac--; av++; /* the "MAC" keyword */ add_mac(cmd, ac, av); /* exits in case of errors */ + check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf)); cmd =3D next_cmd(cmd); ac -=3D 2; av +=3D 2; /* dst-mac and src-mac */ NOT_BLOCK; NEED1("missing mac type"); if (add_mactype(cmd, ac, av[0])) + { + check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf)); cmd =3D next_cmd(cmd); + } ac--; av++; /* any or mac-type */ goto read_options; } @@ -2775,6 +2792,7 @@ else { proto =3D cmd->arg1; prev =3D cmd; + check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf)); cmd =3D next_cmd(cmd); } } else if (first_cmd !=3D cmd) { @@ -2800,6 +2818,7 @@ ac--; av++; if (F_LEN(cmd) !=3D 0) { /* ! any */ prev =3D cmd; + check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf)); cmd =3D next_cmd(cmd); } } @@ -2814,7 +2833,10 @@ add_ports(cmd, *av, proto, O_IP_SRCPORT)) { ac--; av++; if (F_LEN(cmd) !=3D 0) + { + check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf)); cmd =3D next_cmd(cmd); + } } } =20 @@ -2835,6 +2857,7 @@ ac--; av++; if (F_LEN(cmd) !=3D 0) { /* ! any */ prev =3D cmd; + check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf)); cmd =3D next_cmd(cmd); } } @@ -2849,7 +2872,10 @@ add_ports(cmd, *av, proto, O_IP_DSTPORT)) { ac--; av++; if (F_LEN(cmd) !=3D 0) + { + check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf)); cmd =3D next_cmd(cmd); + } } } =20 @@ -3167,6 +3193,7 @@ } if (F_LEN(cmd) > 0) { /* prepare to advance */ prev =3D cmd; + check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf)); cmd =3D next_cmd(cmd); } } @@ -3187,6 +3214,7 @@ if (match_prob !=3D 1) { /* 1 means always match */ dst->opcode =3D O_PROB; dst->len =3D 2; + check_length(dst, 2, rulebuf, sizeof(rulebuf)); *((int32_t *)(dst+1)) =3D (int32_t)(match_prob * 0x7fffffff); dst +=3D dst->len; } @@ -3196,6 +3224,7 @@ */ if (have_state && have_state->opcode !=3D O_CHECK_STATE) { fill_cmd(dst, O_PROBE_STATE, 0, 0); + check_length(dst, F_LEN(dst), rulebuf, sizeof(rulebuf)); dst =3D next_cmd(dst); } /* @@ -3210,6 +3239,7 @@ case O_LIMIT: break; default: + check_length(dst, i, rulebuf, sizeof(rulebuf)); bcopy(src, dst, i * sizeof(u_int32_t)); dst +=3D i; } @@ -3220,6 +3250,7 @@ */ if (have_state && have_state->opcode !=3D O_CHECK_STATE) { i =3D F_LEN(have_state); + check_length(dst, i, rulebuf, sizeof(rulebuf)); bcopy(have_state, dst, i * sizeof(u_int32_t)); dst +=3D i; } @@ -3234,6 +3265,7 @@ src =3D (ipfw_insn *)cmdbuf; if ( src->opcode =3D=3D O_LOG ) { i =3D F_LEN(src); + check_length(dst, i, rulebuf, sizeof(rulebuf)); bcopy(src, dst, i * sizeof(u_int32_t)); dst +=3D i; } @@ -3242,6 +3274,7 @@ */ for (src =3D (ipfw_insn *)actbuf; src !=3D action; src +=3D i) { i =3D F_LEN(src); + check_length(dst, i, rulebuf, sizeof(rulebuf)); bcopy(src, dst, i * sizeof(u_int32_t)); dst +=3D i; } --Dxnq1zWXvFF0Q93v-- --Yylu36WmvOXNoKYn Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.2 (FreeBSD) iD8DBQE+xOy87Ri2jRYZRVMRAstYAJ4yL/eZnXwGKNz7xhSlLmifds+BJQCcCWuw kWA58CT1Q6d+oibcgx4rYcA= =Pj5n -----END PGP SIGNATURE----- --Yylu36WmvOXNoKYn--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030516135052.GA13482>