[FFmpeg-cvslog] Apple ProRes decoder

[Reimar Döffinger]

On Fri, Sep 23, 2011 at 12:37:48AM +0200, Maxim Poliakovski wrote:
> ffmpeg | branch: master | Maxim Poliakovski <max_pole at gmx.de> | Wed Sep 21 13:09:32 2011 +0200| [be64629a135642f20325e1422352707bb81d5c91] | committer: Martin Storsjö
> 
> Apple ProRes decoder
> 
> Signed-off-by: Martin Storsjö <martin at martin.st>

Well, now the mess is fairly complete.

> +    int        chroma_factor;
> +    int        mb_chroma_factor;

I can't see the point in having both those, particularly with none
documented.

> +    width  = AV_RB16(buf + 8);
> +    height = AV_RB16(buf + 10);
> +    if (width != avctx->width || height != avctx->height) {
> +        av_log(avctx, AV_LOG_ERROR,
> +               "picture dimension changed! Old: %d x %d, new: %d x %d\n",
> +               avctx->width, avctx->height, width, height);
> +        return -1;

Why is that an error.

> +        av_log(avctx, AV_LOG_ERROR,
> +               "unsupported picture format: %d!\n", ctx->pic_format);

This is the only place where ctx->pic_format is used, that surely is
wrong.

> +    num_slices = num_x_slices * ctx->num_y_mbs;
> +        ctx->slice_data_index =
> +            av_malloc((num_slices + 1) * sizeof(uint8_t*));
> +    if (hdr_size + num_slices * 2 > data_size) {
> +        data_ptr += AV_RB16(index_ptr + i * 2);

Is it sure none of these can overflow with crafted files?

> +    if (data_ptr > buf + data_size) {

It's not really correct C to check a pointer _after_ it already
went beyond the bounds.
In fact this check actually has a high chance of missing an overflow
if buf is on-stack.
Note that I suspect most of these apply to the GPL variant as well,
though this last one is slightly less risky there.