diff options
author | novakji <j.novak@netsystem.cz> | 2016-11-13 20:20:17 +0100 |
---|---|---|
committer | Michael Mann <mmann78@netscape.net> | 2016-11-16 01:22:58 +0000 |
commit | 737dabb0437bd5a0a1f4c07189ca59f1c83afb55 (patch) | |
tree | e07148b82e444d3c35f088dee629fa04999a23e4 /epan/dissectors | |
parent | 1de9f3ccebdef5b2423c70cd565dc9c4ec203177 (diff) |
SDP: Payload type name for dynamic payload is wrong for reverse RTP channels
Bug: 13132
Change-ID: I61a2575f9d8da958ae2fb01c71f3c71c9643ddea
Reviewed-on: https://code.wireshark.org/review/18804
Reviewed-by: Pascal Quantin <pascal.quantin@gmail.com>
Petri-Dish: Pascal Quantin <pascal.quantin@gmail.com>
Reviewed-by: Michael Mann <mmann78@netscape.net>
Tested-by: Michael Mann <mmann78@netscape.net>
Diffstat (limited to 'epan/dissectors')
-rw-r--r-- | epan/dissectors/packet-sdp.c | 52 |
1 files changed, 13 insertions, 39 deletions
diff --git a/epan/dissectors/packet-sdp.c b/epan/dissectors/packet-sdp.c index befdb9fd55..9bd39943ed 100644 --- a/epan/dissectors/packet-sdp.c +++ b/epan/dissectors/packet-sdp.c @@ -259,7 +259,7 @@ typedef struct { const guint8 *media_type[SDP_MAX_RTP_CHANNELS]; const guint8 *media_port[SDP_MAX_RTP_CHANNELS]; const guint8 *media_proto[SDP_MAX_RTP_CHANNELS]; - guint8 media_count; + gint8 media_count; /* MSRP transport info (as set while parsing path attribute) */ gboolean msrp_transport_address_set; @@ -330,7 +330,7 @@ static void sdp_dump_transport_info(const transport_info_t* info) { } DENDENT(); count = (int)info->media_count; - DPRINT2(("media_count=%d", count)); + DPRINT2(("media_count=%d", count+1)); DPRINT2(("rtp channels:")); DINDENT(); for (i=0; i <= count; i++) { @@ -370,9 +370,9 @@ static void sdp_dump_disposable_media_info(const disposable_media_info_t* info) DPRINT2(("connection_type=%s", info->connection_type ? (const char *)info->connection_type : "NULL")); count = (int)info->media_count; - DPRINT2(("media_count=%d",count)); + DPRINT2(("media_count=%d",count+1)); DINDENT(); - for (i=0; i < count; i++) { + for (i=0; i <= count; i++) { DPRINT2(("media #%d:",i)); DINDENT(); DPRINT2(("media_type=%s", info->media_type[i] ? (const char *)info->media_type[i] : "NULL")); @@ -1796,7 +1796,7 @@ convert_disposable_media(transport_info_t* transport_info, disposable_media_info gint8 n, transport_index; guint proto_bitmask; - for (n = 0; (n < media_info->media_count) && (n+start_transport_info_count < SDP_MAX_RTP_CHANNELS); n++) + for (n = 0; (n <= media_info->media_count) && (n+start_transport_info_count < SDP_MAX_RTP_CHANNELS); n++) { transport_index = n+start_transport_info_count; if (media_info->media_port[n] != NULL) { @@ -1940,6 +1940,7 @@ setup_sdp_transport(tvbuff_t *tvb, packet_info *pinfo, enum sdp_exchange_type ex } memset(&media_info, 0, sizeof(media_info)); + media_info.media_count=-1; if (request_frame != 0) transport_info = (transport_info_t*)wmem_tree_lookup32( sdp_transport_reqs, request_frame ); @@ -1976,8 +1977,8 @@ setup_sdp_transport(tvbuff_t *tvb, packet_info *pinfo, enum sdp_exchange_type ex (transport_info->sdp_status == SDP_EXCHANGE_ANSWER_ACCEPT)) return; - if (transport_info->media_count > 0) - start_transport_info_count = transport_info->media_count; + if (transport_info->media_count >= 0) + start_transport_info_count = transport_info->media_count+1; DPRINT(("start_transport_info_count=%d", start_transport_info_count)); @@ -2026,10 +2027,10 @@ setup_sdp_transport(tvbuff_t *tvb, packet_info *pinfo, enum sdp_exchange_type ex hf = hf_media; /* Increase the count of media channels, but don't walk off the end of the arrays. */ - if (((transport_info->media_count < 0) && (in_media_description == FALSE)) || (transport_info->media_count < (SDP_MAX_RTP_CHANNELS-1))) + if (transport_info->media_count < (SDP_MAX_RTP_CHANNELS-1)) transport_info->media_count++; - if (in_media_description && (media_info.media_count < (SDP_MAX_RTP_CHANNELS-1))) + if (media_info.media_count < (SDP_MAX_RTP_CHANNELS-1)) media_info.media_count++; in_media_description = TRUE; @@ -2062,26 +2063,6 @@ setup_sdp_transport(tvbuff_t *tvb, packet_info *pinfo, enum sdp_exchange_type ex offset = next_offset; } - 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)) - media_info.media_count++; - } - #ifdef DEBUG_CONVERSATION sdp_dump_disposable_media_info(&media_info); #endif @@ -2326,6 +2307,7 @@ dissect_sdp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_) } memset(&media_info, 0, sizeof(media_info)); + media_info.media_count=-1; /* * As RFC 2327 says, "SDP is purely a format for session @@ -2427,7 +2409,7 @@ dissect_sdp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_) if (local_transport_info.media_count < (SDP_MAX_RTP_CHANNELS-1)) local_transport_info.media_count++; - if (in_media_description && (media_info.media_count < (SDP_MAX_RTP_CHANNELS-1))) + if (media_info.media_count < (SDP_MAX_RTP_CHANNELS-1)) media_info.media_count++; in_media_description = TRUE; @@ -2466,14 +2448,6 @@ dissect_sdp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_) offset = next_offset; } - if (in_media_description) { - /* Increase the count of media channels, but don't walk off the end of the arrays. */ - if (local_transport_info.media_count < (SDP_MAX_RTP_CHANNELS-1)) - local_transport_info.media_count++; - if (media_info.media_count < (SDP_MAX_RTP_CHANNELS-1)) - media_info.media_count++; - } - /* Take all of the collected strings and convert them into something permanent * for the life of the capture */ @@ -2493,7 +2467,7 @@ dissect_sdp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_) } #endif - for (n = 0; n < local_transport_info.media_count; n++) + for (n = 0; n <= local_transport_info.media_count; n++) { /* Add (s)rtp and (s)rtcp conversation, if available (overrides t38 if conversation already set) */ /* XXX - This is a placeholder for higher layer protocols that haven't implemented the proper |