NetBSD Problem Report #45751

From www@NetBSD.org  Tue Dec 27 21:34:15 2011
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id DC78F63DD64
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 27 Dec 2011 21:34:15 +0000 (UTC)
Message-Id: <20111227213415.0063163DD4A@www.NetBSD.org>
Date: Tue, 27 Dec 2011 21:34:14 +0000 (UTC)
From: alnsn@yandex.ru
Reply-To: alnsn@yandex.ru
To: gnats-bugs@NetBSD.org
Subject: No overflow check in BPF_LD|BPF_ABS
X-Send-Pr-Version: www-1.0

>Number:         45751
>Category:       security
>Synopsis:       No overflow check in BPF_LD|BPF_ABS
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    security-officer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Dec 27 21:35:01 +0000 2011
>Closed-Date:    Thu Dec 29 21:12:47 +0000 2011
>Last-Modified:  Thu Dec 29 21:12:47 +0000 2011
>Originator:     Alexander Nasonov
>Release:        amd64 current
>Organization:
home sweet home
>Environment:
doesn't matter
>Description:
It's possible to bypass length check in BPF_LD|BPF_ABS|BPF_W for pc->k values from UIN32_MAX-3 to UIN32_MAX and for BPF_LD|BPF_ABS|BPF_H for pc->k values UIN32_MAX-1 and UINT32_MAX.
>How-To-Repeat:
I've not tried reproducing the problem.
>Fix:
1) Fix bpf_filter but performance will suffer:

Index: sys/net/bpf_filter.c
===================================================================
RCS file: /cvsroot/src/sys/net/bpf_filter.c,v
retrieving revision 1.48
diff -p -u -u -r1.48 bpf_filter.c
--- sys/net/bpf_filter.c	14 Jul 2011 12:44:10 -0000	1.48
+++ sys/net/bpf_filter.c	27 Dec 2011 21:14:27 -0000
@@ -168,7 +168,8 @@ bpf_filter(const struct bpf_insn *pc, co

 		case BPF_LD|BPF_W|BPF_ABS:
 			k = pc->k;
-			if (k + sizeof(int32_t) > buflen) {
+			if (k > UINT32_MAX - sizeof(int32_t) ||
+			    k + sizeof(int32_t) > buflen) {
 #ifdef _KERNEL
 				int merr = 0;	/* XXX: GCC */

@@ -187,7 +188,8 @@ bpf_filter(const struct bpf_insn *pc, co

 		case BPF_LD|BPF_H|BPF_ABS:
 			k = pc->k;
-			if (k + sizeof(int16_t) > buflen) {
+			if (k > UINT32_MAX - sizeof(int16_t) ||
+			    k + sizeof(int16_t) > buflen) {
 #ifdef _KERNEL
 				int merr;


2) Fix bpf_validate but it would add a restriction to a range of k:

Index: sys/net/bpf_filter.c
===================================================================
RCS file: /cvsroot/src/sys/net/bpf_filter.c,v
retrieving revision 1.48
diff -p -u -u -r1.48 bpf_filter.c
--- sys/net/bpf_filter.c	14 Jul 2011 12:44:10 -0000	1.48
+++ sys/net/bpf_filter.c	27 Dec 2011 21:28:01 -0000
@@ -469,6 +469,7 @@ bpf_validate(const struct bpf_insn *f, i
 {
 	u_int i, from, len, ok = 0;
 	const struct bpf_insn *p;
+	int width;
 #if defined(KERNEL) || defined(_KERNEL)
 	uint16_t *mem, invalid;
 	size_t size;
@@ -521,6 +522,17 @@ bpf_validate(const struct bpf_insn *f, i
 #endif
 				break;
 			case BPF_ABS:
+				switch (BPF_SIZE(pc->code)) {
+				case BPF_W: width = 4; break;
+				case BPF_H: width = 2; break;
+				case BPF_B: width = 1; break;
+				default:
+					goto out;
+				}
+
+				if (pc->k > UINT32_MAX - with)
+					goto out;
+				break;
 			case BPF_IND:
 			case BPF_MSH:
 			case BPF_IMM:


Neither fix is tested.

>Release-Note:

>Audit-Trail:
From: Alexander Nasonov <alnsn@yandex.ru>
To: gnats-bugs@NetBSD.org
Cc: alnsn@yandex.ru
Subject: Re: security/45751: No overflow check in BPF_LD|BPF_ABS
Date: Thu, 29 Dec 2011 20:19:00 +0000

 This was fixed 13 years ago in FreeBSD [1].

 --- head/sys/net/bpf_filter.c	1998/12/07 16:17:41	41587
 +++ head/sys/net/bpf_filter.c	1998/12/07 16:31:15	41588

 ...
  		case BPF_LD|BPF_W|BPF_ABS:
  			k = pc->k;
 -			if (k + sizeof(int32_t) > buflen) {
 +			if (k > buflen || sizeof(int32_t) > buflen - k) {

 BTW, same problem exists for BPF_IND and they fixed it as well:

  		case BPF_LD|BPF_W|BPF_IND:
  			k = X + pc->k;
 -			if (k + sizeof(int32_t) > buflen) {
 +			if (pc->k > buflen || X > buflen - pc->k || sizeof(int32_t) > buflen - k) {


 [1] http://svnweb.freebsd.org/base/head/sys/net/bpf_filter.c?revision=41588&view=markup

 gnats-admin@netbsd.org wrote:
 > Thank you very much for your problem report.
 > It has the internal identification `security/45751'.
 > The individual assigned to look at your
 > report is: security-officer. 
 > 
 > >Category:       security
 > >Responsible:    security-officer
 > >Synopsis:       No overflow check in BPF_LD|BPF_ABS
 > >Arrival-Date:   Tue Dec 27 21:35:01 +0000 2011
 > 
 > 

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/45751 CVS commit: src/sys/net
Date: Thu, 29 Dec 2011 15:50:06 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Thu Dec 29 20:50:06 UTC 2011

 Modified Files:
 	src/sys/net: bpf_filter.c

 Log Message:
 PR/45751: Alexander Nasonov: No overflow check in BPF_LD|BPF_ABS


 To generate a diff of this commit:
 cvs rdiff -u -r1.48 -r1.49 src/sys/net/bpf_filter.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: alnsn@NetBSD.org
State-Changed-When: Thu, 29 Dec 2011 21:12:47 +0000
State-Changed-Why:
Christos fixed it.


>Unformatted:

Home	PR Database Search