From 0f27a83c5692b2afebe6e6934c1051f76aa2ecf9 Mon Sep 17 00:00:00 2001

From: Jason Cohen <kryojenik2@gmail.com>

Date: Wed, 31 Aug 2022 11:10:17 -0500

Subject: [PATCH] f5ethtrailer: Improve "old-style" heuristic

Remove a chance for an infinate loop in the disection heuristic.

---

 epan/dissectors/packet-f5ethtrailer.c | 108 +++++++++++++-------------

 1 file changed, 56 insertions(+), 52 deletions(-)

diff --git a/epan/dissectors/packet-f5ethtrailer.c b/epan/dissectors/packet-f5ethtrailer.c

index b2ba8f899d..915348ea83 100644

--- a/epan/dissectors/packet-f5ethtrailer.c

+++ b/epan/dissectors/packet-f5ethtrailer.c

@@ -2751,69 +2751,73 @@ dissect_dpt_trailer(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *d

 static gint

 dissect_old_trailer(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data)

 {

-    proto_tree *type_tree   = NULL;

-    proto_item *ti          = NULL;

     guint offset            = 0;

-    guint processed         = 0;

-    f5eth_tap_data_t *tdata = (f5eth_tap_data_t *)data;

-    guint8 type;

-    guint8 len;

-    guint8 ver;

     /* While we still have data in the trailer.  For old format trailers, this needs

      * type, length, version (3 bytes) and for new format trailers, the magic header (4 bytes).

      * All old format trailers are at least 4 bytes long, so just check for length of magic.

      */

-    while (tvb_reported_length_remaining(tvb, offset)) {

-        type = tvb_get_guint8(tvb, offset);

-        len = tvb_get_guint8(tvb, offset + F5_OFF_LENGTH) + F5_OFF_VERSION;

-        ver = tvb_get_guint8(tvb, offset + F5_OFF_VERSION);

-

-        if (len <= tvb_reported_length_remaining(tvb, offset) && type >= F5TYPE_LOW

-            && type <= F5TYPE_HIGH && len >= F5_MIN_SANE && len <= F5_MAX_SANE

-            && ver <= F5TRAILER_VER_MAX) {

-            /* Parse out the specified trailer. */

-            switch (type) {

-            case F5TYPE_LOW:

-                ti        = proto_tree_add_item(tree, hf_low_id, tvb, offset, len, ENC_NA);

-                type_tree = proto_item_add_subtree(ti, ett_f5ethtrailer_low);

-

-                processed = dissect_low_trailer(tvb, pinfo, type_tree, offset, len, ver, tdata);

-                if (processed > 0) {

-                    tdata->trailer_len += processed;

-                    tdata->noise_low = 1;

-                }

-                break;

-            case F5TYPE_MED:

-                ti        = proto_tree_add_item(tree, hf_med_id, tvb, offset, len, ENC_NA);

-                type_tree = proto_item_add_subtree(ti, ett_f5ethtrailer_med);

-

-                processed = dissect_med_trailer(tvb, pinfo, type_tree, offset, len, ver, tdata);

-                if (processed > 0) {

-                    tdata->trailer_len += processed;

-                    tdata->noise_med = 1;

-                }

-                break;

-            case F5TYPE_HIGH:

-                ti        = proto_tree_add_item(tree, hf_high_id, tvb, offset, len, ENC_NA);

-                type_tree = proto_item_add_subtree(ti, ett_f5ethtrailer_high);

-

-                processed =

-                    dissect_high_trailer(tvb, pinfo, type_tree, offset, len, ver, tdata);

-                if (processed > 0) {

-                    tdata->trailer_len += processed;

-                    tdata->noise_high = 1;

-                }

-                break;

+    while (tvb_reported_length_remaining(tvb, offset) >= F5_MIN_SANE) {

+        /* length field does not include the type and length bytes.  Add them back in */

+        guint8 len = tvb_get_guint8(tvb, offset + F5_OFF_LENGTH) + F5_OFF_VERSION;

+        if (len > tvb_reported_length_remaining(tvb, offset)

+            || len < F5_MIN_SANE || len > F5_MAX_SANE) {

+            /* Invalid length - either a malformed trailer, corrupt packet, or not f5ethtrailer */

+            return offset;

+        }

+        guint8 type = tvb_get_guint8(tvb, offset);

+        guint8 ver = tvb_get_guint8(tvb, offset + F5_OFF_VERSION);

+

+        /* Parse out the specified trailer. */

+        proto_tree *type_tree   = NULL;

+        proto_item *ti          = NULL;

+        f5eth_tap_data_t *tdata = (f5eth_tap_data_t *)data;

+        guint processed = 0;

+

+        switch (type) {

+        case F5TYPE_LOW:

+            ti        = proto_tree_add_item(tree, hf_low_id, tvb, offset, len, ENC_NA);

+            type_tree = proto_item_add_subtree(ti, ett_f5ethtrailer_low);

+

+            processed = dissect_low_trailer(tvb, pinfo, type_tree, offset, len, ver, tdata);

+            if (processed > 0) {

+                tdata->trailer_len += processed;

+                tdata->noise_low = 1;

             }

-            if (processed == 0) {

-                proto_item_set_len(ti, 1);

-                return offset;

+            break;

+        case F5TYPE_MED:

+            ti        = proto_tree_add_item(tree, hf_med_id, tvb, offset, len, ENC_NA);

+            type_tree = proto_item_add_subtree(ti, ett_f5ethtrailer_med);

+

+            processed = dissect_med_trailer(tvb, pinfo, type_tree, offset, len, ver, tdata);

+            if (processed > 0) {

+                tdata->trailer_len += processed;

+                tdata->noise_med = 1;

+            }

+            break;

+        case F5TYPE_HIGH:

+            ti        = proto_tree_add_item(tree, hf_high_id, tvb, offset, len, ENC_NA);

+            type_tree = proto_item_add_subtree(ti, ett_f5ethtrailer_high);

+

+            processed =

+                dissect_high_trailer(tvb, pinfo, type_tree, offset, len, ver, tdata);

+            if (processed > 0) {

+                tdata->trailer_len += processed;

+                tdata->noise_high = 1;

             }

+            break;

+        default:

+            /* Unknown type - malformed trailer, corrupt packet, or not f5ethtrailer - bali out*/

+            return offset;

+        }

+        if (processed == 0) {

+            /* couldn't process trailer - bali out */

+            proto_item_set_len(ti, 1);

+            return offset;

         }

         offset += processed;

     }

-return offset;

+    return offset;

 } /* dissect_old_trailer() */

 /*---------------------------------------------------------------------------*/

-- 

GitLab