From d9a787e4995b4b5101e410ed1e93367907ab9fe2 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <denis@ovsienko.info>
Date: Sun, 10 Jan 2021 00:53:43 +0000
Subject: [PATCH] EIGRP: Modernize packet parsing style.

Enable ND_LONGJMP_FROM_TCHECK. Report invalid packets as invalid. Remove
two redundant ND_TCHECK_*() instances. When giving up on a packet for
whatever reason, test that the rest of it is within the buffer. Do the
header length check before accessing any header data and refine the TLV
length checks. Update a test.
---
 print-eigrp.c            | 49 ++++++++++++++++++----------------------
 tests/eigrp-tlv-oobr.out |  3 ++-
 2 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/print-eigrp.c b/print-eigrp.c
index 1bcdf179..7e374aa5 100644
--- a/print-eigrp.c
+++ b/print-eigrp.c
@@ -31,6 +31,7 @@
 
 #include <string.h>
 
+#define ND_LONGJMP_FROM_TCHECK
 #include "netdissect.h"
 #include "extract.h"
 #include "addrtoname.h"
@@ -232,16 +233,22 @@ eigrp_print(netdissect_options *ndo, const u_char *pptr, u_int len)
 
     ndo->ndo_protocol = "eigrp";
     tptr=pptr;
+    tlen = len;
     eigrp_com_header = (const struct eigrp_common_header *)pptr;
-    ND_TCHECK_SIZE(eigrp_com_header);
 
     /*
      * Sanity checking of the header.
      */
+    if (len < sizeof(struct eigrp_common_header)) {
+        ND_PRINT("EIGRP %s, length: %u (too short, < %zu)",
+               tok2str(eigrp_opcode_values, "unknown (%u)",GET_U_1(eigrp_com_header->opcode)),
+               len, sizeof(struct eigrp_common_header));
+        goto check_remainder;
+    }
     if (GET_U_1(eigrp_com_header->version) != EIGRP_VERSION) {
         ND_PRINT("EIGRP version %u packet not supported",
                  GET_U_1(eigrp_com_header->version));
-        return;
+        goto check_remainder;
     }
 
     /* in non-verbose mode just lets print the basic Message Type*/
@@ -249,18 +256,11 @@ eigrp_print(netdissect_options *ndo, const u_char *pptr, u_int len)
         ND_PRINT("EIGRP %s, length: %u",
                tok2str(eigrp_opcode_values, "unknown (%u)",GET_U_1(eigrp_com_header->opcode)),
                len);
-        return;
+        goto check_remainder;
     }
 
     /* ok they seem to want to know everything - lets fully decode it */
-
-    if (len < sizeof(struct eigrp_common_header)) {
-        ND_PRINT("EIGRP %s, length: %u (too short, < %zu)",
-               tok2str(eigrp_opcode_values, "unknown (%u)",GET_U_1(eigrp_com_header->opcode)),
-               len, sizeof(struct eigrp_common_header));
-        return;
-    }
-    tlen=len-sizeof(struct eigrp_common_header);
+    tlen -= sizeof(struct eigrp_common_header);
 
     ND_PRINT("\n\tEIGRP v%u, opcode: %s (%u), chksum: 0x%04x, Flags: [%s]"
              "\n\tseq: 0x%08x, ack: 0x%08x, VRID: %u, AS: %u, length: %u",
@@ -280,20 +280,14 @@ eigrp_print(netdissect_options *ndo, const u_char *pptr, u_int len)
     tptr+=sizeof(struct eigrp_common_header);
 
     while(tlen>0) {
-        /* did we capture enough for fully decoding the object header ? */
-        ND_TCHECK_LEN(tptr, sizeof(struct eigrp_tlv_header));
-
+        if (tlen < sizeof(struct eigrp_tlv_header)) {
+            ND_PRINT("\n\t  (only %u bytes of data)", tlen);
+            goto invalid;
+        }
         eigrp_tlv_header = (const struct eigrp_tlv_header *)tptr;
         eigrp_tlv_len=GET_BE_U_2(eigrp_tlv_header->length);
         eigrp_tlv_type=GET_BE_U_2(eigrp_tlv_header->type);
 
-
-        if (eigrp_tlv_len < sizeof(struct eigrp_tlv_header) ||
-            eigrp_tlv_len > tlen) {
-            print_unknown_data(ndo,tptr+sizeof(struct eigrp_tlv_header),"\n\t    ",tlen);
-            return;
-        }
-
         ND_PRINT("\n\t  %s TLV (0x%04x), length: %u",
                tok2str(eigrp_tlv_values,
                        "Unknown",
@@ -301,10 +295,9 @@ eigrp_print(netdissect_options *ndo, const u_char *pptr, u_int len)
                eigrp_tlv_type,
                eigrp_tlv_len);
 
-        if (eigrp_tlv_len < sizeof(struct eigrp_tlv_header)) {
-                ND_PRINT(" (too short, < %zu)",
-                         sizeof(struct eigrp_tlv_header));
-                break;
+        if (eigrp_tlv_len < sizeof(struct eigrp_tlv_header) ||
+            eigrp_tlv_len > tlen) {
+            goto invalid;
         }
         tlv_tptr=tptr+sizeof(struct eigrp_tlv_header);
         tlv_tlen=eigrp_tlv_len-sizeof(struct eigrp_tlv_header);
@@ -527,6 +520,8 @@ eigrp_print(netdissect_options *ndo, const u_char *pptr, u_int len)
         tlen-=eigrp_tlv_len;
     }
     return;
-trunc:
-    nd_print_trunc(ndo);
+invalid:
+    nd_print_invalid(ndo);
+check_remainder:
+    ND_TCHECK_LEN(tptr, tlen);
 }
diff --git a/tests/eigrp-tlv-oobr.out b/tests/eigrp-tlv-oobr.out
index b84a3be7..ad69e476 100644
--- a/tests/eigrp-tlv-oobr.out
+++ b/tests/eigrp-tlv-oobr.out
@@ -4132,4 +4132,5 @@
 	  Software Version TLV (0x0004), length: 4 (too short, < 8)
 	  Software Version TLV (0x0004), length: 4 (too short, < 8)
 	  Software Version TLV (0x0004), length: 4 (too short, < 8)
-	  Software Version TLV (0x0004), length: 4 (too short, < 8) [|eigrp]
+	  Software Version TLV (0x0004), length: 4 (too short, < 8)
+	  (only 1 bytes of data) (invalid)
-- 
2.39.5