From 57ff158eb0b55157bb925104571f09b4da8cdf5e Mon Sep 17 00:00:00 2001 From: Denis Ovsienko Date: Sat, 10 Oct 2020 23:17:39 +0100 Subject: VTP: Modernize packet parsing style. Enable ND_LONGJMP_FROM_TCHECK. Constify an argument. Add a standard "invalid" label and use it in packet validation. Test whether the packet is truncated even if it is not fully printed. Lose a few excess ND_TCHECK*() instances. Replace a hard-coded value with a named constant. Update two tests. --- print-vtp.c | 33 +++++++++++++++------------------ tests/vtp_asan-2.out | 2 +- tests/vtp_asan-3.out | 2 +- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/print-vtp.c b/print-vtp.c index e1b8377a..e0c7ded2 100644 --- a/print-vtp.c +++ b/print-vtp.c @@ -27,6 +27,7 @@ #include "netdissect-stdinc.h" +#define ND_LONGJMP_FROM_TCHECK #include "netdissect.h" #include "addrtoname.h" #include "extract.h" @@ -117,7 +118,7 @@ static const struct tok vtp_stp_type_values[] = { void vtp_print(netdissect_options *ndo, - const u_char *pptr, u_int length) + const u_char *pptr, const u_int length) { u_int type, len, name_len, tlv_len, tlv_value, mgmtd_len; const u_char *tptr; @@ -125,7 +126,7 @@ vtp_print(netdissect_options *ndo, ndo->ndo_protocol = "vtp"; if (length < VTP_HEADER_LEN) - goto trunc; + goto invalid; tptr = pptr; @@ -140,15 +141,15 @@ vtp_print(netdissect_options *ndo, /* In non-verbose mode, just print version and message type */ if (ndo->ndo_vflag < 1) { - return; + goto tcheck_full_packet; } /* verbose mode print all fields */ ND_PRINT("\n\tDomain name: "); mgmtd_len = GET_U_1(tptr + 3); - if (mgmtd_len < 1 || mgmtd_len > 32) { + if (mgmtd_len < 1 || mgmtd_len > VTP_DOMAIN_NAME_LEN) { ND_PRINT(" [invalid MgmtD Len %u]", mgmtd_len); - return; + goto invalid; } nd_printzp(ndo, tptr + 4, mgmtd_len, NULL); ND_PRINT(", %s: %u", @@ -181,18 +182,15 @@ vtp_print(netdissect_options *ndo, * */ - ND_TCHECK_8(tptr); ND_PRINT("\n\t Config Rev %x, Updater %s", GET_BE_U_4(tptr), GET_IPADDR_STRING(tptr+4)); tptr += 8; - ND_TCHECK_LEN(tptr, VTP_UPDATE_TIMESTAMP_LEN); ND_PRINT(", Timestamp 0x%08x 0x%08x 0x%08x", GET_BE_U_4(tptr), GET_BE_U_4(tptr + 4), GET_BE_U_4(tptr + 8)); tptr += VTP_UPDATE_TIMESTAMP_LEN; - ND_TCHECK_LEN(tptr, VTP_MD5_DIGEST_LEN); ND_PRINT(", MD5 digest: %08x%08x%08x%08x", GET_BE_U_4(tptr), GET_BE_U_4(tptr + 4), @@ -251,8 +249,7 @@ vtp_print(netdissect_options *ndo, vtp_vlan = (const struct vtp_vlan_*)tptr; if (len < VTP_VLAN_INFO_FIXED_PART_LEN) - goto trunc; - ND_TCHECK_SIZE(vtp_vlan); + goto invalid; ND_PRINT("\n\tVLAN info status %s, type %s, VLAN-id %u, MTU %u, SAID 0x%08x, Name ", tok2str(vtp_vlan_status,"Unknown",GET_U_1(vtp_vlan->status)), tok2str(vtp_vlan_type_values,"Unknown",GET_U_1(vtp_vlan->type)), @@ -263,8 +260,7 @@ vtp_print(netdissect_options *ndo, tptr += VTP_VLAN_INFO_FIXED_PART_LEN; name_len = GET_U_1(vtp_vlan->name_len); if (len < 4*((name_len + 3)/4)) - goto trunc; - ND_TCHECK_LEN(tptr, name_len); + goto invalid; nd_printzp(ndo, tptr, name_len, NULL); /* @@ -285,8 +281,7 @@ vtp_print(netdissect_options *ndo, * what we do. */ if (len < 2) - goto trunc; - ND_TCHECK_2(tptr); + goto invalid; type = GET_U_1(tptr); tlv_len = GET_U_1(tptr + 1); @@ -296,7 +291,7 @@ vtp_print(netdissect_options *ndo, if (len < tlv_len * 2 + 2) { ND_PRINT(" (TLV goes past the end of the packet)"); - return; + goto invalid; } ND_TCHECK_LEN(tptr, tlv_len * 2 + 2); @@ -306,7 +301,7 @@ vtp_print(netdissect_options *ndo, */ if (tlv_len != 1) { ND_PRINT(" (invalid TLV length %u != 1)", tlv_len); - return; + goto invalid; } else { tlv_value = GET_BE_U_2(tptr + 2); @@ -390,6 +385,8 @@ vtp_print(netdissect_options *ndo, return; - trunc: - nd_print_trunc(ndo); +invalid: + nd_print_invalid(ndo); +tcheck_full_packet: + ND_TCHECK_LEN(pptr, length); } diff --git a/tests/vtp_asan-2.out b/tests/vtp_asan-2.out index 52417801..cb4b051a 100644 --- a/tests/vtp_asan-2.out +++ b/tests/vtp_asan-2.out @@ -1,2 +1,2 @@ 1 11:13:40.2148532227 FRF.16 Frag, seq 193, Flags [Begin, End], UI 08! VTPv69, Message Subset advertisement (0x02), length 262131 - Domain name: , Seq number: 0, Config Rev fb499603 [|vtp] + Domain name: , Seq number: 0, Config Rev fb499603 (invalid) [|vtp] diff --git a/tests/vtp_asan-3.out b/tests/vtp_asan-3.out index 02ded296..5a8cf267 100644 --- a/tests/vtp_asan-3.out +++ b/tests/vtp_asan-3.out @@ -1,2 +1,2 @@ 1 11:13:40.2148532227 FRF.16 Frag, seq 193, Flags [Begin, End], UI 08! VTPv69, Message Subset advertisement (0x02), length 262131 - Domain name: , Seq number: 0, Config Rev 4040404 [|vtp] + Domain name: , Seq number: 0, Config Rev 4040404 (invalid) [|vtp] -- cgit v1.2.1