https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6519
Michael Mann <mmann78@xxxxxxxxxxxx> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |mmann78@xxxxxxxxxxxx
--- Comment #12 from Michael Mann <mmann78@xxxxxxxxxxxx> 2011-11-02 13:01:05 PDT ---
(In reply to comment #9)
> Ok, found it. But this took a while to step through.
Did it take using
tools/test-fuzzed-cap.sh /path/to/fuzz-2011-10-30-31495.pcap
to find it? I tried tools/fuzz-test.sh fuzz-2011-10-30-31495.pcap (or at least
some of the buildbot captures) (on Windows) and had no luck duplicating it.
What basically happens is
> the following:
> 1. dissect_cip_data get's called the first time
> 2. (!!) p_add_proto_data get's called, at the beginning of the first run
> through dissect_cip_data, and stores a reference to cia_data
> 3. using a heuristic dissector lookup table 'dissect_cip_generic_service_rsp'
> get's called
> 4. using the case, it call's dissect_cip_multiple_service_packet_rsp
> Here the error occurs, by accessing mr_mult_req_info->requests. The problem
> with that access is, that the method relies on the fact, that p_get_proto_data
> does in fact return a cip_req_info where pData is NULL, which would case a
> se_alloc for mr_mult_req_info->requests.
> But this does not happen, as this specific memory has already been written with
> a call to p_add_proto_data during the first call to dissect_cip_data.
> It seems, that dissect_cip_multiple_service_packet_rsp is not prepared to be
> called during a heuristic call, if the memory has been pre-written with
> 0xbaddcafe.
> So, I can not provide a solution directly here. This part of the code needs a
> deeper background knowledge for CIP, to be handled correctly. In principal the
> initialization for mr_mult_req_info->requests has to be handled more securely,
> as it is done at the moment.
So would it just be prefered to initialize all memory after se_alloc() to 0 so
it doesn't have 0xbaddcafe to deal with? I thought this would be a case of not
checking for NULL or not properly initializing to NULL for "NULL checks" to
catch. 0xbaddcafe points to having != NULL checks (that fail), but not
initializing to NULL in the first place.
I have the deeper background knowledge of CIP and I'm the one that introduced
this mess. I was hoping to help get out of it, but I couldn't duplicate it to
run through a debugger (preferably in Wireshark, not in fuzztesting) to verify
if a memset(mr_mult_req_info->requests, 0...) after se_alloc() would actually
fix it.
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
