From cc4a7391c616be7a64ed65742ef9ed3f106eb165 Mon Sep 17 00:00:00 2001
From: Guy Harris <guy@alum.mit.edu>
Date: Sun, 5 Mar 2017 19:56:20 -0800
Subject: [PATCH] CVE-2017-13006/L2TP: Check whether an AVP's content exceeds
 the AVP length.

It's not good enough to check whether all the data specified by the AVP
length was captured - you also have to check whether that length is
large enough for all the required data in the AVP.

This fixes a buffer over-read discovered by Yannick Formaggio.

Add a test using the capture file supplied by the reporter(s).
---
 print-l2tp.c                 | 223 +++++++++++++++++++++++++++--------
 tests/TESTLIST               |   3 +
 tests/l2tp-avp-overflow.out  |  39 ++++++
 tests/l2tp-avp-overflow.pcap | Bin 0 -> 1584 bytes
 4 files changed, 219 insertions(+), 46 deletions(-)
 create mode 100644 tests/l2tp-avp-overflow.out
 create mode 100644 tests/l2tp-avp-overflow.pcap

Index: tcpdump-3.9.8/print-l2tp.c
===================================================================
--- tcpdump-3.9.8.orig/print-l2tp.c
+++ tcpdump-3.9.8/print-l2tp.c
@@ -267,10 +267,14 @@ print_32bits_val(const u_int32_t *dat)
 /* AVP-specific print out routines */
 /***********************************/
 static void
-l2tp_msgtype_print(const u_char *dat)
+l2tp_msgtype_print(const u_char *dat, u_int length)
 {
 	u_int16_t *ptr = (u_int16_t*)dat;
 
+	if (length < 2) {
+		printf("AVP too short");
+		return;
+	}
 	printf("%s", tok2str(l2tp_msgtype2str, "MSGTYPE-#%u",
 	    EXTRACT_16BITS(ptr)));
 }
@@ -280,28 +284,53 @@ l2tp_result_code_print(const u_char *dat
 {
 	u_int16_t *ptr = (u_int16_t *)dat;
 
-	printf("%u", EXTRACT_16BITS(ptr)); ptr++;	/* Result Code */
-	if (length > 2) {				/* Error Code (opt) */
-	        printf("/%u", EXTRACT_16BITS(ptr)); ptr++;
+	/* Result Code */
+	if (length < 2) {
+		printf("AVP too short");
+		return;
 	}
-	if (length > 4) {				/* Error Message (opt) */
-		printf(" ");
-		print_string((u_char *)ptr, length - 4);
+	printf("%u", EXTRACT_16BITS(ptr));
+	ptr++;
+	length -= 2;
+
+	/* Error Code (opt) */
+	if (length == 0)
+		return;
+	if (length < 2) {
+		printf(" AVP too short");
+		return;
 	}
+	printf("/%u", EXTRACT_16BITS(ptr));
+	ptr++;
+	length -= 2;
+
+	/* Error Message (opt) */
+	if (length == 0)
+		return;
+	printf(" ");
+	print_string((const u_char *)ptr, length);
 }
 
 static void
-l2tp_proto_ver_print(const u_int16_t *dat)
+l2tp_proto_ver_print(const u_int16_t *dat, u_int length)
 {
+	if (length < 2) {
+		printf("AVP too short");
+		return;
+	}
 	printf("%u.%u", (EXTRACT_16BITS(dat) >> 8),
 	    (EXTRACT_16BITS(dat) & 0xff));
 }
 
 static void
-l2tp_framing_cap_print(const u_char *dat)
+l2tp_framing_cap_print(const u_char *dat, u_int length)
 {
 	u_int32_t *ptr = (u_int32_t *)dat;
 
+	if (length < 4) {
+		printf("AVP too short");
+		return;
+	}
 	if (EXTRACT_32BITS(ptr) &  L2TP_FRAMING_CAP_ASYNC_MASK) {
 		printf("A");
 	}
@@ -311,10 +340,14 @@ l2tp_framing_cap_print(const u_char *dat
 }
 
 static void
-l2tp_bearer_cap_print(const u_char *dat)
+l2tp_bearer_cap_print(const u_char *dat, u_int length)
 {
 	u_int32_t *ptr = (u_int32_t *)dat;
 
+	if (length < 4) {
+		printf("AVP too short");
+		return;
+	}
 	if (EXTRACT_32BITS(ptr) &  L2TP_BEARER_CAP_ANALOG_MASK) {
 		printf("A");
 	}
@@ -326,19 +359,29 @@ l2tp_bearer_cap_print(const u_char *dat)
 static void
 l2tp_q931_cc_print(const u_char *dat, u_int length)
 {
+	if (length < 3) {
+		printf("AVP too short");
+		return;
+	}
 	print_16bits_val((u_int16_t *)dat);
 	printf(", %02x", dat[2]);
-	if (length > 3) {
+	dat += 3;
+	length -= 3;
+	if (length != 0) {
 		printf(" ");
-		print_string(dat+3, length-3);
+		print_string(dat, length);
 	}
 }
 
 static void
-l2tp_bearer_type_print(const u_char *dat)
+l2tp_bearer_type_print(const u_char *dat, u_int length)
 {
 	u_int32_t *ptr = (u_int32_t *)dat;
 
+	if (length < 4) {
+		printf("AVP too short");
+		return;
+	}
 	if (EXTRACT_32BITS(ptr) &  L2TP_BEARER_TYPE_ANALOG_MASK) {
 		printf("A");
 	}
@@ -348,10 +391,14 @@ l2tp_bearer_type_print(const u_char *dat
 }
 
 static void
-l2tp_framing_type_print(const u_char *dat)
+l2tp_framing_type_print(const u_char *dat, u_int length)
 {
 	u_int32_t *ptr = (u_int32_t *)dat;
 
+	if (length < 4) {
+		printf("AVP too short");
+		return;
+	}
 	if (EXTRACT_32BITS(ptr) &  L2TP_FRAMING_TYPE_ASYNC_MASK) {
 		printf("A");
 	}
@@ -367,67 +414,117 @@ l2tp_packet_proc_delay_print(void)
 }
 
 static void
-l2tp_proxy_auth_type_print(const u_char *dat)
+l2tp_proxy_auth_type_print(const u_char *dat, u_int length)
 {
 	u_int16_t *ptr = (u_int16_t *)dat;
 
+	if (length < 2) {
+		printf("AVP too short");
+		return;
+	}
 	printf("%s", tok2str(l2tp_authentype2str,
 			     "AuthType-#%u", EXTRACT_16BITS(ptr)));
 }
 
 static void
-l2tp_proxy_auth_id_print(const u_char *dat)
+l2tp_proxy_auth_id_print(const u_char *dat, u_int length)
 {
 	u_int16_t *ptr = (u_int16_t *)dat;
 
+	if (length < 2) {
+		printf("AVP too short");
+		return;
+	}
 	printf("%u", EXTRACT_16BITS(ptr) & L2TP_PROXY_AUTH_ID_MASK);
 }
 
 static void
-l2tp_call_errors_print(const u_char *dat)
+l2tp_call_errors_print(const u_char *dat, u_int length)
 {
 	u_int16_t *ptr = (u_int16_t *)dat;
 	u_int16_t val_h, val_l;
 
+	if (length < 2) {
+		printf("AVP too short");
+		return;
+	}
 	ptr++;		/* skip "Reserved" */
+	length -= 2;
 
-	val_h = EXTRACT_16BITS(ptr); ptr++;
-	val_l = EXTRACT_16BITS(ptr); ptr++;
+	if (length < 4) {
+		printf("AVP too short");
+		return;
+	}
+	val_h = EXTRACT_16BITS(ptr); ptr++; length -= 2;
+	val_l = EXTRACT_16BITS(ptr); ptr++; length -= 2;
 	printf("CRCErr=%u ", (val_h<<16) + val_l);
 
-	val_h = EXTRACT_16BITS(ptr); ptr++;
-	val_l = EXTRACT_16BITS(ptr); ptr++;
+	if (length < 4) {
+		printf("AVP too short");
+		return;
+	}
+	val_h = EXTRACT_16BITS(ptr); ptr++; length -= 2;
+	val_l = EXTRACT_16BITS(ptr); ptr++; length -= 2;
 	printf("FrameErr=%u ", (val_h<<16) + val_l);
 
-	val_h = EXTRACT_16BITS(ptr); ptr++;
-	val_l = EXTRACT_16BITS(ptr); ptr++;
+	if (length < 4) {
+		printf("AVP too short");
+		return;
+	}
+	val_h = EXTRACT_16BITS(ptr); ptr++; length -= 2;
+	val_l = EXTRACT_16BITS(ptr); ptr++; length -= 2;
 	printf("HardOver=%u ", (val_h<<16) + val_l);
 
-	val_h = EXTRACT_16BITS(ptr); ptr++;
-	val_l = EXTRACT_16BITS(ptr); ptr++;
+	if (length < 4) {
+		printf("AVP too short");
+		return;
+	}
+	val_h = EXTRACT_16BITS(ptr); ptr++; length -= 2;
+	val_l = EXTRACT_16BITS(ptr); ptr++; length -= 2;
 	printf("BufOver=%u ", (val_h<<16) + val_l);
 
-	val_h = EXTRACT_16BITS(ptr); ptr++;
-	val_l = EXTRACT_16BITS(ptr); ptr++;
+	if (length < 4) {
+		printf("AVP too short");
+		return;
+	}
+	val_h = EXTRACT_16BITS(ptr); ptr++; length -= 2;
+	val_l = EXTRACT_16BITS(ptr); ptr++; length -= 2;
 	printf("Timeout=%u ", (val_h<<16) + val_l);
 
+	if (length < 4) {
+		printf("AVP too short");
+		return;
+	}
 	val_h = EXTRACT_16BITS(ptr); ptr++;
 	val_l = EXTRACT_16BITS(ptr); ptr++;
 	printf("AlignErr=%u ", (val_h<<16) + val_l);
 }
 
 static void
-l2tp_accm_print(const u_char *dat)
+l2tp_accm_print(const u_char *dat, u_int length)
 {
 	u_int16_t *ptr = (u_int16_t *)dat;
 	u_int16_t val_h, val_l;
 
+	if (length < 2) {
+		printf("AVP too short");
+		return;
+	}
 	ptr++;		/* skip "Reserved" */
+	length -= 2;
 
-	val_h = EXTRACT_16BITS(ptr); ptr++;
-	val_l = EXTRACT_16BITS(ptr); ptr++;
+	if (length < 4) {
+		printf("AVP too short");
+		return;
+	}
+	val_h = EXTRACT_16BITS(ptr); ptr++; length -= 2;
+	val_l = EXTRACT_16BITS(ptr); ptr++; length -= 2;
 	printf("send=%08x ", (val_h<<16) + val_l);
 
+	if (length < 4) {
+		printf("AVP too short");
+		return;
+	}
 	val_h = EXTRACT_16BITS(ptr); ptr++;
 	val_l = EXTRACT_16BITS(ptr); ptr++;
 	printf("recv=%08x ", (val_h<<16) + val_l);
@@ -438,14 +535,27 @@ l2tp_ppp_discon_cc_print(const u_char *d
 {
 	u_int16_t *ptr = (u_int16_t *)dat;
 
-	printf("%04x, ", EXTRACT_16BITS(ptr)); ptr++;	/* Disconnect Code */
-	printf("%04x ",  EXTRACT_16BITS(ptr)); ptr++;	/* Control Protocol Number */
+	if (length < 5) {
+		printf("AVP too short");
+		return;
+	}
+	/* Disconnect Code */
+	printf("%04x, ", EXTRACT_16BITS(dat));
+	dat += 2;
+	length -= 2;
+	/* Control Protocol Number */
+	printf("%04x ",  EXTRACT_16BITS(dat));
+	dat += 2;
+	length -= 2;
+	/* Direction */
 	printf("%s", tok2str(l2tp_cc_direction2str,
-			     "Direction-#%u", *((u_char *)ptr++)));
+			     "Direction-#%u", EXTRACT_8BITS(ptr)));
+	ptr++;
+	length--;
 
-	if (length > 5) {
+	if (length != 0) {
 		printf(" ");
-		print_string((const u_char *)ptr, length-5);
+		print_string((const u_char *)ptr, length);
 	}
 }
 
@@ -478,7 +588,12 @@ l2tp_avp_print(const u_char *dat, int le
 	/* If it goes past the end of the remaining length of the captured
 	   data, we'll give up. */
 	TCHECK2(*ptr, len);
-	/* After this point, no need to worry about truncation */
+
+	/*
+	 * After this point, we don't need to check whether we go past
+	 * the length of the captured data; however, we *do* need to
+	 * check whether we go past the end of the AVP.
+	 */
 
 	if (EXTRACT_16BITS(ptr) & L2TP_AVP_HDR_FLAG_MANDATORY) {
 		printf("*");
@@ -507,27 +622,35 @@ l2tp_avp_print(const u_char *dat, int le
 		} else {
 			switch (attr_type) {
 			case L2TP_AVP_MSGTYPE:
-				l2tp_msgtype_print((u_char *)ptr);
+				l2tp_msgtype_print((u_char *)ptr, len-6);
 				break;
 			case L2TP_AVP_RESULT_CODE:
 				l2tp_result_code_print((u_char *)ptr, len-6);
 				break;
 			case L2TP_AVP_PROTO_VER:
-				l2tp_proto_ver_print(ptr);
+				l2tp_proto_ver_print(ptr, len-6);
 				break;
 			case L2TP_AVP_FRAMING_CAP:
-				l2tp_framing_cap_print((u_char *)ptr);
+				l2tp_framing_cap_print((u_char *)ptr, len-6);
 				break;
 			case L2TP_AVP_BEARER_CAP:
-				l2tp_bearer_cap_print((u_char *)ptr);
+				l2tp_bearer_cap_print((u_char *)ptr, len-6);
 				break;
 			case L2TP_AVP_TIE_BREAKER:
+				if (len-6 < 8) {
+					printf("AVP too short");
+					break;
+				}
 				print_octets((u_char *)ptr, 8);
 				break;
 			case L2TP_AVP_FIRM_VER:
 			case L2TP_AVP_ASSND_TUN_ID:
 			case L2TP_AVP_RECV_WIN_SIZE:
 			case L2TP_AVP_ASSND_SESS_ID:
+				if (len-6 < 2) {
+					printf("AVP too short");
+					break;
+				}
 				print_16bits_val(ptr);
 				break;
 			case L2TP_AVP_HOST_NAME:
@@ -552,6 +675,10 @@ l2tp_avp_print(const u_char *dat, int le
 				l2tp_q931_cc_print((u_char *)ptr, len-6);
 				break;
 			case L2TP_AVP_CHALLENGE_RESP:
+				if (len-6 < 16) {
+					printf("AVP too short");
+					break;
+				}
 				print_octets((u_char *)ptr, 16);
 				break;
 			case L2TP_AVP_CALL_SER_NUM:
@@ -560,28 +687,32 @@ l2tp_avp_print(const u_char *dat, int le
 			case L2TP_AVP_TX_CONN_SPEED:
 			case L2TP_AVP_PHY_CHANNEL_ID:
 			case L2TP_AVP_RX_CONN_SPEED:
+				if (len-6 < 4) {
+					printf("AVP too short");
+					break;
+				}
 				print_32bits_val((u_int32_t *)ptr);
 				break;
 			case L2TP_AVP_BEARER_TYPE:
-				l2tp_bearer_type_print((u_char *)ptr);
+				l2tp_bearer_type_print((u_char *)ptr, len-6);
 				break;
 			case L2TP_AVP_FRAMING_TYPE:
-				l2tp_framing_type_print((u_char *)ptr);
+				l2tp_framing_type_print((u_char *)ptr, len-6);
 				break;
 			case L2TP_AVP_PACKET_PROC_DELAY:
 				l2tp_packet_proc_delay_print();
 				break;
 			case L2TP_AVP_PROXY_AUTH_TYPE:
-				l2tp_proxy_auth_type_print((u_char *)ptr);
+				l2tp_proxy_auth_type_print((u_char *)ptr, len-6);
 				break;
 			case L2TP_AVP_PROXY_AUTH_ID:
-				l2tp_proxy_auth_id_print((u_char *)ptr);
+				l2tp_proxy_auth_id_print((u_char *)ptr, len-6);
 				break;
 			case L2TP_AVP_CALL_ERRORS:
-				l2tp_call_errors_print((u_char *)ptr);
+				l2tp_call_errors_print((u_char *)ptr, len-6);
 				break;
 			case L2TP_AVP_ACCM:
-				l2tp_accm_print((u_char *)ptr);
+				l2tp_accm_print((u_char *)ptr, len-6);
 				break;
 			case L2TP_AVP_SEQ_REQUIRED:
 				break;	/* No Attribute Value */
