[FFmpeg-devel] [PATCH 2/2] avformat/webm_chunk: Check header filename length

[Andreas Rheinhardt]

Michael Niedermayer:
> On Fri, May 03, 2019 at 06:31:00AM +0000, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>> ---
>>>  libavformat/webm_chunk.c | 8 +++++++-
>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/webm_chunk.c b/libavformat/webm_chunk.c
>>> index 561ec152e7..e2fbd8be1d 100644
>>> --- a/libavformat/webm_chunk.c
>>> +++ b/libavformat/webm_chunk.c
>>> @@ -88,6 +88,8 @@ static int get_chunk_filename(AVFormatContext *s, int is_header, char filename[M
>>>  {
>>>      WebMChunkContext *wc = s->priv_data;
>>>      AVFormatContext *oc = wc->avf;
>>> +    int len;
>>> +
>>>      if (!filename) {
>>>          return AVERROR(EINVAL);
>>>      }
>>> @@ -96,7 +98,11 @@ static int get_chunk_filename(AVFormatContext *s, int is_header, char filename[M
>>>              av_log(oc, AV_LOG_ERROR, "No header filename provided\n");
>>>              return AVERROR(EINVAL);
>>>          }
>>> -        av_strlcpy(filename, wc->header_filename, MAX_FILENAME_SIZE);
>>> +        len = av_strlcpy(filename, wc->header_filename, MAX_FILENAME_SIZE);
>>> +        if (len >= MAX_FILENAME_SIZE) {
>>> +            av_log(oc, AV_LOG_ERROR, "header filename too long\n");
>>> +            return AVERROR(EINVAL);
>>> +        }
>>>      } else {
>>>          if (av_get_frame_filename(filename, MAX_FILENAME_SIZE,
>>>                                    s->url, wc->chunk_index - 1) < 0) {
>>>
>> len has an unnecessarily broad scope. The string is intentionally
>> started with a lower case letter because the parameter "header" is
>> lower case, too, isn't it? If so, it's fine and LGTM apart from the scope.
> 
> Ill capitalize the error message and move the "int len" into the block
> 
> thanks
> 
Good. The patch that apparently aroused your interest in the
webm_chunk muxer was part of a patchset that was mainly about fixing
ticket #5752, a NULL pointer dereference. Would you mind taking a look
at the other patches? They are here:
https://ffmpeg.org/pipermail/ffmpeg-devel/2019-April/242903.html
https://ffmpeg.org/pipermail/ffmpeg-devel/2019-April/242902.html

- Andreas