rtsp: Don't expose the MS-RTSP RTX data stream to the caller

Message ID 1333749604-32726-1-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö April 6, 2012, 10 p.m.
This avoids exposing a dummy AVStream which won't get any data
and which will make avformat_find_stream_info wait for info about
this stream.
---
 libavformat/rtpdec.c     |    2 +-
 libavformat/rtpdec_asf.c |    2 ++
 libavformat/rtsp.c       |   22 +++++++++++++++-------
 3 files changed, 18 insertions(+), 8 deletions(-)

Comments

Ronald Bultje April 6, 2012, 10:17 p.m. | #1
Hi,

On Fri, Apr 6, 2012 at 3:00 PM, Martin Storsjö <martin@martin.st> wrote:
> This avoids exposing a dummy AVStream which won't get any data
> and which will make avformat_find_stream_info wait for info about
> this stream.
> ---
>  libavformat/rtpdec.c     |    2 +-
>  libavformat/rtpdec_asf.c |    2 ++
>  libavformat/rtsp.c       |   22 +++++++++++++++-------
>  3 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
> index 61653f7..41e6eb4 100644
> --- a/libavformat/rtpdec.c
> +++ b/libavformat/rtpdec.c
> @@ -385,7 +385,7 @@ RTPDemuxContext *ff_rtp_parse_open(AVFormatContext *s1, AVStream *st, URLContext
>             av_free(s);
>             return NULL;
>         }
> -    } else {
> +    } else if (st) {
>         switch(st->codec->codec_id) {
>         case CODEC_ID_MPEG1VIDEO:
>         case CODEC_ID_MPEG2VIDEO:
> diff --git a/libavformat/rtpdec_asf.c b/libavformat/rtpdec_asf.c
> index c1690ef..bbb7609 100644
> --- a/libavformat/rtpdec_asf.c
> +++ b/libavformat/rtpdec_asf.c
> @@ -130,6 +130,8 @@ int ff_wms_parse_sdp_a_line(AVFormatContext *s, const char *p)
>  static int asfrtp_parse_sdp_line(AVFormatContext *s, int stream_index,
>                                  PayloadContext *asf, const char *line)
>  {
> +    if (stream_index < 0)
> +        return 0;

A malicious server could send a corrupt stream identifying as Xiph, or
AMR, or so, and these also have parse_sdp_line functions. Do they need
similar checks?

../libavformat/rtpdec_amr.c:    .parse_sdp_a_line = amr_parse_sdp_line,
../libavformat/rtpdec_amr.c:    .parse_sdp_a_line = amr_parse_sdp_line,
../libavformat/rtpdec_asf.c:int
ff_wms_parse_sdp_a_line(AVFormatContext *s, const char *p)
../libavformat/rtpdec_asf.c:    .parse_sdp_a_line = asfrtp_parse_sdp_line, \
../libavformat/rtpdec_h264.c:    .parse_sdp_a_line = parse_h264_sdp_line,
../libavformat/rtpdec_latm.c:    .parse_sdp_a_line   = latm_parse_sdp_line,
../libavformat/rtpdec_mpeg4.c:    .parse_sdp_a_line   = parse_sdp_line,
../libavformat/rtpdec_mpeg4.c:    .parse_sdp_a_line   = parse_sdp_line,
../libavformat/rtpdec_xiph.c:    .parse_sdp_a_line = xiph_parse_sdp_line,
../libavformat/rtpdec_xiph.c:    .parse_sdp_a_line = xiph_parse_sdp_line,

Ronald
Martin Storsjö April 6, 2012, 10:30 p.m. | #2
On Fri, 6 Apr 2012, Ronald S. Bultje wrote:

> Hi,
>
> On Fri, Apr 6, 2012 at 3:00 PM, Martin Storsjö <martin@martin.st> wrote:
>> This avoids exposing a dummy AVStream which won't get any data
>> and which will make avformat_find_stream_info wait for info about
>> this stream.
>> ---
>>  libavformat/rtpdec.c     |    2 +-
>>  libavformat/rtpdec_asf.c |    2 ++
>>  libavformat/rtsp.c       |   22 +++++++++++++++-------
>>  3 files changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
>> index 61653f7..41e6eb4 100644
>> --- a/libavformat/rtpdec.c
>> +++ b/libavformat/rtpdec.c
>> @@ -385,7 +385,7 @@ RTPDemuxContext *ff_rtp_parse_open(AVFormatContext *s1, AVStream *st, URLContext
>>             av_free(s);
>>             return NULL;
>>         }
>> -    } else {
>> +    } else if (st) {
>>         switch(st->codec->codec_id) {
>>         case CODEC_ID_MPEG1VIDEO:
>>         case CODEC_ID_MPEG2VIDEO:
>> diff --git a/libavformat/rtpdec_asf.c b/libavformat/rtpdec_asf.c
>> index c1690ef..bbb7609 100644
>> --- a/libavformat/rtpdec_asf.c
>> +++ b/libavformat/rtpdec_asf.c
>> @@ -130,6 +130,8 @@ int ff_wms_parse_sdp_a_line(AVFormatContext *s, const char *p)
>>  static int asfrtp_parse_sdp_line(AVFormatContext *s, int stream_index,
>>                                  PayloadContext *asf, const char *line)
>>  {
>> +    if (stream_index < 0)
>> +        return 0;
>
> A malicious server could send a corrupt stream identifying as Xiph, or
> AMR, or so, and these also have parse_sdp_line functions. Do they need
> similar checks?

I guess they do, I'll send a new patch later.

// Martin

Patch

diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index 61653f7..41e6eb4 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -385,7 +385,7 @@  RTPDemuxContext *ff_rtp_parse_open(AVFormatContext *s1, AVStream *st, URLContext
             av_free(s);
             return NULL;
         }
-    } else {
+    } else if (st) {
         switch(st->codec->codec_id) {
         case CODEC_ID_MPEG1VIDEO:
         case CODEC_ID_MPEG2VIDEO:
diff --git a/libavformat/rtpdec_asf.c b/libavformat/rtpdec_asf.c
index c1690ef..bbb7609 100644
--- a/libavformat/rtpdec_asf.c
+++ b/libavformat/rtpdec_asf.c
@@ -130,6 +130,8 @@  int ff_wms_parse_sdp_a_line(AVFormatContext *s, const char *p)
 static int asfrtp_parse_sdp_line(AVFormatContext *s, int stream_index,
                                  PayloadContext *asf, const char *line)
 {
+    if (stream_index < 0)
+        return 0;
     if (av_strstart(line, "stream:", &line)) {
         RTSPState *rt = s->priv_data;
 
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index c94ef02..902a28e 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -374,6 +374,10 @@  static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
 
         if (!strcmp(ff_rtp_enc_name(rtsp_st->sdp_payload_type), "MP2T")) {
             /* no corresponding stream */
+        } else if (rt->server_type == RTSP_SERVER_WMS &&
+                   codec_type == AVMEDIA_TYPE_DATA) {
+            /* RTX stream, a stream that carries all the other actual
+             * audio/video streams. Don't expose this to the callers. */
         } else {
             st = avformat_new_stream(s, NULL);
             if (!st)
@@ -430,9 +434,11 @@  static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
             /* NOTE: rtpmap is only supported AFTER the 'm=' tag */
             get_word(buf1, sizeof(buf1), &p);
             payload_type = atoi(buf1);
-            st = s->streams[s->nb_streams - 1];
             rtsp_st = rt->rtsp_streams[rt->nb_rtsp_streams - 1];
-            sdp_parse_rtpmap(s, st, rtsp_st, payload_type, p);
+            if (rtsp_st->stream_index >= 0) {
+                st = s->streams[rtsp_st->stream_index];
+                sdp_parse_rtpmap(s, st, rtsp_st, payload_type, p);
+            }
         } else if (av_strstart(p, "fmtp:", &p) ||
                    av_strstart(p, "framesize:", &p)) {
             /* NOTE: fmtp is only supported AFTER the 'a=rtpmap:xxx' tag */
@@ -467,14 +473,15 @@  static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
             if (rt->server_type == RTSP_SERVER_WMS)
                 ff_wms_parse_sdp_a_line(s, p);
             if (s->nb_streams > 0) {
+                rtsp_st = rt->rtsp_streams[rt->nb_rtsp_streams - 1];
+
                 if (rt->server_type == RTSP_SERVER_REAL)
-                    ff_real_parse_sdp_a_line(s, s->nb_streams - 1, p);
+                    ff_real_parse_sdp_a_line(s, rtsp_st->stream_index, p);
 
-                rtsp_st = rt->rtsp_streams[rt->nb_rtsp_streams - 1];
                 if (rtsp_st->dynamic_handler &&
                     rtsp_st->dynamic_handler->parse_sdp_a_line)
                     rtsp_st->dynamic_handler->parse_sdp_a_line(s,
-                        s->nb_streams - 1,
+                        rtsp_st->stream_index,
                         rtsp_st->dynamic_protocol_context, buf);
             }
         }
@@ -1250,8 +1257,9 @@  int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port,
              * UDP. When trying to set it up for TCP streams, the server
              * will return an error. Therefore, we skip those streams. */
             if (rt->server_type == RTSP_SERVER_WMS &&
-                s->streams[rtsp_st->stream_index]->codec->codec_type ==
-                    AVMEDIA_TYPE_DATA)
+                (rtsp_st->stream_index < 0 ||
+                 s->streams[rtsp_st->stream_index]->codec->codec_type ==
+                    AVMEDIA_TYPE_DATA))
                 continue;
             snprintf(transport, sizeof(transport) - 1,
                      "%s/TCP;", trans_pref);