ipfw2 buffer overruns
Peter Pentchev
roam at ringlet.net
Fri May 16 06:53:40 PDT 2003
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
--
Peter Pentchev roam at ringlet.net roam at sbnd.net roam at 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 finished reading.
-------------- next part --------------
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.108.24.0/24 or 193.108.32.0/23 or 193.109.54.0/23 or 193.110.159.0/24 or 193.110.216.0/21 or 193.110.82.0/24 or 193.111.194.0/23 or 193.111.89.0/24 or 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.201.172.0/24 or 193.201.240.0/22 or 193.254.29.0/24 or 193.41.182.0/23 or 193.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.145.63.0/24 or 194.153.145.0/24 or 195.138.128.0/19 or 195.212.63.0/24 or 195.230.0.0/19 or 195.234.236.0/22 or 195.24.32.0/19 or 195.34.96.0/19 or 195.72.112.0/24 or 195.75.71.0/24 or 195.96.224.0/19 or 209.239.78.0/23 or 212.116.128.0/19 or 212.122.160.0/19 or 212.124.64.0/19 or 212.21.128.0/19 or 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.56.0/19 or 212.7.192.0/19 or 212.72.192.0/19 or 212.91.160.0/19 or 212.95.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 217.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
-------------- next part --------------
Index: src/sbin/ipfw/ipfw2.c
===================================================================
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;
}
+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 = match_token(rule_actions, *av);
ac--; av++;
+ check_length(action, 1, actbuf, sizeof(actbuf)); /* superfluous.. */
action->len = 1; /* default */
switch(i) {
case TOK_CHECKSTATE:
@@ -2602,6 +2611,7 @@
case TOK_QUEUE:
case TOK_PIPE:
action->len = F_INSN_SIZE(ipfw_insn_pipe);
+ check_length(action, action->len, actbuf, sizeof(actbuf));
case TOK_SKIPTO:
if (i == TOK_QUEUE)
action->opcode = O_QUEUE;
@@ -2639,6 +2649,7 @@
action->opcode = O_FORWARD_IP;
action->len = F_INSN_SIZE(ipfw_insn_sa);
+ check_length(action, action->len, actbuf, sizeof(actbuf));
p->sa.sin_len = sizeof(struct sockaddr_in);
p->sa.sin_family = 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 = next_cmd(action);
/*
@@ -2687,6 +2699,7 @@
errx(EX_DATAERR, "logamount must be positive");
ac--; av++;
}
+ check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf));
cmd = next_cmd(cmd);
}
@@ -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 = next_cmd(cmd);
ac -= 2; av += 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 = next_cmd(cmd);
+ }
ac--; av++; /* any or mac-type */
goto read_options;
}
@@ -2775,6 +2792,7 @@
else {
proto = cmd->arg1;
prev = cmd;
+ check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf));
cmd = next_cmd(cmd);
}
} else if (first_cmd != cmd) {
@@ -2800,6 +2818,7 @@
ac--; av++;
if (F_LEN(cmd) != 0) { /* ! any */
prev = cmd;
+ check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf));
cmd = next_cmd(cmd);
}
}
@@ -2814,7 +2833,10 @@
add_ports(cmd, *av, proto, O_IP_SRCPORT)) {
ac--; av++;
if (F_LEN(cmd) != 0)
+ {
+ check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf));
cmd = next_cmd(cmd);
+ }
}
}
@@ -2835,6 +2857,7 @@
ac--; av++;
if (F_LEN(cmd) != 0) { /* ! any */
prev = cmd;
+ check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf));
cmd = next_cmd(cmd);
}
}
@@ -2849,7 +2872,10 @@
add_ports(cmd, *av, proto, O_IP_DSTPORT)) {
ac--; av++;
if (F_LEN(cmd) != 0)
+ {
+ check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf));
cmd = next_cmd(cmd);
+ }
}
}
@@ -3167,6 +3193,7 @@
}
if (F_LEN(cmd) > 0) { /* prepare to advance */
prev = cmd;
+ check_length(cmd, F_LEN(cmd), cmdbuf, sizeof(cmdbuf));
cmd = next_cmd(cmd);
}
}
@@ -3187,6 +3214,7 @@
if (match_prob != 1) { /* 1 means always match */
dst->opcode = O_PROB;
dst->len = 2;
+ check_length(dst, 2, rulebuf, sizeof(rulebuf));
*((int32_t *)(dst+1)) = (int32_t)(match_prob * 0x7fffffff);
dst += dst->len;
}
@@ -3196,6 +3224,7 @@
*/
if (have_state && have_state->opcode != O_CHECK_STATE) {
fill_cmd(dst, O_PROBE_STATE, 0, 0);
+ check_length(dst, F_LEN(dst), rulebuf, sizeof(rulebuf));
dst = 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 += i;
}
@@ -3220,6 +3250,7 @@
*/
if (have_state && have_state->opcode != O_CHECK_STATE) {
i = F_LEN(have_state);
+ check_length(dst, i, rulebuf, sizeof(rulebuf));
bcopy(have_state, dst, i * sizeof(u_int32_t));
dst += i;
}
@@ -3234,6 +3265,7 @@
src = (ipfw_insn *)cmdbuf;
if ( src->opcode == O_LOG ) {
i = F_LEN(src);
+ check_length(dst, i, rulebuf, sizeof(rulebuf));
bcopy(src, dst, i * sizeof(u_int32_t));
dst += i;
}
@@ -3242,6 +3274,7 @@
*/
for (src = (ipfw_insn *)actbuf; src != action; src += i) {
i = F_LEN(src);
+ check_length(dst, i, rulebuf, sizeof(rulebuf));
bcopy(src, dst, i * sizeof(u_int32_t));
dst += i;
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-ipfw/attachments/20030516/c2543b01/attachment.bin
More information about the freebsd-ipfw
mailing list