rose: Add length checks to CALL_REQUEST parsing

commit e0bccd315db0c2f919e7fcf9cb60db21d9986f52 upstream.

Define some constant offsets for CALL_REQUEST based on the description
at <http://www.techfest.com/networking/wan/x25plp.htm> and the
definition of ROSE as using 10-digit (5-byte) addresses.  Use them
consistently.  Validate all implicit and explicit facilities lengths.
Validate the address length byte rather than either trusting or
assuming its value.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

diff --git a/include/net/rose.h b/include/net/rose.h
index 5ba9f02731eb353921854816bd503bb04978c298..555dd198aab755c9e9ef21a751992ccf653b64c7 100644 (file)
--- a/include/net/rose.h
+++ b/include/net/rose.h
@@ -14,6 +14,12 @@
 
 #define        ROSE_MIN_LEN                    3
 
+#define        ROSE_CALL_REQ_ADDR_LEN_OFF      3
+#define        ROSE_CALL_REQ_ADDR_LEN_VAL      0xAA    /* each address is 10 digits */
+#define        ROSE_CALL_REQ_DEST_ADDR_OFF     4
+#define        ROSE_CALL_REQ_SRC_ADDR_OFF      9
+#define        ROSE_CALL_REQ_FACILITIES_OFF    14
+
 #define        ROSE_GFI                        0x10
 #define        ROSE_Q_BIT                      0x80
 #define        ROSE_D_BIT                      0x40
@@ -214,7 +220,7 @@ extern void rose_requeue_frames(struct sock *);
 extern int  rose_validate_nr(struct sock *, unsigned short);
 extern void rose_write_internal(struct sock *, int);
 extern int  rose_decode(struct sk_buff *, int *, int *, int *, int *, int *);
-extern int  rose_parse_facilities(unsigned char *, struct rose_facilities_struct *);
+extern int  rose_parse_facilities(unsigned char *, unsigned int, struct rose_facilities_struct *);
 extern void rose_disconnect(struct sock *, int, int, int);
 
 /* rose_timer.c */

diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index d325c9d93a07b2727e89f2a3cfdb3738004a12ef..547e5cd6d375e5fb7c73d5c2fc8ea3cd790b21ac 100644 (file)
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -985,7 +985,7 @@ int rose_rx_call_request(struct sk_buff *skb, struct net_device *dev, struct ros
        struct sock *make;
        struct rose_sock *make_rose;
        struct rose_facilities_struct facilities;
-       int n, len;
+       int n;
 
        skb->sk = NULL;         /* Initially we don't know who it's for */
 
@@ -994,9 +994,9 @@ int rose_rx_call_request(struct sk_buff *skb, struct net_device *dev, struct ros
         */
        memset(&facilities, 0x00, sizeof(struct rose_facilities_struct));
 
-       len  = (((skb->data[3] >> 4) & 0x0F) + 1) >> 1;
-       len += (((skb->data[3] >> 0) & 0x0F) + 1) >> 1;
-       if (!rose_parse_facilities(skb->data + len + 4, &facilities)) {
+       if (!rose_parse_facilities(skb->data + ROSE_CALL_REQ_FACILITIES_OFF,
+                                  skb->len - ROSE_CALL_REQ_FACILITIES_OFF,
+                                  &facilities)) {
                rose_transmit_clear_request(neigh, lci, ROSE_INVALID_FACILITY, 76);
                return 0;
        }

diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
index ae4a9d99aec70da821722a4e204ee6eacf1df872..344456206b70bfbfdfdac7a342687687db4cb0c8 100644 (file)
--- a/net/rose/rose_loopback.c
+++ b/net/rose/rose_loopback.c
@@ -73,9 +73,20 @@ static void rose_loopback_timer(unsigned long param)
        unsigned int lci_i, lci_o;
 
        while ((skb = skb_dequeue(&loopback_queue)) != NULL) {
+               if (skb->len < ROSE_MIN_LEN) {
+                       kfree_skb(skb);
+                       continue;
+               }
                lci_i     = ((skb->data[0] << 8) & 0xF00) + ((skb->data[1] << 0) & 0x0FF);
                frametype = skb->data[2];
-               dest      = (rose_address *)(skb->data + 4);
+               if (frametype == ROSE_CALL_REQUEST &&
+                   (skb->len <= ROSE_CALL_REQ_FACILITIES_OFF ||
+                    skb->data[ROSE_CALL_REQ_ADDR_LEN_OFF] !=
+                    ROSE_CALL_REQ_ADDR_LEN_VAL)) {
+                       kfree_skb(skb);
+                       continue;
+               }
+               dest      = (rose_address *)(skb->data + ROSE_CALL_REQ_DEST_ADDR_OFF);
                lci_o     = ROSE_DEFAULT_MAXVC + 1 - lci_i;
 
                skb_reset_transport_header(skb);

diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index cbc244a128bd1611829f6e943ae7a669cc81f2d3..ae3f06d37862549a078db3df0ea8cc86a010801c 100644 (file)
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -855,7 +855,7 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
        unsigned int lci, new_lci;
        unsigned char cause, diagnostic;
        struct net_device *dev;
-       int len, res = 0;
+       int res = 0;
        char buf[11];
 
 #if 0
@@ -863,10 +863,17 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
                return res;
 #endif
 
+       if (skb->len < ROSE_MIN_LEN)
+               return res;
        frametype = skb->data[2];
        lci = ((skb->data[0] << 8) & 0xF00) + ((skb->data[1] << 0) & 0x0FF);
-       src_addr  = (rose_address *)(skb->data + 9);
-       dest_addr = (rose_address *)(skb->data + 4);
+       if (frametype == ROSE_CALL_REQUEST &&
+           (skb->len <= ROSE_CALL_REQ_FACILITIES_OFF ||
+            skb->data[ROSE_CALL_REQ_ADDR_LEN_OFF] !=
+            ROSE_CALL_REQ_ADDR_LEN_VAL))
+               return res;
+       src_addr  = (rose_address *)(skb->data + ROSE_CALL_REQ_SRC_ADDR_OFF);
+       dest_addr = (rose_address *)(skb->data + ROSE_CALL_REQ_DEST_ADDR_OFF);
 
        spin_lock_bh(&rose_neigh_list_lock);
        spin_lock_bh(&rose_route_list_lock);
@@ -1004,12 +1011,11 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
                goto out;
        }
 
-       len  = (((skb->data[3] >> 4) & 0x0F) + 1) >> 1;
-       len += (((skb->data[3] >> 0) & 0x0F) + 1) >> 1;
-
        memset(&facilities, 0x00, sizeof(struct rose_facilities_struct));
 
-       if (!rose_parse_facilities(skb->data + len + 4, &facilities)) {
+       if (!rose_parse_facilities(skb->data + ROSE_CALL_REQ_FACILITIES_OFF,
+                                  skb->len - ROSE_CALL_REQ_FACILITIES_OFF,
+                                  &facilities)) {
                rose_transmit_clear_request(rose_neigh, lci, ROSE_INVALID_FACILITY, 76);
                goto out;
        }

diff --git a/net/rose/rose_subr.c b/net/rose/rose_subr.c
index 174d51c9ce37774f1a046ff49f0157d265e1a5bb..f6c71caa94b9d8b7fe2d7873264929218af2a97c 100644 (file)
--- a/net/rose/rose_subr.c
+++ b/net/rose/rose_subr.c
@@ -142,7 +142,7 @@ void rose_write_internal(struct sock *sk, int frametype)
                *dptr++ = ROSE_GFI | lci1;
                *dptr++ = lci2;
                *dptr++ = frametype;
-               *dptr++ = 0xAA;
+               *dptr++ = ROSE_CALL_REQ_ADDR_LEN_VAL;
                memcpy(dptr, &rose->dest_addr,  ROSE_ADDR_LEN);
                dptr   += ROSE_ADDR_LEN;
                memcpy(dptr, &rose->source_addr, ROSE_ADDR_LEN);
@@ -246,12 +246,16 @@ static int rose_parse_national(unsigned char *p, struct rose_facilities_struct *
        do {
                switch (*p & 0xC0) {
                case 0x00:
+                       if (len < 2)
+                               return -1;
                        p   += 2;
                        n   += 2;
                        len -= 2;
                        break;
 
                case 0x40:
+                       if (len < 3)
+                               return -1;
                        if (*p == FAC_NATIONAL_RAND)
                                facilities->rand = ((p[1] << 8) & 0xFF00) + ((p[2] << 0) & 0x00FF);
                        p   += 3;
@@ -260,32 +264,48 @@ static int rose_parse_national(unsigned char *p, struct rose_facilities_struct *
                        break;
 
                case 0x80:
+                       if (len < 4)
+                               return -1;
                        p   += 4;
                        n   += 4;
                        len -= 4;
                        break;
 
                case 0xC0:
+                       if (len < 2)
+                               return -1;
                        l = p[1];
+                       if (len < 2 + l)
+                               return -1;
                        if (*p == FAC_NATIONAL_DEST_DIGI) {
                                if (!fac_national_digis_received) {
+                                       if (l < AX25_ADDR_LEN)
+                                               return -1;
                                        memcpy(&facilities->source_digis[0], p + 2, AX25_ADDR_LEN);
                                        facilities->source_ndigis = 1;
                                }
                        }
                        else if (*p == FAC_NATIONAL_SRC_DIGI) {
                                if (!fac_national_digis_received) {
+                                       if (l < AX25_ADDR_LEN)
+                                               return -1;
                                        memcpy(&facilities->dest_digis[0], p + 2, AX25_ADDR_LEN);
                                        facilities->dest_ndigis = 1;
                                }
                        }
                        else if (*p == FAC_NATIONAL_FAIL_CALL) {
+                               if (l < AX25_ADDR_LEN)
+                                       return -1;
                                memcpy(&facilities->fail_call, p + 2, AX25_ADDR_LEN);
                        }
                        else if (*p == FAC_NATIONAL_FAIL_ADD) {
+                               if (l < 1 + ROSE_ADDR_LEN)
+                                       return -1;
                                memcpy(&facilities->fail_addr, p + 3, ROSE_ADDR_LEN);
                        }
                        else if (*p == FAC_NATIONAL_DIGIS) {
+                               if (l % AX25_ADDR_LEN)
+                                       return -1;
                                fac_national_digis_received = 1;
                                facilities->source_ndigis = 0;
                                facilities->dest_ndigis   = 0;
@@ -319,24 +339,32 @@ static int rose_parse_ccitt(unsigned char *p, struct rose_facilities_struct *fac
        do {
                switch (*p & 0xC0) {
                case 0x00:
+                       if (len < 2)
+                               return -1;
                        p   += 2;
                        n   += 2;
                        len -= 2;
                        break;
 
                case 0x40:
+                       if (len < 3)
+                               return -1;
                        p   += 3;
                        n   += 3;
                        len -= 3;
                        break;
 
                case 0x80:
+                       if (len < 4)
+                               return -1;
                        p   += 4;
                        n   += 4;
                        len -= 4;
                        break;
 
                case 0xC0:
+                       if (len < 2)
+                               return -1;
                        l = p[1];
 
                        /* Prevent overflows*/
@@ -365,49 +393,44 @@ static int rose_parse_ccitt(unsigned char *p, struct rose_facilities_struct *fac
        return n;
 }
 
-int rose_parse_facilities(unsigned char *p,
+int rose_parse_facilities(unsigned char *p, unsigned packet_len,
        struct rose_facilities_struct *facilities)
 {
        int facilities_len, len;
 
        facilities_len = *p++;
 
-       if (facilities_len == 0)
+       if (facilities_len == 0 || (unsigned)facilities_len > packet_len)
                return 0;
 
-       while (facilities_len > 0) {
-               if (*p == 0x00) {
-                       facilities_len--;
-                       p++;
-
-                       switch (*p) {
-                       case FAC_NATIONAL:              /* National */
-                               len = rose_parse_national(p + 1, facilities, facilities_len - 1);
-                               if (len < 0)
-                                       return 0;
-                               facilities_len -= len + 1;
-                               p += len + 1;
-                               break;
-
-                       case FAC_CCITT:         /* CCITT */
-                               len = rose_parse_ccitt(p + 1, facilities, facilities_len - 1);
-                               if (len < 0)
-                                       return 0;
-                               facilities_len -= len + 1;
-                               p += len + 1;
-                               break;
-
-                       default:
-                               printk(KERN_DEBUG "ROSE: rose_parse_facilities - unknown facilities family %02X\n", *p);
-                               facilities_len--;
-                               p++;
-                               break;
-                       }
-               } else
-                       break;  /* Error in facilities format */
+       while (facilities_len >= 3 && *p == 0x00) {
+               facilities_len--;
+               p++;
+
+               switch (*p) {
+               case FAC_NATIONAL:              /* National */
+                       len = rose_parse_national(p + 1, facilities, facilities_len - 1);
+                       break;
+
+               case FAC_CCITT:         /* CCITT */
+                       len = rose_parse_ccitt(p + 1, facilities, facilities_len - 1);
+                       break;
+
+               default:
+                       printk(KERN_DEBUG "ROSE: rose_parse_facilities - unknown facilities family %02X\n", *p);
+                       len = 1;
+                       break;
+               }
+
+               if (len < 0)
+                       return 0;
+               if (WARN_ON(len >= facilities_len))
+                       return 0;
+               facilities_len -= len + 1;
+               p += len + 1;
        }
 
-       return 1;
+       return facilities_len == 0;
 }
 
 static int rose_create_facilities(unsigned char *buffer, struct rose_sock *rose)