Kacper Michajlow:
> On Fri, 4 Jul 2025 at 20:22, Andreas Rheinhardt
> <andreas.rheinha...@outlook.com> wrote:
>>
>> Kacper Michajłow:
>>> av_get_token() allocates an output buffer with the same size as the
>>> input. Generally, this is harmless, but when the input string is large
>>> and consists of many small tokens, calling av_get_token() repeatedly to
>>> extract all tokens will significantly amplify memory allocations.
>>>
>>> To fix this, after obtaining the return value, simply realloc the buffer
>>> to the actual size needed for output string.
>>>
>>> Fixes OOM when parsing filter graph string.
>>> Fixes OSS-Fuzz: 394983446
>>>
>>> Signed-off-by: Kacper Michajłow <kaspe...@gmail.com>
>>> ---
>>> libavutil/avstring.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavutil/avstring.c b/libavutil/avstring.c
>>> index 875eb691db..b4266aefe5 100644
>>> --- a/libavutil/avstring.c
>>> +++ b/libavutil/avstring.c
>>> @@ -142,7 +142,7 @@ end:
>>>
>>> char *av_get_token(const char **buf, const char *term)
>>> {
>>> - char *out = av_malloc(strlen(*buf) + 1);
>>> + char *out = av_realloc(NULL, strlen(*buf) + 1);
>>
>> I thought pointers provided by av_malloc could be used with
>> av_realloc()? Or is it to avoid the unnecessary alignment provided by
>> av_malloc()?
>
> I was thinking about this. But the docs, of av_realloc() say
>
> * @param ptr Pointer to a memory block already allocated with
> * av_realloc() or `NULL`
>
> so to honor this, I changed to av_realloc(NULL). I've looked at other
> uses and it seems to be a pattern that is used for cases like this
> one.
>
>>> char *ret = out, *end = out;
>>> const char *p = *buf;
>>> if (!out)
>>> @@ -172,7 +172,7 @@ char *av_get_token(const char **buf, const char *term)
>>>
>>> *buf = p;
>>>
>>> - return ret;
>>> + return av_realloc(ret, out - ret + 2);
>>
>> You seem to presume that av_realloc() can't fail when used for
>> shrinking. But this is not documented.
>
> This function returns a pointer. So if it fails, returning NULL is
> valid. Unless you mean that we add another possible point of failure?
> I don't think it matters, if we are already close to OOM state, this
> small alloc wouldn't be a difference maker.
>
There is a leak in case of reallocation failure. Which is unacceptable.
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
