Wireshark · Wireshark-dev: Re: [Wireshark-dev] [PATCH] babel: fix infinite loop with TLVs of length 0.

[Pascal Quantin <pascal@xxxxxxxxxxxxx>]

Hi Juliusz,

do you intend to push the patch set to our Gerrit as explained in https://www.wireshark.org/docs/wsdg_html_chunked/ChSrcContribute.html ?

Our workflow is not using mail based patches.

Presumably we should also remove the test on sublen == 0 I added to fix the infinite loop (as you stated this was valid). Moreover in case of MESSAGE_SUB_PAD1, is really beg variable only incremented by 1 and not 2? (you fetched a sublen field also and highlighted 2 bytes for the hf_babel_subtlv field).

Best regards,

Pascal.

Le mar. 5 nov. 2019 à 15:25, Juliusz Chroboczek <jch@xxxxxxx> a écrit :

From: Sawssen Hadded <saw.hadded@xxxxxxxxx>

Sublen was misinterpreted -- it's the length of the value, not of the TLV.

Fixes #15856.

Change-Id: I8090425abd83654304a3539ac2ea6bc3f107ef5c
---
 epan/dissectors/packet-babel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/epan/dissectors/packet-babel.c b/epan/dissectors/packet-babel.c
index 141a70ed90..361d4584f5 100644
--- a/epan/dissectors/packet-babel.c
+++ b/epan/dissectors/packet-babel.c
@@ -264,7 +264,7 @@ dissect_babel_subtlvs(tvbuff_t * tvb, packet_info *pinfo, guint8 type,

         sub_item =
           proto_tree_add_uint_format(message_tree, hf_babel_subtlv,
-                                     tvb, beg, sublen, subtype,
+                                     tvb, beg, sublen+2, subtype,
                                      "Sub TLV %s (%u)",
                                      val_to_str_const(subtype, subtlvs, "unknown"),
                                      subtype);
@@ -280,7 +280,7 @@ dissect_babel_subtlvs(tvbuff_t * tvb, packet_info *pinfo, guint8 type,
         }

         if(subtype == MESSAGE_SUB_PAD1){
-            beg += sublen;
+            beg += 1;
             continue;
         }
         if ((MANDATORY_FLAG & subtype) != 0) {
--
2.23.0