The following reply was made to PR kern/32198; it has been noted by GNATS.

From: Rui Paulo <rpaulo@fnop.net>
To: Guy Harris <guy@alum.mit.edu>
Cc: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org,
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/32198: bpf_validate() needs to do more checks
Date: Wed, 30 Nov 2005 20:00:54 +0000

 On 2005.11.30 10:56:59 -0800, Guy Harris wrote:
 | Rui Paulo wrote:
 | >On 2005.11.30 11:42:00 +0000, guy@alum.mit.edu wrote:
 | >| >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;
 | >
 | >This is done in bpf_setf();
 | >  
 | The check for no instructions isn't - is that check necessary?
 
 Ah, it returns EINVAL but still does an out-of-bounds access in
 bpf_validate if len < 1...
 
 | >|         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).
 | >
 | >What's wrong with the current checks in bpf_validate() ?
 | >  
 | If that question applies to both of those items:
 | 
 | For the first item, to quote a message from Dawson Engler:
 | >we're developing a symbolic execution tool to find bugs in code and it
 | >seems to have turned up two errors in recent BPF code that appear to
 | >let filters read/write arbitrary memory.  Vern recommended that I ask
 | >you guys if these were legitimate bugs.
 | >
 | >If you have a filter with either:
 | >
 | >        code == BPF_STX;
 | >  or
 | >        code == BPF_LDX|BPF_MEM;
 | >
 | >then from what I can tell with testing bpf_validate forgets to check
 | >their associated offset stored in field "k" (it does check
 | >for BPF_ST and BPF_LD but not the *X versions):
 | >
 | >                 * 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;
 | >
 | >then the interpreter lets you read/write arbitrary offsets off of
 | >mem:
 | >
 | >                case BPF_LDX|BPF_MEM:
 | >                        X = mem[pc->k];
 | >                        continue;
 | >
 | >                case BPF_STX:
 | >                        mem[pc->k] = X;
 | >                        continue;
 | Did he miss something, or is that, indeed, a risk?
 
 Oh, that's really a problem.
 
 | For the second item:
 | 
 | The install-time check isn't necessary, but it makes the code more 
 | closely resemble the code in OpenBSD, to make keeping the code in sync 
 | easier.
 
 IMHO this is not needed and the code already differs from the three
 BSDs.
 
 		-- Rui Paulo