[FFmpeg-devel] [PATCH v2] avformat/imfdec: check track valid before use it

[Andreas Rheinhardt]

Steven Liu:
> fix CID: 1512414
> And return AVERROR_INVALIDDATA when get_next_track_with_minimum_timestamp
> incorrect in imf_read_packet;
> 
> Signed-off-by: Steven Liu <lq at chinaffmpeg.org>
> ---
>  libavformat/imfdec.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
> index 5bbe7a53f8..08f342bc1a 100644
> --- a/libavformat/imfdec.c
> +++ b/libavformat/imfdec.c
> @@ -697,8 +697,9 @@ static IMFVirtualTrackPlaybackCtx *get_next_track_with_minimum_timestamp(AVForma
>          }
>      }
>  
> -    av_log(s, AV_LOG_DEBUG, "Found next track to read: %d (timestamp: %lf / %lf)\n",
> -           track->index, av_q2d(track->current_timestamp), av_q2d(minimum_timestamp));
> +    if (track)
> +        av_log(s, AV_LOG_DEBUG, "Found next track to read: %d (timestamp: %lf / %lf)\n",
> +               track->index, av_q2d(track->current_timestamp), av_q2d(minimum_timestamp));

Coverity actually complained about track being uninitialized, which this
patch does not address. And the reason it does this is that it doesn't
understand the algorithm: track will always be initialized in the first
iteration of the loop. (If there is a first iteration of the loop -- is
this actually guaranteed? A file without tracks seems to be pretty useless.)
FYI: In Coverity's analysis there are loop iterations, but it just
assumed that track is not initialized in the loop (which boils down to
saying that it presumed the tracks' current_timestamp to be invalid
(denominator 0). I hope this can't happen.
(There is btw another issue: The initialization of minimum_timestamp
presumes that int are 32bit which need not be true.)

>      return track;
>  }
>  
> @@ -760,6 +761,8 @@ static int imf_read_packet(AVFormatContext *s, AVPacket *pkt)
>      AVRational next_timestamp;
>  
>      track = get_next_track_with_minimum_timestamp(s);
> +    if (!track)
> +        return AVERROR_INVALIDDATA;
>  
>      ret = get_resource_context_for_timestamp(s, track, &resource);
>      if (ret)