[FFmpeg-devel] [PATCH 1/9] avformat/matroskaenc: Avoid atoi()

[Andreas Rheinhardt]

Andreas Rheinhardt:
> It has undefined behaviour in case the value does not fit into an int.
> Also stop allowing to override a stream level "alpha_mode" tag
> by an AVFormatContext one and properly check that the stereo_mode
> number given via a tag is actually in the range 0..14: Negative
> values would have been treated as zero before this patch.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
>  libavformat/matroskaenc.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index e813ef86cf..9bdf087c67 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1592,7 +1592,7 @@ static int mkv_write_stereo_mode(AVFormatContext *s, EbmlWriter *writer,
>      // convert metadata into proper side data and add it to the stream
>      if ((tag = av_dict_get(st->metadata, "stereo_mode", NULL, 0)) ||
>          (tag = av_dict_get( s->metadata, "stereo_mode", NULL, 0))) {
> -        int stereo_mode = atoi(tag->value);
> +        long stereo_mode = strtol(tag->value, NULL, 0);
>  
>          for (int i = 0; i < MATROSKA_VIDEO_STEREOMODE_TYPE_NB; i++)
>              if (!strcmp(tag->value, ff_matroska_video_stereo_mode[i])){
> @@ -1600,7 +1600,7 @@ static int mkv_write_stereo_mode(AVFormatContext *s, EbmlWriter *writer,
>                  break;
>              }
>  
> -        if (stereo_mode < MATROSKA_VIDEO_STEREOMODE_TYPE_NB &&
> +        if ((unsigned long)stereo_mode < MATROSKA_VIDEO_STEREOMODE_TYPE_NB &&
>              stereo_mode != 10 && stereo_mode != 12) {
>              int ret = ff_mkv_stereo3d_conv(st, stereo_mode);
>              if (ret < 0)
> @@ -1748,11 +1748,10 @@ static int mkv_write_track_video(AVFormatContext *s, MatroskaMuxContext *mkv,
>      if (ret < 0)
>          return ret;
>  
> -    if (((tag = av_dict_get(st->metadata, "alpha_mode", NULL, 0)) && atoi(tag->value)) ||
> -        ((tag = av_dict_get( s->metadata, "alpha_mode", NULL, 0)) && atoi(tag->value)) ||
> -        (par->format == AV_PIX_FMT_YUVA420P)) {
> +    if (par->format == AV_PIX_FMT_YUVA420P ||
> +        ((tag = av_dict_get(st->metadata, "alpha_mode", NULL, 0)) ||
> +         (tag = av_dict_get( s->metadata, "alpha_mode", NULL, 0))) && strtol(tag->value, NULL, 0))
>          ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEOALPHAMODE, 1);
> -    }
>  
>      // write DisplayWidth and DisplayHeight, they contain the size of
>      // a single source view and/or the display aspect ratio

Will apply this patchset tomorrow unless there are objections.

- Andreas