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

[James Almer]

On 1/30/2019 10:20 PM, Chris Cunningham wrote:
>>
>>>> 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?)

Where does this happen? A call to avcodec_parameters_from_context()
should also copy codec_id.

>    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.

In avformat_find_stream_info(), i think. In any case, st->codec should
have no effect on parsers. What is used in parse_packet() however is
st->internal->avctx, so that context is the one that evidently contains
codec_id == UNKNOWN when it should be in sync with codecpar.

>    - Should we reset st->parser whenever we reset st->codecpar->codec_id
>    (step 5 above)?
> 
> Thanks,
> Chris
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>