From 30ba425e7e95f7b61b3a3e5ff0c46e4be9d3d8d7 Mon Sep 17 00:00:00 2001 From: Hadriel Kaplan Date: Sun, 16 Mar 2014 17:20:07 -0400 Subject: [PATCH] Fix Bug 9885: 'Buildbot crash output: fuzz-2014-03-14-15333.pcap' The Buildbot found a crash which is cause by a bug that has been there all along, but a recent change exposed. This bug is likely in 1.10.6 as well, so I'll backport this if I can reproduce it in 1.10.6. Change-Id: I505bc73cbe6281e6d64f00de441c8e6231b55000 Reviewed-on: https://code.wireshark.org/review/702 Reviewed-by: Hadriel Kaplan Reviewed-by: Evan Huus Reviewed-by: Martin Kaiser Reviewed-by: Michael Mann --- epan/dissectors/packet-rtp.c | 5 +++-- epan/dissectors/packet-sdp.c | 12 ++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/epan/dissectors/packet-rtp.c b/epan/dissectors/packet-rtp.c index e2e05386b9..6300c2271e 100644 --- a/epan/dissectors/packet-rtp.c +++ b/epan/dissectors/packet-rtp.c @@ -972,8 +972,9 @@ srtp_add_address(packet_info *pinfo, address *addr, int port, int other_port, /* * Update the conversation data. */ - /* Free the hash if already exists */ - rtp_free_hash_dyn_payload(p_conv_data->rtp_dyn_payload); + /* Free the hash if a different one already exists */ + if (p_conv_data->rtp_dyn_payload != rtp_dyn_payload) + rtp_free_hash_dyn_payload(p_conv_data->rtp_dyn_payload); g_strlcpy(p_conv_data->method, setup_method, MAX_RTP_SETUP_METHOD_SIZE+1); p_conv_data->frame_number = setup_frame_number; diff --git a/epan/dissectors/packet-sdp.c b/epan/dissectors/packet-sdp.c index d7cbaa62b2..3989fe52e5 100644 --- a/epan/dissectors/packet-sdp.c +++ b/epan/dissectors/packet-sdp.c @@ -1816,6 +1816,18 @@ setup_sdp_transport(tvbuff_t *tvb, packet_info *pinfo, enum sdp_exchange_type ex if (in_media_description) { /* Increase the count of media channels, but don't walk off the end of the arrays. */ + /* XXX: I don't know why this was done here - I'm keeping it here in case + * removing it causes problems, but it's wrong. transport_info->media_count + * is already incremented in the while() loop above. Incrementing it + * again here will cause bugs. The name of this is misleading, because + * 'transport_info->media_count' is actually an index, not count. + * In other words, it's a 0-based number, of the current rtp channel. + * So debug printing shows bogus rtp channels get created and then later + * removed because luckily it knows they were bogus. But it will cause bugs + * because if we're not delaying, then for the SDP_EXCHANGE_ANSWER_ACCEPT + * run through this function, it will add new RTP channels at a +1 index, + * which will likely cause problems. + */ if (transport_info->media_count < (SDP_MAX_RTP_CHANNELS-1)) transport_info->media_count++; if (media_info.media_count < (SDP_MAX_RTP_CHANNELS-1))