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?
> | 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?
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.