[FFmpeg-devel] [PATCH 1/2 v2] avformat/dashdec: enable overriding of the maximum manifest size

[Andreas Rheinhardt]

Jan Ekström:
> On Tue, Sep 1, 2020 at 9:31 PM Andreas Rheinhardt
> <andreas.rheinhardt at gmail.com> wrote:
>>
>> Jan Ekström:
>>> This enables people to override the sanity check without compiling
>>> a new binary.
>>> ---
>>>  libavformat/dashdec.c | 17 ++++++++++++++---
>>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
>>> index c5a5ff607b..4080b9b650 100644
>>> --- a/libavformat/dashdec.c
>>> +++ b/libavformat/dashdec.c
>>> @@ -160,6 +160,7 @@ typedef struct DASHContext {
>>>      int is_init_section_common_video;
>>>      int is_init_section_common_audio;
>>>
>>> +    uint64_t maximum_manifest_size;
>>>  } DASHContext;
>>>
>>>  static int ishttp(char *url)
>>> @@ -1256,14 +1257,21 @@ static int parse_manifest(AVFormatContext *s, const char *url, AVIOContext *in)
>>>      }
>>>
>>>      filesize = avio_size(in);
>>> -    if (filesize > MAX_MANIFEST_SIZE) {
>>> -        av_log(s, AV_LOG_ERROR, "Manifest too large: %"PRId64"\n", filesize);
>>> +    if (c->maximum_manifest_size && filesize > c->maximum_manifest_size) {
>>> +        av_log(s, AV_LOG_ERROR,
>>> +               "Manifest size too large: %"PRId64" (this sanity check can be "
>>> +               "adjusted by using the option 'maximum_manifest_size')\n",
>>> +               filesize);
>>>          return AVERROR_INVALIDDATA;
>>>      }
>>>
>>>      av_bprint_init(&buf, (filesize > 0) ? filesize + 1 : DEFAULT_MANIFEST_SIZE, AV_BPRINT_SIZE_UNLIMITED);
>>>
>>> -    if ((ret = avio_read_to_bprint(in, &buf, MAX_MANIFEST_SIZE)) < 0 ||
>>> +    if ((ret = avio_read_to_bprint(in, &buf,
>>> +                                   c->maximum_manifest_size > 0 ?
>>> +                                   c->maximum_manifest_size :
>>
>> You are treating zero as "no limit", despite this not being documented.
>>
> 
> Yes. I just wanted to see how people would respond to the idea. But it
> seems like due to the underlying bprint api I would in any case have
> to limit it to UINT_MAX, thus making an option of "don't limit the
> size" not really feasible.
> 
>>> +                                   (filesize > MAX_MANIFEST_SIZE ?
>>> +                                    filesize : MAX_MANIFEST_SIZE))) < 0 ||
>>
>> Would be clearer as FFMAX(filesize, MAX_MANIFEST_SIZE). But honestly I
>> have trouble understanding why you are not just using filesize here
>> (presuming it is >= 0, which is not checked here or anywhere).
> 
> True, that would be clearer. I think I just utilized it this way
> because if file size is larger than MAX_MANIFEST_SIZE, it should
> definitely be larger than zero :) .
> 

My thinking was this: If filesize is >= 0 and if it passed all checks
already, then just use avio_read_to_bprint(in, &buf, filesize). After
all, we know the filesize or is there some reason to believe it to be wrong?

> Jan
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>