- make the k field in struct bpf_insn unsigned, as promised in the

manual page.
- more strict bpf code validation, preventing arbitrary kernel memory
read and writes.
Some help from frantzen@ and canacar@; testing jmc@ markus@;
ok canacar@ henning@ franzen@

2 files changed, 97 insertions, 34 deletions

diff --git a/sys/net/bpf.h b/sys/net/bpf.h
index a118ed946ed..ef1a64e63eb 100644
--- a/sys/net/bpf.h
+++ b/sys/net/bpf.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bpf.h,v 1.25 2004/02/06 22:38:58 tedu Exp $	*/
+/*	$OpenBSD: bpf.h,v 1.26 2004/04/26 08:10:10 otto Exp $	*/
 /*	$NetBSD: bpf.h,v 1.15 1996/12/13 07:57:33 mikel Exp $	*/
 
 /*
@@ -235,7 +235,7 @@ struct bpf_insn {
 	u_int16_t code;
 	u_char 	  jt;
 	u_char 	  jf;
-	int32_t	  k;
+	u_int32_t k;
 };
 
 /*
diff --git a/sys/net/bpf_filter.c b/sys/net/bpf_filter.c
index 0468a06f659..fe9063a4840 100644
--- a/sys/net/bpf_filter.c
+++ b/sys/net/bpf_filter.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bpf_filter.c,v 1.13 2004/02/23 19:06:50 markus Exp $	*/
+/*	$OpenBSD: bpf_filter.c,v 1.14 2004/04/26 08:10:10 otto Exp $	*/
 /*	$NetBSD: bpf_filter.c,v 1.12 1996/02/13 22:00:00 christos Exp $	*/
 
 /*
@@ -78,13 +78,16 @@
 	} \
 }
 
-int	bpf_m_xword(struct mbuf *, int, int *);
-int	bpf_m_xhalf(struct mbuf *, int, int *);
+extern int bpf_maxbufsize;
+
+int	bpf_m_xword(struct mbuf *, u_int32_t, int *);
+int	bpf_m_xhalf(struct mbuf *, u_int32_t, int *);
 
 int
 bpf_m_xword(m, k, err)
 	struct mbuf *m;
-	int k, *err;
+	u_int32_t k;
+	int *err;
 {
 	int len;
 	u_char *cp, *np;
@@ -92,7 +95,7 @@ bpf_m_xword(m, k, err)
 
 	MINDEX(len, m, k);
 	cp = mtod(m, u_char *) + k;
-	if (len - k >= 4) {
+	if (len >= k + 4) {
 		*err = 0;
 		return EXTRACT_LONG(cp);
 	}
@@ -120,7 +123,8 @@ bpf_m_xword(m, k, err)
 int
 bpf_m_xhalf(m, k, err)
 	struct mbuf *m;
-	int k, *err;
+	u_int32_t k;
+	int *err;
 {
 	int len;
 	u_char *cp;
@@ -128,7 +132,7 @@ bpf_m_xhalf(m, k, err)
 
 	MINDEX(len, m, k);
 	cp = mtod(m, u_char *) + k;
-	if (len - k >= 2) {
+	if (len >= k + 2) {
 		*err = 0;
 		return EXTRACT_SHORT(cp);
 	}
@@ -158,7 +162,7 @@ bpf_filter(pc, p, wirelen, buflen)
 	u_int buflen;
 {
 	u_int32_t A = 0, X = 0;
-	int k;
+	u_int32_t k;
 	int32_t mem[BPF_MEMWORDS];
 
 	if (pc == 0)
@@ -482,39 +486,98 @@ bpf_validate(f, len)
 	struct bpf_insn *f;
 	int len;
 {
-	int i;
+	u_int i, from;
 	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 that jumps are forward, and within 
-		 * the code block.
+		 * Check that memory operations use valid addresses.
 		 */
-		p = &f[i];
-		if (BPF_CLASS(p->code) == BPF_JMP) {
-			int from = i + 1;
-
-			if (BPF_OP(p->code) == BPF_JA) {
-				if (from + p->k < from ||
-				    from + p->k >= len)
+		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;
 			}
-			else if (from + p->jt >= len || from + p->jf >= len)
+			break;
+		case BPF_ST:
+		case BPF_STX:
+			if (p->k >= BPF_MEMWORDS)
 				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 || p->k < 0))
-			return 0;
-		/*
-		 * Check for constant division by 0.
-		 */
-		if (p->code == (BPF_ALU|BPF_DIV|BPF_K) && p->k == 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 that jumps are forward, and within 
+			 * the code block.
+			 */
+			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;
 }