If you do

        sudo tcpdump -i {interface} -O 'link[0xFFFFFFFC:4] != 0xf00ba4f'

the generated code is

        (000) ld       #0xfffffffc
        (001) st       M[0]
        (002) ldx      M[0]
        (003) ld       [x + 0]
        (004) st       M[1]
        (005) ld       #0xf00ba4f
        (006) st       M[2]
        (007) ldx      M[2]
        (008) ld       M[1]
        (009) sub      x
        (010) jeq      #0x0             jt 11   jf 12
        (011) ret      #0
        (012) ret      #65535

This code will pass bpf_validate(), as no "k" operand in any instruction is >=
bpf_maxbufsize.  (The "-O" argument is necessary to make the load an indexed
load; an absolute load would fail in bpf_validate() as the "k" operand is
0xfffffffc.

However, the actual offset that's used is 0xfffffffc, which will almost
certainly be past the end of the packet.  The code for an indexed "load word"
instruction is


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

                                if (buflen != 0)
                                        return 0;
                                A = bpf_m_xword((struct mbuf *)p, k, &merr);
                                if (merr != 0)
                                        return 0;
                                continue;
#else
                                return 0;
#endif
                        }
                        A = EXTRACT_LONG(&p[k]);
                        continue;

Even though buflen is 0 in most calls for incoming or outgoing packets, on a
32-bit platform, 0xfffffffc + sizeof(int32_t) is 0, so the code will just use
EXTRACT_LONG(&p[k]) and extract some random value from memory.

This means that, on a 32-bit platform, that filter will pass most if not all
packets, even though it should reject them all.

On a 64-bit platform, the sum will not overflow, and the code path inside the
if will fail immediately if buflen is 0, and should fail in the MINDEX() macro
bpf_m_xword() otherwise.

A similar problem exists with "load halfword" instructions (the corresponding
filter would be something such as "link[0xFFFFFFFE:2] != 0xf00d").  "load
byte"'s check does no such addition and is OK.

A fix, modeled after the from other BSDs, is attached.  The check for absolute
loads should be OK without the change, at least in the kernel, as
bpd_validate() makes sure the "k" field is < bpf_maxbufsize, but the patch
changes it anyway, to match other OSes versions of bpd_filter() and to make
the code safe if used in userland; the check for indexed loads needs to
change.

("k > buflen" suffices; "k >= buflen" would also work, but, if k == buflen,
the subsequent check will fail.  The other BSDs check for k > buflen, so this
makes the tests similar.)

[demime 1.01d removed an attachment of type application/octet-stream which had 
a name of patch]

Reply via email to