Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 9612] Dissector of AMQP 1.0

[bugzilla-daemon@xxxxxxxxxxxxx]

Comment # 19 on bug 9612 from Evan Huus

(In reply to comment #18)
> (In reply to comment #17)
> > Created attachment 12490 [details]
> > Fuzz file triggering valgrind
> > 
> > The attached capture file produces a few minor errors when run under
> > valgrind (with ./tools/valgrind-wireshark.sh).
> > 
> > I took a quick look but it wasn't immediately obvious what the problem was.
> > Some stack value somewhere isn't getting initialized.
> 
> I suspect valgrind is wrong here. Trying to locate the potential issue,
> frame 19 caused it, and "offset" variable in get_amqp_1_0_type_formatter
> method with BT (code lines dont 100% match):
> 
> ==6003==    at 0x638A10E: get_amqp_1_0_value_formatter (packet-amqp.c:6722)
> ==6003==    by 0x638A424: get_amqp_1_0_type_value_formatter
> (packet-amqp.c:10421)
> ==6003==    by 0x638ADC8: dissect_amqp_1_0_frame (packet-amqp.c:6935)
> ==6003==    by 0x6978C45: tcp_dissect_pdus (packet-tcp.c:2291)
> ==6003==    by 0x638D159: dissect_amqp (packet-amqp.c:2796)
> ==6003==    by 0x6307613: call_dissector_through_handle (packet.c:582)
> ==6003==    by 0x6307EF4: call_dissector_work (packet.c:669)
> ==6003==    by 0x630855B: dissector_try_uint_new (packet.c:1100)
> ==6003==    by 0x6978F8D: decode_tcp_ports (packet-tcp.c:3921)
> ==6003==    by 0x69792FE: process_tcp_payload (packet-tcp.c:3980)
> ==6003==    by 0x69798F6: dissect_tcp_payload (packet-tcp.c:1805)
> ==6003==    by 0x697B58E: dissect_tcp (packet-tcp.c:4831)
> ==6003==  Uninitialised value was created by a stack allocation
> ==6003==    at 0x6389900: get_amqp_1_0_value_formatter (packet-amqp.c:10193)
> 
> Adding some printf("%d\n", offset) commands lead to observation that:
> - printf at beginning of get_amqp_1_0_type_formatter prints the valgrind
> warning
> - printf in get_amqp_1_0_type_value_formatter just before calling
> get_amqp_1_0_type_formatter does _not_ print the warning
> - neither so printing offset, type_length_size and bound just before
> AMQP_INCREMENT in the same method
> 
> So, valgrind complains of unitialized "offset" variable, that is apparently
> set properly just before calling the method.
> 
> I run valgrind against several other AMQP 1.0 tcpdumps I have, and no issue
> found.
> 
> I noticed valgrind detected some (in)directly loss bytes (i.e. a sort of
> memory leak), but none of them is AMQP dissector relevant.

You are correct, the memory leaks are a known issue, not related to this.

I'm not entirely sure if this is a result of valgrind getting confused by
optimized code, or actually a compiler bug, but after your investigation I
tried disabling optimization and got much more useful output from valgrind.
Specifically: in dissect_amqp_1_0_map(), the stack variable
decoded_element_size is not initialized if the call to decode_fixed_type()
returns false. This appears to happen in frame 21 of the capture.

Not sure if the correct thing is to zero it to start, or if we should be
checking the return value of decode_fixed_type() and doing something else in
there.

---

      You are receiving this mail because:

• You are watching all bug changes.