Skip site navigation (1)Skip section navigation (2)
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: <http://docs.FreeBSD.org/cgi/mid.cgi?20030516135052.GA13482>