Evan Huus
changed
bug 8961
Comment # 7
on bug 8961
from Evan Huus
Hi Linas, sorry for all the back-and-forth. I need to be a bit more proactive
in describing the correct solutions, not just the problems, as I forget
sometimes that most people aren't as familiar with the API. My apologies.
There is no need to check for errors in most fields (although it makes the
dissection more complete, it is a big job). It is, however, required to check
for errors in values which affect dissection, as otherwise it is possible to
create packets which can cause crashes or infinite loops etc.
As far as I can tell the current version of the patch has no more infinite or
too-long loops, which is a good thing. However, throwing an exception is the
wrong way to signal a corrupt packet, and trying to do further work immediately
after a THROW results in unreachable code.
The correct way to signal a corrupt packet is through the expert API, however I
believe that proto_item_set_expert_flags is now deprecated. I have CCed Michael
to double-check this, but I *think* that all expert API calls not taking an
expert_field struct are being removed. Additionally, expert calls (like column
calls) must occur regardless of whether or not the tree parameter is NULL.
Here are a few tips that might help:
1. "if (tree)" checks are generally kept to a minimum, and only where it is
obvious they do not enclose any expert, column, or other code. All API calls
are safe even when tree is NULL, so it is simplest to remove the checks
completely. They are an optimization only.
2. When a value (like pkt_size) is fetched from the packet, it should
immediately be checked for incorrect values (in this case probably against
tvb_reported_length_remaining). If the value is incorrect, an expert info
should be added for that field and the fetched value should be set to some safe
value (again, probably tvb_reported_length_remaining). This should greatly
simplify your loop, because it can then assume that the input values are
correct.
If you have any further questions, feel free to ask.
Evan
You are receiving this mail because:
- You are watching all bug changes.