[FFmpeg-devel] avcodec/bsf: set pctx to NULL when av_bsf_alloc failed

av_bsf_free will free invalid pointer when av_bsf_alloc failed.
because av_bsf_list_parse_str called av_bsf_get_null_filter,
av_bsf_get_null_filter called av_bsf_alloc, and av_bsf_alloc
should set a value to the *pctx before return success or failed,
because it dose not initial a null pointer ever, so it will free
invalid pointer in av_bsf_free which is called by ff_decode_bsfs_init.

Found-by: Zu-Ming Jiang <jjzuming@outlook.com>
Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavcodec/bsf.c | 1 +
 1 file changed, 1 insertion(+)

On 1/16/2021 2:24 AM, Steven Liu wrote:
> av_bsf_free will free invalid pointer when av_bsf_alloc failed.
> because av_bsf_list_parse_str called av_bsf_get_null_filter,
> av_bsf_get_null_filter called av_bsf_alloc, and av_bsf_alloc
> should set a value to the *pctx before return success or failed,
> because it dose not initial a null pointer ever, so it will free
> invalid pointer in av_bsf_free which is called by ff_decode_bsfs_init.

The pointer passed to av_bsf_list_parse_str() in ff_decode_bsfs_init() 
is already NULL, because avctx->internal was allocated with av_mallocz().

In what scenario is av_bsf_free() getting an invalid pointer?

> 
> Found-by: Zu-Ming Jiang <jjzuming@outlook.com>
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>   libavcodec/bsf.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/bsf.c b/libavcodec/bsf.c
> index d71bc32584..5bb3349138 100644
> --- a/libavcodec/bsf.c
> +++ b/libavcodec/bsf.c
> @@ -141,6 +141,7 @@ int av_bsf_alloc(const AVBitStreamFilter *filter, AVBSFContext **pctx)
>       return 0;
>   fail:
>       av_bsf_free(&ctx);
> +    *pctx = NULL;
>       return ret;
>   }
>   
>

> 在 2021年1月16日，21:19，James Almer <jamrial@gmail.com> 写道：
> 
> On 1/16/2021 2:24 AM, Steven Liu wrote:
>> av_bsf_free will free invalid pointer when av_bsf_alloc failed.
>> because av_bsf_list_parse_str called av_bsf_get_null_filter,
>> av_bsf_get_null_filter called av_bsf_alloc, and av_bsf_alloc
>> should set a value to the *pctx before return success or failed,
>> because it dose not initial a null pointer ever, so it will free
>> invalid pointer in av_bsf_free which is called by ff_decode_bsfs_init.
> 
> The pointer passed to av_bsf_list_parse_str() in ff_decode_bsfs_init() is already NULL, because avctx->internal was allocated with av_mallocz().
It’s good point, yes, you are right.
> 
> In what scenario is av_bsf_free() getting an invalid pointer?
only call ff_decode_bsfs_init and not use avcodec_open2, 
call it by myself write internal code, maybe nobody will use it like my way :(
> 
>> Found-by: Zu-Ming Jiang <jjzuming@outlook.com>
>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
>> ---
>>  libavcodec/bsf.c | 1 +
>>  1 file changed, 1 insertion(+)
>> diff --git a/libavcodec/bsf.c b/libavcodec/bsf.c
>> index d71bc32584..5bb3349138 100644
>> --- a/libavcodec/bsf.c
>> +++ b/libavcodec/bsf.c
>> @@ -141,6 +141,7 @@ int av_bsf_alloc(const AVBitStreamFilter *filter, AVBSFContext **pctx)
>>      return 0;
>>  fail:
>>      av_bsf_free(&ctx);
>> +    *pctx = NULL;
>>      return ret;
>>  }
>>  
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Thanks
Steven