kern/89752: bpf_validate() needs to do more checks
Guy Harris
guy at alum.mit.edu
Wed Nov 30 10:20:31 GMT 2005
>Number: 89752
>Category: kern
>Synopsis: bpf_validate() needs to do more checks
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Wed Nov 30 10:20:02 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator: Guy Harris
>Release:
>Organization:
>Environment:
>Description:
OpenBSD's bpf_validate() in sys/net/bpf_filter.c does some additional checks to catch:
BPF programs with no instructions or with more than BPF_MAXINSNS instructions;
BPF_STX and BPF_LDX|BPF_MEM instructions that have out-of-range offsets (which could be made to fetch or store into arbitrary memory locations);
BPF_DIV instructions with a constant 0 divisor (that's a check also done at run time).
Here's a patch to make the FreeBSD bpf_validate() match it (with some changes to comments; I've submitted the changes to comments to the OpenBSD bug database).
>How-To-Repeat:
>Fix:
Index: bpf_filter.c
===================================================================
RCS file: /home/ncvs/src/sys/net/bpf_filter.c,v
retrieving revision 1.23
diff -c -r1.23 bpf_filter.c
*** bpf_filter.c 7 Jan 2005 01:45:34 -0000 1.23
--- bpf_filter.c 30 Nov 2005 09:57:08 -0000
***************
*** 507,513 ****
/*
* Return true if the 'fcode' is a valid filter program.
* The constraints are that each jump be forward and to a valid
! * code. The code must terminate with either an accept or reject.
*
* The kernel needs to be able to verify an application's filter code.
* Otherwise, a bogus program could easily crash the system.
--- 507,516 ----
/*
* Return true if the 'fcode' is a valid filter program.
* The constraints are that each jump be forward and to a valid
! * code, that memory accesses are within valid ranges (to the
! * extent that this can be checked statically; loads of packet
! * data have to be, and are, also checked at run time), and that
! * the code terminates with either an accept or reject.
*
* The kernel needs to be able to verify an application's filter code.
* Otherwise, a bogus program could easily crash the system.
***************
*** 517,555 ****
const struct bpf_insn *f;
int len;
{
! register int i;
register const struct bpf_insn *p;
for (i = 0; i < len; ++i) {
/*
! * Check that that jumps are forward, and within
! * the code block.
*/
! p = &f[i];
! if (BPF_CLASS(p->code) == BPF_JMP) {
! register int from = i + 1;
!
! if (BPF_OP(p->code) == BPF_JA) {
! if (from >= len || p->k >= len - from)
return 0;
}
! else if (from >= len || p->jt >= len - from ||
! p->jf >= len - from)
return 0;
! }
! /*
! * Check that memory operations use valid addresses.
! */
! if ((BPF_CLASS(p->code) == BPF_ST ||
! (BPF_CLASS(p->code) == BPF_LD &&
! (p->code & 0xe0) == BPF_MEM)) &&
! p->k >= BPF_MEMWORDS)
! return 0;
! /*
! * Check for constant division by 0.
! */
! if (p->code == (BPF_ALU|BPF_DIV|BPF_K) && p->k == 0)
return 0;
}
return BPF_CLASS(f[len - 1].code) == BPF_RET;
}
--- 520,628 ----
const struct bpf_insn *f;
int len;
{
! u_int i, from;
register const struct bpf_insn *p;
+ if (len < 1 || len > BPF_MAXINSNS)
+ return 0;
+
for (i = 0; i < len; ++i) {
+ p = &f[i];
+ switch (BPF_CLASS(p->code)) {
/*
! * Check that memory operations use valid addresses.
*/
! case BPF_LD:
! case BPF_LDX:
! switch (BPF_MODE(p->code)) {
! case BPF_IMM:
! break;
! case BPF_ABS:
! case BPF_IND:
! case BPF_MSH:
! /*
! * More strict check with actual packet length
! * is done runtime.
! */
! if (p->k >= bpf_maxbufsize)
! return 0;
! break;
! case BPF_MEM:
! if (p->k >= BPF_MEMWORDS)
return 0;
+ break;
+ case BPF_LEN:
+ break;
+ default:
+ return 0;
}
! break;
! case BPF_ST:
! case BPF_STX:
! if (p->k >= BPF_MEMWORDS)
return 0;
! break;
! case BPF_ALU:
! switch (BPF_OP(p->code)) {
! case BPF_ADD:
! case BPF_SUB:
! case BPF_OR:
! case BPF_AND:
! case BPF_LSH:
! case BPF_RSH:
! case BPF_NEG:
! break;
! case BPF_DIV:
! /*
! * Check for constant division by 0.
! */
! if (BPF_RVAL(p->code) == BPF_K && p->k == 0)
! return 0;
! default:
! return 0;
! }
! break;
! case BPF_JMP:
! /*
! * Check that jumps are within the code block,
! * and that unconditional branches don't go
! * backwards as a result of an overflow.
! * Unconditional branches have a 32-bit offset,
! * so they could overflow; we check to make
! * sure they don't. Conditional branches have
! * an 8-bit offset, and the from address is <=
! * BPF_MAXINSNS, and we assume that BPF_MAXINSNS
! * is sufficiently small that adding 255 to it
! * won't overflow.
! *
! * We know that len is <= BPF_MAXINSNS, and we
! * assume that BPF_MAXINSNS is < the maximum size
! * of a u_int, so that i + 1 doesn't overflow.
! */
! from = i + 1;
! switch (BPF_OP(p->code)) {
! case BPF_JA:
! if (from + p->k < from || from + p->k >= len)
! return 0;
! break;
! case BPF_JEQ:
! case BPF_JGT:
! case BPF_JGE:
! case BPF_JSET:
! if (from + p->jt >= len || from + p->jf >= len)
! return 0;
! break;
! default:
! return 0;
! }
! break;
! case BPF_RET:
! break;
! case BPF_MISC:
! break;
! default:
return 0;
+ }
}
return BPF_CLASS(f[len - 1].code) == BPF_RET;
}
>Release-Note:
>Audit-Trail:
>Unformatted:
More information about the freebsd-bugs
mailing list