Wireshark · Wireshark-dev: [Wireshark-dev] VOIP calls dialog memleak and tap reset callback question

[Peter Wu <peter@xxxxxxxxxxxxx>]

Hi!

The VOIP calls dialog (ui/voip_calls.c) currently leaks memory:

    /* free the strinfo data items first */
    list = g_list_first(tapinfo->rtp_stream_list);
    while(list)
    {
        strinfo = (rtp_stream_info_t *)list->data;
        wmem_free(NULL, strinfo->payload_type_name);
        // g_free(strinfo) is missing here?
        list = g_list_next(list);
    }
    g_list_free(tapinfo->rtp_stream_list);
    tapinfo->rtp_stream_list = NULL;

While that place can be patched, it seems that there is some duplication
between voip_calls_reset_all_taps (above code) and rtp_reset(). Is this
understanding correct:

- voip_calls_reset_all_taps: called when Voip Dialog closes, before
  unregistering the tap listeners.
- rtp_reset: called for the RTP tap, before reading a new capture.

Is it possible to drop the former function? README.tapping says that the
reset callback is normally invoked before reading a new capture, but
can't we just invoke it also before unregistering? Any issues other than
the one mentioned below?


A possible concern is use-after-free though (freeing the structures
while a dialog is still open). For instance, a crash can be triggered:

 1. Open capture file and VoIP Calls dialog.
 2. Close capture file.
 3. Click a stream.
 4. Crash in in VoipCallsDialog::on_callTreeWidget_itemActivated via
    ui/qt/voip_calls_dialog.cpp:617 (v2.3.0rc0-1586-g9887cd7).
-- 
Kind regards,
Peter Wu
https://lekensteyn.nl