From: Guy Harris Date: Mon, 6 Mar 2017 03:56:20 +0000 (-0800) Subject: CVE-2017-13006/L2TP: Check whether an AVP's content exceeds the AVP length. X-Git-Tag: tcpdump-4.99-bp~1931 X-Git-Url: https://git.tcpdump.org/tcpdump/commitdiff_plain/cc4a7391c616be7a64ed65742ef9ed3f106eb165 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). --- diff --git a/print-l2tp.c b/print-l2tp.c index 42ae3910..d70d434f 100644 --- a/print-l2tp.c +++ b/print-l2tp.c @@ -297,10 +297,14 @@ print_32bits_val(netdissect_options *ndo, const uint32_t *dat) /* AVP-specific print out routines */ /***********************************/ static void -l2tp_msgtype_print(netdissect_options *ndo, const u_char *dat) +l2tp_msgtype_print(netdissect_options *ndo, const u_char *dat, u_int length) { const uint16_t *ptr = (const uint16_t *)dat; + if (length < 2) { + ND_PRINT((ndo, "AVP too short")); + return; + } ND_PRINT((ndo, "%s", tok2str(l2tp_msgtype2str, "MSGTYPE-#%u", EXTRACT_16BITS(ptr)))); } @@ -310,28 +314,53 @@ l2tp_result_code_print(netdissect_options *ndo, const u_char *dat, u_int length) { const uint16_t *ptr = (const uint16_t *)dat; - ND_PRINT((ndo, "%u", EXTRACT_16BITS(ptr))); ptr++; /* Result Code */ - if (length > 2) { /* Error Code (opt) */ - ND_PRINT((ndo, "/%u", EXTRACT_16BITS(ptr))); ptr++; + /* Result Code */ + if (length < 2) { + ND_PRINT((ndo, "AVP too short")); + return; } - if (length > 4) { /* Error Message (opt) */ - ND_PRINT((ndo, " ")); - print_string(ndo, (const u_char *)ptr, length - 4); + ND_PRINT((ndo, "%u", EXTRACT_16BITS(ptr))); + ptr++; + length -= 2; + + /* Error Code (opt) */ + if (length == 0) + return; + if (length < 2) { + ND_PRINT((ndo, " AVP too short")); + return; } + ND_PRINT((ndo, "/%u", EXTRACT_16BITS(ptr))); + ptr++; + length -= 2; + + /* Error Message (opt) */ + if (length == 0) + return; + ND_PRINT((ndo, " ")); + print_string(ndo, (const u_char *)ptr, length); } static void -l2tp_proto_ver_print(netdissect_options *ndo, const uint16_t *dat) +l2tp_proto_ver_print(netdissect_options *ndo, const uint16_t *dat, u_int length) { + if (length < 2) { + ND_PRINT((ndo, "AVP too short")); + return; + } ND_PRINT((ndo, "%u.%u", (EXTRACT_16BITS(dat) >> 8), (EXTRACT_16BITS(dat) & 0xff))); } static void -l2tp_framing_cap_print(netdissect_options *ndo, const u_char *dat) +l2tp_framing_cap_print(netdissect_options *ndo, const u_char *dat, u_int length) { const uint32_t *ptr = (const uint32_t *)dat; + if (length < 4) { + ND_PRINT((ndo, "AVP too short")); + return; + } if (EXTRACT_32BITS(ptr) & L2TP_FRAMING_CAP_ASYNC_MASK) { ND_PRINT((ndo, "A")); } @@ -341,10 +370,14 @@ l2tp_framing_cap_print(netdissect_options *ndo, const u_char *dat) } static void -l2tp_bearer_cap_print(netdissect_options *ndo, const u_char *dat) +l2tp_bearer_cap_print(netdissect_options *ndo, const u_char *dat, u_int length) { const uint32_t *ptr = (const uint32_t *)dat; + if (length < 4) { + ND_PRINT((ndo, "AVP too short")); + return; + } if (EXTRACT_32BITS(ptr) & L2TP_BEARER_CAP_ANALOG_MASK) { ND_PRINT((ndo, "A")); } @@ -356,19 +389,29 @@ l2tp_bearer_cap_print(netdissect_options *ndo, const u_char *dat) static void l2tp_q931_cc_print(netdissect_options *ndo, const u_char *dat, u_int length) { + if (length < 3) { + ND_PRINT((ndo, "AVP too short")); + return; + } print_16bits_val(ndo, (const uint16_t *)dat); ND_PRINT((ndo, ", %02x", dat[2])); - if (length > 3) { + dat += 3; + length -= 3; + if (length != 0) { ND_PRINT((ndo, " ")); - print_string(ndo, dat+3, length-3); + print_string(ndo, dat, length); } } static void -l2tp_bearer_type_print(netdissect_options *ndo, const u_char *dat) +l2tp_bearer_type_print(netdissect_options *ndo, const u_char *dat, u_int length) { const uint32_t *ptr = (const uint32_t *)dat; + if (length < 4) { + ND_PRINT((ndo, "AVP too short")); + return; + } if (EXTRACT_32BITS(ptr) & L2TP_BEARER_TYPE_ANALOG_MASK) { ND_PRINT((ndo, "A")); } @@ -378,10 +421,14 @@ l2tp_bearer_type_print(netdissect_options *ndo, const u_char *dat) } static void -l2tp_framing_type_print(netdissect_options *ndo, const u_char *dat) +l2tp_framing_type_print(netdissect_options *ndo, const u_char *dat, u_int length) { const uint32_t *ptr = (const uint32_t *)dat; + if (length < 4) { + ND_PRINT((ndo, "AVP too short")); + return; + } if (EXTRACT_32BITS(ptr) & L2TP_FRAMING_TYPE_ASYNC_MASK) { ND_PRINT((ndo, "A")); } @@ -397,67 +444,117 @@ l2tp_packet_proc_delay_print(netdissect_options *ndo) } static void -l2tp_proxy_auth_type_print(netdissect_options *ndo, const u_char *dat) +l2tp_proxy_auth_type_print(netdissect_options *ndo, const u_char *dat, u_int length) { const uint16_t *ptr = (const uint16_t *)dat; + if (length < 2) { + ND_PRINT((ndo, "AVP too short")); + return; + } ND_PRINT((ndo, "%s", tok2str(l2tp_authentype2str, "AuthType-#%u", EXTRACT_16BITS(ptr)))); } static void -l2tp_proxy_auth_id_print(netdissect_options *ndo, const u_char *dat) +l2tp_proxy_auth_id_print(netdissect_options *ndo, const u_char *dat, u_int length) { const uint16_t *ptr = (const uint16_t *)dat; + if (length < 2) { + ND_PRINT((ndo, "AVP too short")); + return; + } ND_PRINT((ndo, "%u", EXTRACT_16BITS(ptr) & L2TP_PROXY_AUTH_ID_MASK)); } static void -l2tp_call_errors_print(netdissect_options *ndo, const u_char *dat) +l2tp_call_errors_print(netdissect_options *ndo, const u_char *dat, u_int length) { const uint16_t *ptr = (const uint16_t *)dat; uint16_t val_h, val_l; + if (length < 2) { + ND_PRINT((ndo, "AVP too short")); + return; + } ptr++; /* skip "Reserved" */ + length -= 2; - val_h = EXTRACT_16BITS(ptr); ptr++; - val_l = EXTRACT_16BITS(ptr); ptr++; + if (length < 4) { + ND_PRINT((ndo, "AVP too short")); + return; + } + val_h = EXTRACT_16BITS(ptr); ptr++; length -= 2; + val_l = EXTRACT_16BITS(ptr); ptr++; length -= 2; ND_PRINT((ndo, "CRCErr=%u ", (val_h<<16) + val_l)); - val_h = EXTRACT_16BITS(ptr); ptr++; - val_l = EXTRACT_16BITS(ptr); ptr++; + if (length < 4) { + ND_PRINT((ndo, "AVP too short")); + return; + } + val_h = EXTRACT_16BITS(ptr); ptr++; length -= 2; + val_l = EXTRACT_16BITS(ptr); ptr++; length -= 2; ND_PRINT((ndo, "FrameErr=%u ", (val_h<<16) + val_l)); - val_h = EXTRACT_16BITS(ptr); ptr++; - val_l = EXTRACT_16BITS(ptr); ptr++; + if (length < 4) { + ND_PRINT((ndo, "AVP too short")); + return; + } + val_h = EXTRACT_16BITS(ptr); ptr++; length -= 2; + val_l = EXTRACT_16BITS(ptr); ptr++; length -= 2; ND_PRINT((ndo, "HardOver=%u ", (val_h<<16) + val_l)); - val_h = EXTRACT_16BITS(ptr); ptr++; - val_l = EXTRACT_16BITS(ptr); ptr++; + if (length < 4) { + ND_PRINT((ndo, "AVP too short")); + return; + } + val_h = EXTRACT_16BITS(ptr); ptr++; length -= 2; + val_l = EXTRACT_16BITS(ptr); ptr++; length -= 2; ND_PRINT((ndo, "BufOver=%u ", (val_h<<16) + val_l)); - val_h = EXTRACT_16BITS(ptr); ptr++; - val_l = EXTRACT_16BITS(ptr); ptr++; + if (length < 4) { + ND_PRINT((ndo, "AVP too short")); + return; + } + val_h = EXTRACT_16BITS(ptr); ptr++; length -= 2; + val_l = EXTRACT_16BITS(ptr); ptr++; length -= 2; ND_PRINT((ndo, "Timeout=%u ", (val_h<<16) + val_l)); + if (length < 4) { + ND_PRINT((ndo, "AVP too short")); + return; + } val_h = EXTRACT_16BITS(ptr); ptr++; val_l = EXTRACT_16BITS(ptr); ptr++; ND_PRINT((ndo, "AlignErr=%u ", (val_h<<16) + val_l)); } static void -l2tp_accm_print(netdissect_options *ndo, const u_char *dat) +l2tp_accm_print(netdissect_options *ndo, const u_char *dat, u_int length) { const uint16_t *ptr = (const uint16_t *)dat; uint16_t val_h, val_l; + if (length < 2) { + ND_PRINT((ndo, "AVP too short")); + return; + } ptr++; /* skip "Reserved" */ + length -= 2; - val_h = EXTRACT_16BITS(ptr); ptr++; - val_l = EXTRACT_16BITS(ptr); ptr++; + if (length < 4) { + ND_PRINT((ndo, "AVP too short")); + return; + } + val_h = EXTRACT_16BITS(ptr); ptr++; length -= 2; + val_l = EXTRACT_16BITS(ptr); ptr++; length -= 2; ND_PRINT((ndo, "send=%08x ", (val_h<<16) + val_l)); + if (length < 4) { + ND_PRINT((ndo, "AVP too short")); + return; + } val_h = EXTRACT_16BITS(ptr); ptr++; val_l = EXTRACT_16BITS(ptr); ptr++; ND_PRINT((ndo, "recv=%08x ", (val_h<<16) + val_l)); @@ -468,14 +565,27 @@ l2tp_ppp_discon_cc_print(netdissect_options *ndo, const u_char *dat, u_int lengt { const uint16_t *ptr = (const uint16_t *)dat; - ND_PRINT((ndo, "%04x, ", EXTRACT_16BITS(ptr))); ptr++; /* Disconnect Code */ - ND_PRINT((ndo, "%04x ", EXTRACT_16BITS(ptr))); ptr++; /* Control Protocol Number */ + if (length < 5) { + ND_PRINT((ndo, "AVP too short")); + return; + } + /* Disconnect Code */ + ND_PRINT((ndo, "%04x, ", EXTRACT_16BITS(dat))); + dat += 2; + length -= 2; + /* Control Protocol Number */ + ND_PRINT((ndo, "%04x ", EXTRACT_16BITS(dat))); + dat += 2; + length -= 2; + /* Direction */ ND_PRINT((ndo, "%s", tok2str(l2tp_cc_direction2str, - "Direction-#%u", *((const u_char *)ptr++)))); + "Direction-#%u", EXTRACT_8BITS(ptr)))); + ptr++; + length--; - if (length > 5) { + if (length != 0) { ND_PRINT((ndo, " ")); - print_string(ndo, (const u_char *)ptr, length-5); + print_string(ndo, (const u_char *)ptr, length); } } @@ -508,7 +618,12 @@ l2tp_avp_print(netdissect_options *ndo, const u_char *dat, int length) /* If it goes past the end of the remaining length of the captured data, we'll give up. */ ND_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) { ND_PRINT((ndo, "*")); @@ -537,27 +652,35 @@ l2tp_avp_print(netdissect_options *ndo, const u_char *dat, int length) } else { switch (attr_type) { case L2TP_AVP_MSGTYPE: - l2tp_msgtype_print(ndo, (const u_char *)ptr); + l2tp_msgtype_print(ndo, (const u_char *)ptr, len-6); break; case L2TP_AVP_RESULT_CODE: l2tp_result_code_print(ndo, (const u_char *)ptr, len-6); break; case L2TP_AVP_PROTO_VER: - l2tp_proto_ver_print(ndo, ptr); + l2tp_proto_ver_print(ndo, ptr, len-6); break; case L2TP_AVP_FRAMING_CAP: - l2tp_framing_cap_print(ndo, (const u_char *)ptr); + l2tp_framing_cap_print(ndo, (const u_char *)ptr, len-6); break; case L2TP_AVP_BEARER_CAP: - l2tp_bearer_cap_print(ndo, (const u_char *)ptr); + l2tp_bearer_cap_print(ndo, (const u_char *)ptr, len-6); break; case L2TP_AVP_TIE_BREAKER: + if (len-6 < 8) { + ND_PRINT((ndo, "AVP too short")); + break; + } print_octets(ndo, (const 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) { + ND_PRINT((ndo, "AVP too short")); + break; + } print_16bits_val(ndo, ptr); break; case L2TP_AVP_HOST_NAME: @@ -582,6 +705,10 @@ l2tp_avp_print(netdissect_options *ndo, const u_char *dat, int length) l2tp_q931_cc_print(ndo, (const u_char *)ptr, len-6); break; case L2TP_AVP_CHALLENGE_RESP: + if (len-6 < 16) { + ND_PRINT((ndo, "AVP too short")); + break; + } print_octets(ndo, (const u_char *)ptr, 16); break; case L2TP_AVP_CALL_SER_NUM: @@ -590,28 +717,32 @@ l2tp_avp_print(netdissect_options *ndo, const u_char *dat, int length) case L2TP_AVP_TX_CONN_SPEED: case L2TP_AVP_PHY_CHANNEL_ID: case L2TP_AVP_RX_CONN_SPEED: + if (len-6 < 4) { + ND_PRINT((ndo, "AVP too short")); + break; + } print_32bits_val(ndo, (const uint32_t *)ptr); break; case L2TP_AVP_BEARER_TYPE: - l2tp_bearer_type_print(ndo, (const u_char *)ptr); + l2tp_bearer_type_print(ndo, (const u_char *)ptr, len-6); break; case L2TP_AVP_FRAMING_TYPE: - l2tp_framing_type_print(ndo, (const u_char *)ptr); + l2tp_framing_type_print(ndo, (const u_char *)ptr, len-6); break; case L2TP_AVP_PACKET_PROC_DELAY: l2tp_packet_proc_delay_print(ndo); break; case L2TP_AVP_PROXY_AUTH_TYPE: - l2tp_proxy_auth_type_print(ndo, (const u_char *)ptr); + l2tp_proxy_auth_type_print(ndo, (const u_char *)ptr, len-6); break; case L2TP_AVP_PROXY_AUTH_ID: - l2tp_proxy_auth_id_print(ndo, (const u_char *)ptr); + l2tp_proxy_auth_id_print(ndo, (const u_char *)ptr, len-6); break; case L2TP_AVP_CALL_ERRORS: - l2tp_call_errors_print(ndo, (const u_char *)ptr); + l2tp_call_errors_print(ndo, (const u_char *)ptr, len-6); break; case L2TP_AVP_ACCM: - l2tp_accm_print(ndo, (const u_char *)ptr); + l2tp_accm_print(ndo, (const u_char *)ptr, len-6); break; case L2TP_AVP_SEQ_REQUIRED: break; /* No Attribute Value */ diff --git a/tests/TESTLIST b/tests/TESTLIST index 8ae61099..f9ae40dd 100644 --- a/tests/TESTLIST +++ b/tests/TESTLIST @@ -500,6 +500,9 @@ lmpv1_busyloop lmpv1_busyloop.pcap lmpv1_busyloop.out -vvv -e juniper_atm1 juniper_atm1.pcap juniper_atm1.out -vvv -e juniper_es juniper_es.pcap juniper_es.out -vvv -e +# bad packets from Yannick Formaggio +l2tp-avp-overflow l2tp-avp-overflow.pcap l2tp-avp-overflow.out -v + # RTP tests # fuzzed pcap rtp-seg-fault-1 rtp-seg-fault-1.pcap rtp-seg-fault-1.out -v -T rtp diff --git a/tests/l2tp-avp-overflow.out b/tests/l2tp-avp-overflow.out new file mode 100644 index 00000000..a3b1db2d --- /dev/null +++ b/tests/l2tp-avp-overflow.out @@ -0,0 +1,39 @@ +IP (tos 0x30, ttl 48, id 12331, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->ab7b)!) + 127.0.0.229.12416 > 127.0.128.1.1701: l2tp:[TL](560/2056) AVP-#60963() |... +IP (tos 0x30, ttl 48, id 12336, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->2a8b)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) AVP-#48() |... +IP (tos 0x30, ttl 48, id 12331, offset 0, flags [none], proto UDP (17), length 8752, bad cksum 1f51 (->3890)!) + 127.0.0.229.32767 > 127.236.0.1.1701: l2tp:[TL](560/2056) ACCM(AVP too short) |... +IP (tos 0x30, ttl 48, id 12336, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->2a8b)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) ACCM(AVP too short) |... +IP (tos 0x30, ttl 48, id 12331, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->2a90)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) ACCM(AVP too short) |... +IP (tos 0x30, ttl 48, id 12336, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->ab5d)!) + 127.0.0.0.0 > 0.0.0.0.2048: UDP, bad length 17704 > 12308 +[|ether] +IP (tos 0x30, ttl 48, id 12336, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->2a8b)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) AVP-#48() |... +IP (tos 0x30, ttl 48, id 12331, offset 0, flags [none], proto UDP (17), length 8752, bad cksum 1f51 (->3890)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) ACCM(AVP too short) |... +IP (tos 0x30, ttl 48, id 12336, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->2a8b)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) ACCM(AVP too short) |... +IP (tos 0x30, ttl 48, id 12331, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->2a90)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) ACCM(AVP too short) |... +IP (tos 0x30, ttl 48, id 12336, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->ab5d)!) + 127.0.0.0.0 > 0.0.0.0.2048: UDP, bad length 17704 > 12308 +[|ether] +IP (tos 0x30, ttl 48, id 12336, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->2a8b)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) AVP-#48() |... +IP (tos 0x30, ttl 48, id 12331, offset 0, flags [none], proto UDP (17), length 8752, bad cksum 1f51 (->3890)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) ACCM(AVP too short) |... +IP (tos 0x30, ttl 48, id 12336, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->2a8b)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) ACCM(AVP too short) |... +IP (tos 0x30, ttl 48, id 12331, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->2a90)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) ACCM(AVP too short) |... +IP (tos 0x30, ttl 48, id 12336, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->2a8b)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) AVP-#48() |... +IP (tos 0x30, ttl 48, id 12331, offset 0, flags [none], proto UDP (17), length 8752, bad cksum 1f51 (->3890)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) VENDOR0001:ATTR0023(0530) |... +IP (tos 0x30, ttl 48, id 12336, offset 0, flags [none], proto UDP (17), length 12336, bad cksum 1f51 (->2a8b)!) + 127.0.0.229.12416 > 127.236.0.1.1701: l2tp:[TL](560/2056) VENDOR0080:ATTR06a5(19e8) |... +EXIT CODE 00000100 diff --git a/tests/l2tp-avp-overflow.pcap b/tests/l2tp-avp-overflow.pcap new file mode 100644 index 00000000..5a6c4067 Binary files /dev/null and b/tests/l2tp-avp-overflow.pcap differ