[FFmpeg-devel] [FFmpeg-cvslog] avcodec/hcadec: support decoding with extradata provided in first packet

[James Almer]

On 10/2/2023 7:23 PM, Michael Niedermayer wrote:
> Hi
> 
> On Tue, Sep 05, 2023 at 09:25:45PM +0000, Paul B Mahol wrote:
>> ffmpeg | branch: master | Paul B Mahol <onemda at gmail.com> | Tue Sep  5 23:14:58 2023 +0200| [d464a687c9dd03246795d62151809167e8381932] | committer: Paul B Mahol
>>
>> avcodec/hcadec: support decoding with extradata provided in first packet
> 
> I cannot find this patch on the mailing list
> 
> Also this adds null pointer writes
> The init_hca() function which previously was only called once and failure
> ended all further processing now is called optionally per frame and its
> failure does not stop further processing so half initialized contexts
> can be created by an attacker
> 
> Note, this sort of stuff delays the release
> 
> thx

Does the following fix it?

> diff --git a/libavcodec/hcadec.c b/libavcodec/hcadec.c
> index 6f277afb96..4e30d553de 100644
> --- a/libavcodec/hcadec.c
> +++ b/libavcodec/hcadec.c
> @@ -65,6 +65,7 @@ typedef struct HCAContext {
>      uint8_t stereo_band_count;
>      uint8_t bands_per_hfr_group;
> 
> +    // Set during init() and freed on close(). Untouched on flush()
>      av_tx_fn           tx_fn;
>      AVTXContext       *tx_ctx;
>      AVFloatDSPContext *fdsp;
> @@ -196,6 +197,13 @@ static inline unsigned ceil2(unsigned a, unsigned b)
>      return (b > 0) ? (a / b + ((a % b) ? 1 : 0)) : 0;
>  }
> 
> +static av_cold void decode_flush(AVCodecContext *avctx)
> +{
> +    HCAContext *c = avctx->priv_data;
> +
> +    memset(c, 0, offsetof(HCAContext, tx_fn));
> +}
> +
>  static int init_hca(AVCodecContext *avctx, const uint8_t *extradata,
>                      const int extradata_size)
>  {
> @@ -205,6 +213,8 @@ static int init_hca(AVCodecContext *avctx, const uint8_t *extradata,
>      unsigned b, chunk;
>      int version, ret;
> 
> +    decode_flush(avctx);
> +
>      if (extradata_size < 36)
>          return AVERROR_INVALIDDATA;
> 
> @@ -340,6 +350,9 @@ static int init_hca(AVCodecContext *avctx, const uint8_t *extradata,
>              return AVERROR_INVALIDDATA;
>      }
> 
> +    // Done last to signal init() finished
> +    c->crc_table = av_crc_get_table(AV_CRC_16_ANSI);
> +
>      return 0;
>  }
> 
> @@ -350,7 +363,6 @@ static av_cold int decode_init(AVCodecContext *avctx)
>      int ret;
> 
>      avctx->sample_fmt = AV_SAMPLE_FMT_FLTP;
> -    c->crc_table = av_crc_get_table(AV_CRC_16_ANSI);
> 
>      if (avctx->ch_layout.nb_channels <= 0 || avctx->ch_layout.nb_channels > FF_ARRAY_ELEMS(c->ch))
>          return AVERROR(EINVAL);
> @@ -534,6 +546,9 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
>          }
>      }
> 
> +    if (!c->crc_table)
> +        return AVERROR_INVALIDDATA;
> +
>      if (c->key || c->subkey) {
>          uint8_t *data, *cipher = c->cipher;
> 
> @@ -602,6 +617,7 @@ const FFCodec ff_hca_decoder = {
>      .priv_data_size = sizeof(HCAContext),
>      .init           = decode_init,
>      FF_CODEC_DECODE_CB(decode_frame),
> +    .flush          = decode_flush,
>      .close          = decode_close,
>      .p.capabilities = AV_CODEC_CAP_DR1,
>      .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,