[bugzilla-daemon@xxxxxxxxxxxxx]

Jeff Morriss changed bug 9983

What	Removed	Added
Status	CONFIRMED	RESOLVED
CC		jeff.morriss.ws@gmail.com
Resolution	---	DUPLICATE

Comment # 10 on bug 9983 from Jeff Morriss

(In reply to comment #9)
> This seems to be the problematic part of proto_tree_add_bytes_format
> 
>     if (!(PTREE_DATA(tree)->visible)) {                 \
>         if (PTREE_FINFO(tree)) {                \
>             if ((hfinfo->ref_type != HF_REF_TYPE_DIRECT)    \
>                 && (hfinfo->type != FT_PROTOCOL ||      \
>                 PTREE_DATA(tree)->fake_protocols)) {    \
>                 /* just return tree back to the caller */\
>                 return tree;                \
>             }                           \
>         }                               \
>     }
> 
> ref_type is HF_REF_TYPE_NONE, type is FT_BYTES (and fake_protocols 1). So
> proto_tree_add_bytes_format will return without checking lengths or
> anything. I am not familiar in this part, perhaps you have more ideas to fix
> this?
> 
> Should we add a tvb_ensure_bytes_exist(tvb, offset, payload_length) before
> calling proto_tree_add_bytes_format? Similar for the padding?

Yep, nice catch.

This is the same as bug 3290.  So far that's been fixed only for
proto_tree_add_item() (IIRC) on the theory that for other types the dissector
has likely already fetched the item (with tvb_*()) which would throw an
exception if necessary.

Obviously FT_BYTES is a reasonable exception to that rule: in the case of, say,
a uint you probably fetched the value for a reason (i.e., that's why you're not
using proto_tree_add_item()), but you probably have no reason to fetch FT_BYTES
before calling proto_tree_add_bytes_format() (so the API should do like
proto_tree_add_item() and test it for you).

*** This bug has been marked as a duplicate of bug 3290 ***

---

      You are receiving this mail because:

• You are watching all bug changes.