[FFmpeg-devel] [PATCH] avformat/vpk: fix divide by zero

[Kacper Michajlow]

On Fri, 9 Aug 2024 at 22:51, Michael Niedermayer <michael at niedermayer.cc> wrote:
>
> On Wed, Aug 07, 2024 at 03:42:46PM +0200, Kacper Michajłow wrote:
> > Can happen after calling avformat_find_stream_info() when the codec
> > fails to open, but return value is 0 and subsequent uses of this context
> > have zero value in channel number.
> >
> > Found by OSS-Fuzz.
> >
> > Signed-off-by: Kacper Michajłow <kasper93 at gmail.com>
> > ---
> >  libavformat/vpk.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/libavformat/vpk.c b/libavformat/vpk.c
> > index 001ad33555..aa98ef2dd4 100644
> > --- a/libavformat/vpk.c
> > +++ b/libavformat/vpk.c
> > @@ -86,6 +86,8 @@ static int vpk_read_packet(AVFormatContext *s, AVPacket *pkt)
> >
> >      vpk->current_block++;
> >      if (vpk->current_block == vpk->block_count) {
> > +        if (par->ch_layout.nb_channels <= 0)
> > +            return AVERROR_INVALIDDATA;
> >          unsigned size = vpk->last_block_size / par->ch_layout.nb_channels;
> >          unsigned skip = (par->block_align - vpk->last_block_size) / par->ch_layout.nb_channels;
> >          uint64_t pos = avio_tell(s->pb);
>
> iam not sure if a parser or other should replace a valid set of
> parameters by an invalid
> (this patch implies that such a action occured)
>
> can you explain more detailedly by what and why channels is set to 0 ?
>

You are right, it might be better to improve this to not override the
params. Let me explain what happens, I didn't read through the whole
avformat_find_stream_info() to know what would be the best approach
yet. I will try to look at it, but if you have immediate ideas, that
would be nice.

1. avformat_open_input() sets nb_channels to 108

2. Just after that we call avformat_find_stream_info(avfc, NULL); this
returns 0 (success), but as a result it overrides params already
present in the context.
log for reference, during the find stream info call
[ffmpeg/demuxer] vpk: Before avformat_find_stream_info() pos:
538976288 bytes read:21 seeks:1 nb_streams:1
[ffmpeg/demuxer] vpk: Failed to open codec in avformat_find_stream_info
[lavf] mp_seek(0x512000018090, 0, size)
[lavf] 0=mp_read(0x512000018090, 0x7fe4c7ce8800, 50000000), pos:
538976288, eof:1
[lavf] 0=mp_read(0x512000018090, 0x52d00000a400, 32768), pos: 538976288, eof:1
[ffmpeg/audio] adpcm_psx: Decoder requires channel layout to be set
[ffmpeg/demuxer] vpk: Failed to open codec in avformat_find_stream_info
[lavf] mp_seek(0x512000018090, 0, size)
[lavf] mp_seek(0x512000018090, 0, size)
[lavf] mp_seek(0x512000018090, 0, size)
[ffmpeg/demuxer] vpk: stream 0: start_time: NOPTS duration: 0.069852
[ffmpeg/demuxer] vpk: format: start_time: NOPTS duration: 0.069852
(estimate from stream) bitrate=2 kb/s
[ffmpeg/demuxer] vpk: Could not find codec parameters for stream 0
(Audio: adpcm_psx, 538976288 Hz, 0 channels): unspecified sample
format
[ffmpeg/demuxer] Consider increasing the value for the
'analyzeduration' (0) and 'probesize' (5000000) options
[ffmpeg/demuxer] vpk: After avformat_find_stream_info() pos: 538976288
bytes read:21 seeks:1 frames:0

3. the nb_channels value is cleared in avformat_find_stream_info() ->
avcodec_parameters_from_context() -> codec_parameters_reset() and
remains 0.

4. as we can see there were errors, but it still returns success, so we proceed.

5. on the next av_seek_frame() which goes to vpk_read_packet() it will
fail because nb_channels is 0 at this point.

Sorry for only a high level overview, but at this moment, I'm not sure
how exactly it is supposed to work. I thought it might be intended to
override headers parameters later if we know better later on, that's
why my initial patch only tackled this case.

- Kacper