Require "link proto" argument value to be within range.

pcap_compile() fails to reject invalid link-layer protocol numbers, for
example:

$ filtertest EN10MB 'link proto 0x12345678'
(000) ldh      [12]
(001) jeq      #0x12345678      jt 2	jf 3
(002) ret      #262144
(003) ret      #0
(loads a 16-bit value and compares it with a 32-bit value)

$ filtertest EN10MB 'link proto 1200'
(000) ldh      [12]
(001) jgt      #0x5dc           jt 5	jf 2
(002) ldb      [14]
(003) jeq      #0x4b0           jt 4	jf 5
(004) ret      #262144
(005) ret      #0
(loads an 8-bit value and compares it with a 16-bit value)

Fixing this failure to fail requires well more than one check because
the valid range and values of a protocol number are specific to the DLT
and/or the protocol, also in some cases there is a mapping between
different protocols.  Add a reject filter test for every new check.

diff --git a/CHANGES b/CHANGES
index b58cd7a036f765e031416abecf4da133acc06fe5..ab607aa7d8ab825e21fe27ec6e049b26a29372b6 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -58,6 +58,7 @@ DayOfTheWeek, Month DD, YYYY / The Tcpdump Group
       Use the correct bit mask for IS-IS PDU type value.
       Fix the "|" and "&" operators of the "byte" primitive.
       Require "byte" argument value to be within range.
+      Require "link proto" argument value to be within range.
     rpcap:
       Support user names and passwords in rpcap:// and rpcaps:// URLs.
       Add a -t flag to rpcapd to specify the data channel port; from

diff --git a/gencode.c b/gencode.c
index fc84b087896c4642934f241415820c747b817635..746a7ec053ae102864eb41fe001bfe427088a27f 100644 (file)
--- a/gencode.c
+++ b/gencode.c
@@ -668,7 +668,7 @@ static struct slist *gen_load_ppi_llprefixlen(compiler_state_t *);
 static void insert_compute_vloffsets(compiler_state_t *, struct block *);
 static struct slist *gen_abs_offset_varpart(compiler_state_t *,
     bpf_abs_offset *);
-static bpf_u_int32 ethertype_to_ppptype(bpf_u_int32);
+static uint16_t ethertype_to_ppptype(compiler_state_t *, bpf_u_int32);
 static struct block *gen_linktype(compiler_state_t *, bpf_u_int32);
 static struct block *gen_snap(compiler_state_t *, bpf_u_int32, bpf_u_int32);
 static struct block *gen_llc_linktype(compiler_state_t *, bpf_u_int32);
@@ -2511,6 +2511,7 @@ gen_ether_linktype(compiler_state_t *cstate, bpf_u_int32 ll_proto)
 
        default:
                if (ll_proto <= ETHERMTU) {
+                       assert_maxval(cstate, "LLC DSAP", ll_proto, UINT8_MAX);
                        /*
                         * This is an LLC SAP value, so the frames
                         * that match would be 802.2 frames.
@@ -2524,6 +2525,7 @@ gen_ether_linktype(compiler_state_t *cstate, bpf_u_int32 ll_proto)
                        gen_and(b0, b1);
                        return b1;
                } else {
+                       assert_maxval(cstate, "EtherType", ll_proto, UINT16_MAX);
                        /*
                         * This is an Ethernet type, so compare
                         * the length/type field with it (if
@@ -2723,6 +2725,7 @@ gen_linux_sll_linktype(compiler_state_t *cstate, bpf_u_int32 ll_proto)
 
        default:
                if (ll_proto <= ETHERMTU) {
+                       assert_maxval(cstate, "LLC DSAP", ll_proto, UINT8_MAX);
                        /*
                         * This is an LLC SAP value, so the frames
                         * that match would be 802.2 frames.
@@ -2736,6 +2739,7 @@ gen_linux_sll_linktype(compiler_state_t *cstate, bpf_u_int32 ll_proto)
                        gen_and(b0, b1);
                        return b1;
                } else {
+                       assert_maxval(cstate, "EtherType", ll_proto, UINT16_MAX);
                        /*
                         * This is an Ethernet type, so compare
                         * the length/type field with it (if
@@ -3477,8 +3481,8 @@ gen_abs_offset_varpart(compiler_state_t *cstate, bpf_abs_offset *off)
 /*
  * Map an Ethernet type to the equivalent PPP type.
  */
-static bpf_u_int32
-ethertype_to_ppptype(bpf_u_int32 ll_proto)
+static uint16_t
+ethertype_to_ppptype(compiler_state_t *cstate, bpf_u_int32 ll_proto)
 {
        switch (ll_proto) {
 
@@ -3511,7 +3515,8 @@ ethertype_to_ppptype(bpf_u_int32 ll_proto)
        case LLCSAP_IPX:
                return PPP_IPX;
        }
-       return (ll_proto);
+       assert_maxval(cstate, "PPP protocol", ll_proto, UINT16_MAX);
+       return (uint16_t)ll_proto;
 }
 
 /*
@@ -3591,6 +3596,7 @@ gen_linktype(compiler_state_t *cstate, bpf_u_int32 ll_proto)
 
        case DLT_C_HDLC:
        case DLT_HDLC:
+               assert_maxval(cstate, "HDLC protocol", ll_proto, UINT16_MAX);
                switch (ll_proto) {
 
                case LLCSAP_ISONS:
@@ -3716,7 +3722,7 @@ gen_linktype(compiler_state_t *cstate, bpf_u_int32 ll_proto)
                 * map them to the corresponding PPP protocol types.
                 */
                return gen_cmp(cstate, OR_LINKTYPE, 0, BPF_H,
-                   ethertype_to_ppptype(ll_proto));
+                   ethertype_to_ppptype(cstate, ll_proto));
                /*NOTREACHED*/
 
        case DLT_PPP_BSDOS:
@@ -3740,7 +3746,7 @@ gen_linktype(compiler_state_t *cstate, bpf_u_int32 ll_proto)
 
                default:
                        return gen_cmp(cstate, OR_LINKTYPE, 0, BPF_H,
-                           ethertype_to_ppptype(ll_proto));
+                           ethertype_to_ppptype(cstate, ll_proto));
                }
                /*NOTREACHED*/
 
@@ -4017,6 +4023,7 @@ gen_linktype(compiler_state_t *cstate, bpf_u_int32 ll_proto)
                         * it's not, it needs to be handled specially
                         * above.)
                         */
+                       assert_maxval(cstate, "EtherType", ll_proto, UINT16_MAX);
                        return gen_cmp(cstate, OR_LINKTYPE, 0, BPF_H, ll_proto);
                        /*NOTREACHED */
                }
@@ -4320,12 +4327,14 @@ gen_llc_linktype(compiler_state_t *cstate, bpf_u_int32 ll_proto)
                 * here, but should we check for the IPX Ethertype?
                 */
                if (ll_proto <= ETHERMTU) {
+                       assert_maxval(cstate, "LLC DSAP", ll_proto, UINT8_MAX);
                        /*
                         * This is an LLC SAP value, so check
                         * the DSAP.
                         */
                        return gen_cmp(cstate, OR_LLC, 0, BPF_B, ll_proto);
                } else {
+                       assert_maxval(cstate, "EtherType", ll_proto, UINT16_MAX);
                        /*
                         * This is an Ethernet type; we assume that it's
                         * unlikely that it'll appear in the right place

diff --git a/testprogs/TESTrun b/testprogs/TESTrun
index 40c96c35be8c13391d4bb1a73103958956e6d56e..befd8aaefe55c101c4ffbadc9a226ec301f00261 100755 (executable)
--- a/testprogs/TESTrun
+++ b/testprogs/TESTrun
@@ -14234,6 +14234,70 @@ my @reject_tests = (
                expr => 'geneve 123456789',
                errstr => 'Geneve VNI 123456789 greater than maximum 16777215',
        },
+       # gen_linktype()
+       {
+               name => 'link_proto_65536_C_HDLC',
+               DLT => 'C_HDLC',
+               expr => 'link proto 65536',
+               errstr => 'HDLC protocol 65536 greater than maximum 65535',
+       },
+       {
+               name => 'link_proto_65536_PPP',
+               DLT => 'PPP',
+               expr => 'link proto 65536',
+               errstr => 'PPP protocol 65536 greater than maximum 65535',
+       },
+       {
+               name => 'link_proto_65536_PPP_BSDOS',
+               DLT => 'PPP_BSDOS',
+               expr => 'link proto 65536',
+               errstr => 'PPP protocol 65536 greater than maximum 65535',
+       },
+       {
+               name => 'link_proto_65536_APPLE_IP_OVER_IEEE1394',
+               DLT => 'APPLE_IP_OVER_IEEE1394', # the default case
+               expr => 'link proto 65536',
+               errstr => 'EtherType 65536 greater than maximum 65535',
+       },
+       # gen_ether_linktype()
+       {
+               name => 'link_proto_65536_EN10MB',
+               DLT => 'EN10MB',
+               expr => 'link proto 65536',
+               errstr => 'EtherType 65536 greater than maximum 65535',
+       },
+       {
+               name => 'link_proto_1500_EN10MB',
+               DLT => 'EN10MB',
+               expr => 'link proto 1500',
+               errstr => 'LLC DSAP 1500 greater than maximum 255',
+       },
+       # gen_llc_linktype
+       {
+               name => 'link_proto_65536_IP_OVER_FC',
+               DLT => 'IP_OVER_FC',
+               expr => 'link proto 65536',
+               errstr => 'EtherType 65536 greater than maximum 65535',
+       },
+       {
+               name => 'link_proto_1500_IP_OVER_FC',
+               DLT => 'IP_OVER_FC',
+               expr => 'link proto 1500',
+               errstr => 'LLC DSAP 1500 greater than maximum 255',
+       },
+       # gen_linux_sll_linktype
+       {
+               name => 'link_proto_65536_LINUX_SLL',
+               DLT => 'LINUX_SLL',
+               expr => 'link proto 65536',
+               errstr => 'EtherType 65536 greater than maximum 65535',
+       },
+       {
+               name => 'link_proto_1500_LINUX_SLL',
+               DLT => 'LINUX_SLL',
+               expr => 'link proto 1500',
+               errstr => 'LLC DSAP 1500 greater than maximum 255',
+       },
 );
 
 push @reject_tests, {