CVE-2011-4914: Improper Input Validation (CWE-20)
value: The ROSE protocol implementation in the Linux kernel before 2.6.39 does not verify that certain data-length values are consistent with the amount of data sent, which might allow remote attackers to obtain sensitive information from kernel memory or cause a denial of service (out-of-bounds read) via crafted data to a ROSE socket.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
Published: 2012-06-21T23:55Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: include/net/rose.h, net/rose/af_rose.c, net/rose/rose_loopback.c, net/rose/rose_route.c, net/rose/rose_subr.c
Commit date: 2011-03-27 17:59:04 -0700
Commit message: rose: Add length checks to CALL_REQUEST parsing

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>

Diff: diff --git a/include/net/rose.h b/include/net/rose.h
index 5ba9f02731eb3..555dd198aab75 100644
--- a/include/net/rose.h
+++ b/include/net/rose.h
@@ -12,10 +12,16 @@
 
 #define	ROSE_ADDR_LEN			5
 
 #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
 #define	ROSE_M_BIT			0x10
 
@@ -212,11 +218,11 @@ extern void rose_clear_queues(struct sock *);
 extern void rose_frames_acked(struct sock *, unsigned short);
 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 */
 extern void rose_start_heartbeat(struct sock *);
 extern void rose_start_t1timer(struct sock *);
diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index 5ee0c62046a03..a80aef6e3d1fb 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -976,22 +976,22 @@ int rose_rx_call_request(struct sk_buff *skb, struct net_device *dev, struct ros
 {
 	struct sock *sk;
 	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 */
 
 	/*
 	 *	skb->data points to the rose frame start
 	 */
 	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;
 	}
 
 	sk = rose_find_listener(&facilities.source_addr, &facilities.source_call);
diff --git a/net/rose/rose_loopback.c b/net/rose/rose_loopback.c
index ae4a9d99aec70..344456206b70b 100644
--- a/net/rose/rose_loopback.c
+++ b/net/rose/rose_loopback.c
@@ -71,13 +71,24 @@ static void rose_loopback_timer(unsigned long param)
 	struct sock *sk;
 	unsigned short frametype;
 	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);
 
 		sk = rose_find_socket(lci_o, rose_loopback_neigh);
diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 88a77e90e7e86..08dcd2f29cdc7 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -859,22 +859,29 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
 	struct sock *sk;
 	unsigned short frametype;
 	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
 	if (call_in_firewall(PF_ROSE, skb->dev, skb->data, NULL, &skb) != FW_ACCEPT)
 		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);
 
 	rose_neigh = rose_neigh_list;
@@ -1008,16 +1015,15 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
 	if (frametype != ROSE_CALL_REQUEST) {	/* XXX */
 		res = 0;
 		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 174d51c9ce377..f6c71caa94b9d 100644
--- a/net/rose/rose_subr.c
+++ b/net/rose/rose_subr.c
@@ -140,11 +140,11 @@ void rose_write_internal(struct sock *sk, int frametype)
 	switch (frametype) {
 	case ROSE_CALL_REQUEST:
 		*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);
 		dptr   += ROSE_ADDR_LEN;
 		memcpy(dptr, buffer, faclen);
@@ -244,50 +244,70 @@ static int rose_parse_national(unsigned char *p, struct rose_facilities_struct *
 	int fac_national_digis_received = 0;
 
 	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;
 			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];
+			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;
 				for (pt = p + 2, lg = 0 ; lg < l ; pt += AX25_ADDR_LEN, lg += AX25_ADDR_LEN) {
 					if (pt[6] & AX25_HBIT) {
@@ -317,28 +337,36 @@ static int rose_parse_ccitt(unsigned char *p, struct rose_facilities_struct *fac
 	char callsign[11];
 
 	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*/
 			if (l < 10 || l > 20)
 				return -1;
@@ -363,53 +391,48 @@ static int rose_parse_ccitt(unsigned char *p, struct rose_facilities_struct *fac
 	} while (*p != 0x00 && len > 0);
 
 	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)
 {
 	unsigned char *p = buffer + 1;

Vulnerable code: extern int  rose_parse_facilities(unsigned char *, struct rose_facilities_struct *);
	int n, len;
	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)) {
		dest      = (rose_address *)(skb->data + 4);
	int len, res = 0;
	src_addr  = (rose_address *)(skb->data + 9);
	dest_addr = (rose_address *)(skb->data + 4);
	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)) {
		*dptr++ = 0xAA;
int rose_parse_facilities(unsigned char *p,
	if (facilities_len == 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 */
	return 1;
Fixed code: #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

extern int  rose_parse_facilities(unsigned char *, unsigned int, struct rose_facilities_struct *);
	int n;
	if (!rose_parse_facilities(skb->data + ROSE_CALL_REQ_FACILITIES_OFF,
				   skb->len - ROSE_CALL_REQ_FACILITIES_OFF,
				   &facilities)) {
		if (skb->len < ROSE_MIN_LEN) {
			kfree_skb(skb);
			continue;
		}
		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);
	int res = 0;
	if (skb->len < ROSE_MIN_LEN)
		return res;
	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);
	if (!rose_parse_facilities(skb->data + ROSE_CALL_REQ_FACILITIES_OFF,
				   skb->len - ROSE_CALL_REQ_FACILITIES_OFF,
				   &facilities)) {
		*dptr++ = ROSE_CALL_REQ_ADDR_LEN_VAL;
			if (len < 2)
				return -1;
			if (len < 3)
				return -1;
			if (len < 4)
				return -1;
			if (len < 2)
				return -1;
			if (len < 2 + l)
				return -1;
					if (l < AX25_ADDR_LEN)
						return -1;
					if (l < AX25_ADDR_LEN)
						return -1;
				if (l < AX25_ADDR_LEN)
					return -1;
				if (l < 1 + ROSE_ADDR_LEN)
					return -1;
				if (l % AX25_ADDR_LEN)
					return -1;
			if (len < 2)
				return -1;
			if (len < 3)
				return -1;
			if (len < 4)
				return -1;
			if (len < 2)
				return -1;
int rose_parse_facilities(unsigned char *p, unsigned packet_len,
	if (facilities_len == 0 || (unsigned)facilities_len > packet_len)
	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 facilities_len == 0;