[FFmpeg-devel] [PATCH] avformat/asfdec: fix crash caused by free wlid pointers

[Andreas Rheinhardt]

XiaoYang:
> 
> 
> 
> At 2022-01-11 17:29:35, "Andreas Rheinhardt" <andreas.rheinhardt at outlook.com> wrote:
>> Yang Xiao:
>>> From: Yang Xiao <yshaw99 at outlook.com>
>>>
>>> This commit fixed a crash when seeking wma frames, asf decoder will try to demux in function asf_read_pts().
>>> Pointer member side_data of AVPacket that allocated by stack may be wild pointer.
>>> Prevent releasing wild pointers in AVPacket when some functions try to call av_packet_unref, example av_read_frame().
>>> ---
>>>  libavformat/asfdec_f.c | 2 +-
>>>  libavformat/mpc.c      | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
>>> index a8f36ed286..bae0ecfc7c 100644
>>> --- a/libavformat/asfdec_f.c
>>> +++ b/libavformat/asfdec_f.c
>>> @@ -1433,7 +1433,7 @@ static int64_t asf_read_pts(AVFormatContext *s, int stream_index,
>>>  {
>>>      FFFormatContext *const si = ffformatcontext(s);
>>>      ASFContext *asf     = s->priv_data;
>>> -    AVPacket pkt1, *pkt = &pkt1;
>>> +    AVPacket pkt1 = {0}, *pkt = &pkt1;
>>>      ASFStream *asf_st;
>>>      int64_t pts;
>>>      int64_t pos = *ppos;
>>> diff --git a/libavformat/mpc.c b/libavformat/mpc.c
>>> index b5b2bab33c..ad0d693152 100644
>>> --- a/libavformat/mpc.c
>>> +++ b/libavformat/mpc.c
>>> @@ -189,7 +189,7 @@ static int mpc_read_seek(AVFormatContext *s, int stream_index, int64_t timestamp
>>>      AVStream *st = s->streams[stream_index];
>>>      FFStream *const sti = ffstream(st);
>>>      MPCContext *c = s->priv_data;
>>> -    AVPacket pkt1, *pkt = &pkt1;
>>> +    AVPacket pkt1 = {0}, *pkt = &pkt1;
>>>      int ret;
>>>      int index = av_index_search_timestamp(st, FFMAX(timestamp - DELAY_FRAMES, 0), flags);
>>>      uint32_t lastframe;
>>>
>>
>> Do you have FF_API_INIT_PACKET set to 0 (it should still be set to 1)?
>> Because av_read_frame() is supposed to (and documented to) treat the
>> packet it is given as uninitialized.
> 
>>
>> - Andreas
> 
> 
> 
> Thanks for your comment!
> But I have a question,  av_init_packet() has been markedattribute_deprecated, If av_init_packet() is deprecated completely in the future, how to ensure ff_read_packet() safely (support to input a uninitialized packet) ? AVPacket->side_data will not be safely initialized with only av_packet_unref().
> 

Uninitialized packets won't be a thing in the future because all packets
will in the future originate from av_packet_alloc() (save for some in
libavcodec itself).
(This does not mean that I like the av_packet_unref(); it'd rather
document that the packet has to be blank (as if coming from
av_packet_alloc(), av_packet_unref() or as if it had been used as src
packet in av_packet_move_ref()).)

> 
> And as a user of the FFmpeg API, I want to disable some of the functions marked attribute_deprecated early, because upgrading FFmpeg has a large impact. 
> Do I have other ways to achieve this goal?
> 

Actually, you are not really supposed to set the FF_API_* defines
yourself. Although adding a configure option that already disables as
many of these as possible would be nice for users to test their
compatibility with future versions as well as for users that don't need
the old API already.

- Andreas