Wireshark-bugs: [Wireshark-bugs] [Bug 9323] Buildbot crash output: fuzz-2013-10-25-12569.pcap
Comment # 18
on bug 9323
from Evan Huus
OK, new plan.
(In reply to comment #7)
> FT_STRING ("not necessarily NULL terminated" according to README.dissector)
> but proto_tree_add_string() + proto_tree_set_string() are assuming that it IS
> NULL terminated?
proto_tree_add_string and friends can be used for either FT_STRING or
FT_STRINGZ. The different types are for different representations in the
packet, but don't necessarily have anything to do with the parameter passed
into proto_tree_add_string (or else the user could just use
proto_tree_add_item).
--
So, what if we undid r52905 and r52908 (except the change to ftype-string.c, we
need that). Then create functions proto_tree_add_string_count etc that take an
additional length parameter of the actual size of the string (because it does
not necessarily have any relation to the number of bytes we highlight in the
packet). Then switch the problem lines in the HTTP dissector to the _count
version. Presumably also note in the doxygen that proto_tree_add_string et al
require a null-terminated string, proto_tree_add_string_count et al do not.
I *think* this solves all the relevant problems (including Jakub's point about
encoding), though it may mean a few rounds of manually appending _count to
functions as valgrind finds more cases like this one. Sadly it is not as easily
backportable, but this error has not appeared to cause any actual problems so
far due to wiretap overallocating, so we're probably OK without.
Thoughts?
You are receiving this mail because:
- You are watching all bug changes.
