[Asterisk-code-review] AST-2018-001: rtp / channel: Don't allow an unnegotiated for... (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/8313 )

Change subject: AST-2018-001: rtp / channel: Don't allow an unnegotiated format to be passed up.
......................................................................

AST-2018-001: rtp / channel: Don't allow an unnegotiated format to be passed up.

When an RTP packet is received by an RTP engine it has to map the
payload into the Asterisk format. The code was incorrectly checking
our own static list for ALL payloads if it couldn't find a negotiated one.
This included dynamic payloads. If the payload mapped to a format
of a different type (for example receiving a video packet on an audio
RTP instance) then the core stream code could cause a crash if a legacy
channel driver was in use as no stream would be present.

To provide further protection the core stream code will no longer assume
that a video or audio frame will always have a stream for legacy channel
drivers. If no stream is present the frame is dropped.

ASTERISK-27488

Change-Id: I022556f524ad8379ee73f14037040af17ea3316a
---
M main/channel.c
M main/rtp_engine.c
2 files changed, 18 insertions(+), 3 deletions(-)

Approvals:
  Jenkins2: Verified
  Joshua Colp: Looks good to me, approved; Approved for Submit



diff --git a/main/channel.c b/main/channel.c
index fc89d67..c71d19b 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -3667,7 +3667,17 @@
 				 * originated from and update the frame to include it.
 				 */
 				stream = default_stream = ast_channel_get_default_stream(chan, ast_format_get_type(f->subclass.format));
-				f->stream_num = ast_stream_get_position(stream);
+				/* In order to allow media to be passed up the underlying media type has to have a format negotiated on
+				 * the channel itself. In cases where this hasn't happened the channel driver is incorrectly passing up
+				 * a frame for a format that has not been negotiated. If this occurs just drop the frame as we have no
+				 * stream that it came from.
+				 */
+				if (!stream) {
+					ast_frfree(f);
+					f = &ast_null_frame;
+				} else {
+					f->stream_num = ast_stream_get_position(stream);
+				}
 			}
 		}
 	} else {
@@ -3700,7 +3710,12 @@
 			 */
 			if (f && (f->frametype == AST_FRAME_VOICE || f->frametype == AST_FRAME_VIDEO)) {
 				stream = default_stream = ast_channel_get_default_stream(chan, ast_format_get_type(f->subclass.format));
-				f->stream_num = ast_stream_get_position(stream);
+				if (!stream) {
+					ast_frfree(f);
+					f = &ast_null_frame;
+				} else {
+					f->stream_num = ast_stream_get_position(stream);
+				}
 			}
 		}
 		else
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index f108a70..627605a 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -1216,7 +1216,7 @@
 	}
 	ast_rwlock_unlock(&codecs->codecs_lock);
 
-	if (!type) {
+	if (!type && payload <= AST_RTP_PT_LAST_STATIC) {
 		ast_rwlock_rdlock(&static_RTP_PT_lock);
 		type = ao2_bump(static_RTP_PT[payload]);
 		ast_rwlock_unlock(&static_RTP_PT_lock);

-- 
To view, visit https://gerrit.asterisk.org/8313
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I022556f524ad8379ee73f14037040af17ea3316a
Gerrit-Change-Number: 8313
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180221/c9ebdd5c/attachment-0001.html>