Wireshark-bugs: [Wireshark-bugs] [Bug 9652] Buildbot crash output: fuzz-2014-01-17-15845.pcap
Comment # 28
on bug 9652
from Guy Harris
(In reply to comment #27)
> The underlying bug is probably that fuzzing 10052-wireshark_vs_omnipeek.jpg
> produced a pcap-ng file.
No, the underlying problem is that
1) fuzz-test.sh runs editcap with "-T ether", which means it'll write out
the file in pcap-ng format regardless of the input format
and
2) neither editcap or the pcap-ng writing code will report an error if the
packet is too big for pcap-ng
and
3) the code to read "this is just a file, not a capture file" file formats,
such as JPEG, no longer arbitrarily breaks the file into packets with sizes <
WTAP_MAX_PACKET_SIZE.
3) was a change I made, which is enabled by not reading into fixed-length
arrays in the seek-read routines, but reading into Buffers instead; a Buffer
can grow as large as needed. We still impose a file size limit for those file
types; as the comment in wiretap/mime_file.c says:
/*
* Impose a not-too-large limit on the maximum file size, to avoid eating
* up 99% of the (address space, swap partition, disk space for swap/page
* files); if we were to return smaller chunks and let the dissector do
* reassembly, it would *still* have to allocate a buffer the size of
* the file, so it's not as if we'd neve try to allocate a buffer the
* size of the file.
*
* For now, go for 16MB.
*/
That change simplifies the code that reads those files (and means that the
seek-read code can share most of its code path with the read code; this will
come in handy if we make changes to both code paths, as I intend to do for bug
8590 (both routines will read records that aren't necessarily packets),
eliminates the need for reassembly in dissectors for those file formats, and,
having done that, fixes some incorrect dissections I've seen (some regression
tests from my changes showed the old version misdissecting some files and the
new version not doing so).
So I don't think undoing 3) is a good idea.
I think fixing 2) is a good idea; writing out files that we will subsequently
refuse to read is a bit rude, so we should report errors when we attempt to do
that.
However, that still leaves 1). I'm not sure why we treat "-T ether" as an
indication that, no matter *what* we've been handed, we should write it out as
a pcap-ng file, and I'm not sure why we use "-T ether" in the first place -
that means that every fuzzed file will be turned into an Ethernet capture file,
which would mean we wouldn't necessarily fuzz-test the code path for any other
link-layer header type in capture files, for example (the only way we'd test it
would be if that packet type were somehow encapsulated in something that
ultimately ran over Ethernet).
Gerald, why does the fuzz-test generator use "-T ether"?
You are receiving this mail because:
- You are watching all bug changes.
