Jeff Morriss
changed
bug 9983
| What |
Removed |
Added |
| Status |
CONFIRMED
|
RESOLVED
|
| CC |
|
[email protected]
|
| 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.
