[FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

[Chris Cunningham]

>
> >> And this definitely means there's a bug elsewhere. This parser should
> >> have not been used for codecs ids other than GSM and GSM_MS. That's
> >> precisely what this assert() is making sure of.
> >>
> >
> > I'll take a closer look at how this parser was selected.
>

Ok, here are full details of how this happens:

   1. AV_CODEC_ID_GSM_MS is assigned to on st->codecpar->codec_id
   in ogm_header() (oggparseogm.c:75)
   2. The (deprecated) st->codec->codec_id is not assigned at that time and
   remains 0 (UNKNOWN)
   3. AV_CODEC_ID_GSM_MS from st->codecpar is passed to av_parser_init,
   selecting the gsm parser (in read_frame_internal())
   4. The fuzzer next (only) packet has a size of 0, so the first call to
   parse_packet() (still in read_frame_internal()) does nothing
   5. After this call we assign several members of st->internal->avctx to
   st->codecpar. This leaves codecpar->codec_id = UNKNOWN. Interestingly, we
   do not reset the st->parser at this point (maybe we should?)
   6. Next iteration of the read_frame_internal() loop we get EOF from
   ff_read_packet. This triggers the "flush the parsers" call to
   parse_packet() which finally reaches gsm_parse() to trigger the abort
   (avctx->codec_id is still 0).

Questions (guessing at the fix):

   - At what point should st->codec->codec_id be synchronized with
   st->codecpar->codec_id? It never is in this test.
   - Should we reset st->parser whenever we reset st->codecpar->codec_id
   (step 5 above)?

Thanks,
Chris