https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5531
--- Comment #16 from beroset@xxxxxxxxxxxxxx 2011-10-25 01:35:18 PDT ---
(In reply to comment #15)
> (In reply to comment #14)
> > Created an attachment (id=7297)
--> (https://bugs.wireshark.org/bugzilla/attachment.cgi?id=7297) [details]
[details]
> > added better error checking via tvb_offset_exists() calls where appropriate
> >
> > As per review suggestions, I've added more checking to make sure that the
> > dissector doesn't run off the end of a tvb by making calls to
> > tvb_offset_exists() where appropriate.
>
> Hmm, I don't know about that. Typically it's better to let the dissector read
> off the end of the TVB and throw an exception (indicating a malformed packet if
> it reads past the reported end of the TVB or "packet truncated during
> capturing" if it reads past the end of what was captured--e.g., if a snaplen
> was used while capturing). Why does that need to be done in this dissector?
It's what I thought comment #8 was asking for. It said this:
> All cases where "length" or "*length" is decremented by a value should have
> checks to make sure it's >= that value - *before* whatever is being dissected
> is dissected.
The code originally looked like this:
password = tvb_get_ephemeral_string(tvb, *offset, 20);
proto_tree_add_item(tree, hf_c1222_security_password, tvb, *offset, 20,
ENC_NA);
*offset += 20;
*length -= 20;
In trying to satisfy this request, I changed it to the code that's in the patch
like this:
if (tvb_offset_exists(tvb, (*offset)+20)) {
password = tvb_get_ephemeral_string(tvb, *offset, 20);
proto_tree_add_item(tree, hf_c1222_security_password, tvb, *offset, 20,
ENC_NA);
*offset += 20;
*length -= 20;
...
} else {
expert_add_info_format(pinfo, tree, PI_MALFORMED, PI_ERROR, "C12.22
SECURITY command truncated");
}
If that's not it, then I don't understand what's being requested. Perhaps
someone can enlighten me on that point.
> On an unrelated note: the asn1/c1222/Makefile.* changes don't reflect the
> current trunk's way of doing things (that changed recently). That needs to be
> changed (by either you or the committer before checkin).
I'll investigate and do that.
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.
