[FFmpeg-devel] [PATCH] add Weston Capture demuxer, parser and decoder

[Tomas Härdin]

On Sun, 2017-09-10 at 21:03 +0200, Paul B Mahol wrote:
> +static av_cold int wcap_decode_init(AVCodecContext *avctx)
> +{
> +    WCAPContext *s = avctx->priv_data;
> +    uint32_t format;
> +
> +    if (avctx->extradata && avctx->extradata_size >= 4) {
> +        format = AV_RL32(avctx->extradata);
> +
> +        switch (format) {
> +        case 0x34325852: avctx->pix_fmt = AV_PIX_FMT_RGB0; break;
> +        case 0x34325842: avctx->pix_fmt = AV_PIX_FMT_BGR0; break;
> +        case 0x34325258: avctx->pix_fmt = AV_PIX_FMT_0RGB; break;
> +        case 0x34324258: avctx->pix_fmt = AV_PIX_FMT_0BGR; break;
> +        }

default: return AVERROR_INVALIDDATA ?

> +    }
> +
> +    s->frame = av_frame_alloc();
> +    if (!s->frame)
> +        return AVERROR(ENOMEM);
> +
> +    return 0;
> +}
> +
> +static void clear(AVCodecContext *avctx)
> +{
> +    WCAPContext *s = avctx->priv_data;
> +    int y;
> +
> +    if (!s->frame->buf[0])
> +        return;
> +
> +    for (y = 0; y < avctx->height; y++) {
> +        memset(s->frame->data[0] + y * s->frame->linesize[0], 0,
> avctx->width * 4);
> +    }
> +}

Wasn't there a AVFrame clear function added to lavu recently?

> +
> +static int wcap_decode_frame(AVCodecContext *avctx, void *data,
> +                            int *got_frame, AVPacket *avpkt)
> +{
> +    WCAPContext *s = avctx->priv_data;
> +    AVFrame *frame = s->frame;
> +    uint32_t nrects, x1, y1, x2, y2;
> +    int ret, n, i, k, x;
> +    GetByteContext gb;
> +    uint8_t *dst;
> +
> +    if ((ret = av_image_check_size(avctx->width, avctx->height, 0,
> NULL)) < 0)
> +        return ret;
> +
> +    bytestream2_init(&gb, avpkt->data, avpkt->size);
> +
> +    if ((ret = ff_reget_buffer(avctx, frame)) < 0)
> +        return ret;
> +
> +    if (avpkt->flags & AV_PKT_FLAG_KEY) {
> +        clear(avctx);
> +    }
> +
> +    bytestream2_skip(&gb, 4);
> +    nrects = bytestream2_get_le32(&gb);
> +
> +    for (n = 0; n < nrects; n++) {

n is signed, nrects is unsigned -> infinite loop for any value >=
0x80000000

Might want to bound nrecs <= avpkt->size / 20 too, or check if
the GetByteContext ran out of bits

> +        x1 = bytestream2_get_le32(&gb);
> +        y1 = bytestream2_get_le32(&gb);
> +        x2 = bytestream2_get_le32(&gb);
> +        y2 = bytestream2_get_le32(&gb);
> +
> +        if (x1 >= x2 || y1 >= y2 || x2 > avctx->width || y2 > avctx-
> >height ||
> +            (x2 - x1) > avctx->width || (y2 - y1) > avctx->height)
> +            return AVERROR_INVALIDDATA;
> +
> +        x = x1;
> +        dst = frame->data[0] + (avctx->height - y1 - 1) * frame-
> >linesize[0];
> +
> +        for (i = 0; i < (x2 - x1) * (y2 - y1);) {

Frames can't be larger than 2 Gi pixels, right?

> +            unsigned v = bytestream2_get_le32(&gb);
> +            int run_len = v >> 24;
> +
> +            if (run_len < 0xE0)
> +                run_len++;
> +            else
> +                run_len = 1 << (run_len - 0xE0 + 7);

This overflows on 32-bit if run_len >= 0xF8

(holy crap, C's lack of static checks is annoying)

> +
> +            i += run_len;
> +            for (k = 0; k < run_len; k++) {
> +                dst[x*4 + 1] += v & 0xFF;
> +                dst[x*4 + 2] += (v >> 8) & 0xFF;
> +                dst[x*4 + 3] += (v >> 16) & 0xFF;
> +                x++;
> +                if (x == x2) {
> +                    x = x1;
> +                    dst -= frame->linesize[0];
> +                }

This can easily write past the end of the frame


/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170910/04cd4490/attachment.sig>