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

[Pascal Quantin <pascal@xxxxxxxxxxxxx>]

Hi Juliusz and Sawssen,

Le mar. 5 nov. 2019 à 15:39, Pascal Quantin <pascal@xxxxxxxxxxxxx> a écrit :

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

[Resent with fixed CC.]

> 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.

We tried, but failed.  "git review" returned a 500 error.  Slightly later,
authentication failed (I was using the password autogenerated in the
Gerrit user interface).

> Presumably we should also remove the test on sublen == 0 I added to fix the
> infinite loop (as you stated this was valid).

Right, we missed that.

> Moreover in case of MESSAGE_SUB_PAD1, is really beg variable only
> incremented by 1 and not 2?

That's right, PAD1 is just a lone byte of value 0, not a real TLV.

  https://tools.ietf.org/html/draft-ietf-babel-rfc6126bis-15#section-4.7.1

Then the changed in proto_tree_add_uint_format() must be adapted accordingly to differentiate both cases.

> (you fetched a sublen field also and highlighted 2 bytes for the
> hf_babel_subtlv field).

Right again.

I'll fix the code, see again if I can get Gerrit to work, then submit
a new version.

If you face any issue, send me the patch as an email attachment and I will submit it for you.

As I did not get any feedback, I pushed my own version of the patch with a few more fixes. See https://code.wireshark.org/review/#/c/35086/ for details.

Best regards,

Pascal.