[FFmpeg-devel] [PATCH 1/4] avformat/asfdec_o: Don't segfault with lots of attached pics

[Andreas Rheinhardt]

Andreas Rheinhardt:
> The ASF file format has a limit of 127 streams and the "asf_o" demuxer
> (the ASF demuxer from Libav) has an array of pointers for a structure
> called ASFStream that is allocated on demand for every stream. Attached
> pictures are not streams in the sense of the ASF specification, yet the
> demuxer created an ASFStream for them; and in one codepath it also
> forgot to check whether the array of ASFStreams is already full. The
> result is a write beyond the end of the array and a segfault lateron.
> 
> Fixing this is easy: Don't create ASFStreams for attached picture
> streams.
> 
> (Other results of the current state of affairs are unnecessary allocations
> (of ASFStreams structures), the misparsing of valid files (there might not
> be enough ASFStreams left for the valid streams if attached pictures take
> up too many); furthermore, the ASFStreams created for attached pictures all
> have the stream number 0, an invalid stream number (the valid range is
> 1-127). This means that invalid data (packets for a stream with stream
> number 0) won't get rejected lateron.)
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavformat/asfdec_o.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/libavformat/asfdec_o.c b/libavformat/asfdec_o.c
> index b142f83541..0a7e47d8cc 100644
> --- a/libavformat/asfdec_o.c
> +++ b/libavformat/asfdec_o.c
> @@ -357,7 +357,6 @@ static int asf_set_metadata(AVFormatContext *s, const uint8_t *name,
>   * but in reality this is only loosely similar */
>  static int asf_read_picture(AVFormatContext *s, int len)
>  {
> -    ASFContext *asf       = s->priv_data;
>      AVPacket pkt          = { 0 };
>      const CodecMime *mime = ff_id3v2_mime_tags;
>      enum  AVCodecID id    = AV_CODEC_ID_NONE;
> @@ -365,7 +364,6 @@ static int asf_read_picture(AVFormatContext *s, int len)
>      uint8_t  *desc = NULL;
>      AVStream   *st = NULL;
>      int ret, type, picsize, desc_len;
> -    ASFStream *asf_st;
>  
>      /* type + picsize + mime + desc */
>      if (len < 1 + 4 + 2 + 2) {
> @@ -422,22 +420,14 @@ static int asf_read_picture(AVFormatContext *s, int len)
>          ret = AVERROR(ENOMEM);
>          goto fail;
>      }
> -    asf->asf_st[asf->nb_streams] = av_mallocz(sizeof(*asf_st));
> -    asf_st = asf->asf_st[asf->nb_streams];
> -    if (!asf_st) {
> -        ret = AVERROR(ENOMEM);
> -        goto fail;
> -    }
>  
>      st->disposition              |= AV_DISPOSITION_ATTACHED_PIC;
> -    st->codecpar->codec_type      = asf_st->type = AVMEDIA_TYPE_VIDEO;
> +    st->codecpar->codec_type      = AVMEDIA_TYPE_VIDEO;
>      st->codecpar->codec_id        = id;
>      st->attached_pic              = pkt;
> -    st->attached_pic.stream_index = asf_st->index = st->index;
> +    st->attached_pic.stream_index = st->index;
>      st->attached_pic.flags       |= AV_PKT_FLAG_KEY;
>  
> -    asf->nb_streams++;
> -
>      if (*desc) {
>          if (av_dict_set(&st->metadata, "title", desc, AV_DICT_DONT_STRDUP_VAL) < 0)
>              av_log(s, AV_LOG_WARNING, "av_dict_set failed.\n");
> 
Will apply this patchset later today unless there are objections.

- Andreas