[FFmpeg-devel] [PATCH] Add CPiA video decoder

[Michael Niedermayer]

On Thu, Aug 30, 2012 at 11:34:03AM +0200, Stephan Hilb wrote:
> My bad, I missed to remove the reference to the cpia_decode_end
> function, now fixed.
> 

[...]

> +static int cpia_decode_frame(AVCodecContext* avctx,
> +        void* data, int* data_size, AVPacket* avpkt)
> +{
> +    CpiaContext* const cpia = avctx->priv_data;
> +    int i,j;
> +
> +    uint8_t* const header = avpkt->data;
> +    uint8_t* src;
> +    int src_size;
> +    uint16_t linelength;
> +    uint8_t skip;
> +
> +    AVFrame* const frame = &cpia->frame;
> +    uint8_t *y, *u, *v;
> +
> +    // Get buffer filled with previous frame

> +    if (avctx->reget_buffer(avctx, frame)) {
> +        av_log(avctx, AV_LOG_ERROR, "reget_buffer() failed!\n");
> +        return -1;
> +    }

the error code from reget_buffer could be used as return


> +
> +    // Check header
> +    if (    avpkt->size < FRAME_HEADER_SIZE
> +        ||    header[0] != MAGIC_0 || header[1] != MAGIC_1
> +        ||    (header[17] != SUBSAMPLE_420 && header[17] != SUBSAMPLE_422)
> +        ||    (header[18] != YUVORDER_YUYV && header[18] != YUVORDER_UYVY)
> +        ||    (header[28] != NOT_COMPRESSED && header[28] != COMPRESSED)
> +        ||    (header[29] != NO_DECIMATION && header[29] != DECIMATION_ENAB)
> +    ) {
> +        av_log(avctx, AV_LOG_ERROR, "Invalid header!\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +

> +    // currently unsupported properties
> +    if (header[17] == SUBSAMPLE_422) {
> +        av_log(avctx, AV_LOG_ERROR, "Unsupported subsample!\n");
> +        return -1;
> +    }
> +    if (header[18] == YUVORDER_UYVY) {
> +        av_log(avctx, AV_LOG_ERROR, "Unsupported YUV byte order!\n");
> +        return -1;
> +    }
> +    if (header[29] == DECIMATION_ENAB) {
> +        av_log(avctx, AV_LOG_ERROR, "Decimation unsupported!\n");
> +        return -1;
> +    }

the return codes could be changed to AVERROR_PATCHWELCOME


> +
> +    src = header + FRAME_HEADER_SIZE;
> +    src_size = avpkt->size - FRAME_HEADER_SIZE;
> +
> +    if (header[28] == NOT_COMPRESSED) {
> +        frame->pict_type = AV_PICTURE_TYPE_I;
> +        frame->key_frame = 1;
> +    } else {
> +        frame->pict_type = AV_PICTURE_TYPE_P;
> +        frame->key_frame = 0;
> +    }
> +
> +
> +    for ( i = 0;
> +          i < frame->height;
> +          i++, src += linelength, src_size -= linelength
> +    ) {
> +        // Read line length, two byte little endian
> +        linelength = AV_RL16(src);
> +        src += 2;
> +
> +        if (src_size < linelength) {
> +            frame->decode_error_flags = FF_DECODE_ERROR_INVALID_BITSTREAM;
> +            av_log(avctx, AV_LOG_WARNING, "Frame ended enexpectedly!\n");
> +            break;
> +        }
> +        if (src[linelength - 1] != EOL) {
> +            frame->decode_error_flags = FF_DECODE_ERROR_INVALID_BITSTREAM;
> +            av_log(avctx, AV_LOG_WARNING, "Wrong line length %d or line not terminated properly (found 0x%02x)!\n", linelength, src[linelength - 1]);
> +            break;
> +        }
> +
> +        /* Update the data pointers. Y data is on every line.
> +         * U and V data on every second line
> +         */
> +        y = &frame->data[0][i * frame->linesize[0]];
> +        u = &frame->data[1][(i >> 1) * frame->linesize[1]];
> +        v = &frame->data[2][(i >> 1) * frame->linesize[2]];
> +

> +        if ((i & 1) && header[17] == SUBSAMPLE_420) {
> +            /* We are on a odd line and 420 subsample is used.
> +             * On this line only Y values are specified, one per pixel.
> +             */
> +            for (j = 0; j < linelength - 1; j++) {
> +                if (src[j] & 1 && header[28] == COMPRESSED) {
> +                    /* It seems that odd lines are always uncompressed, but
> +                     * we do it according to specification anyways.
> +                     */
> +                    skip = src[j] >> 1;
> +                    y += skip;
> +                } else {
> +                    *(y++) = src[j];
> +                }
> +            }

This looks exploitable, there is no check that  the writes are within
the picture
the same issue exists in the u/v code

also if you like to be the official maintainer of this, then add
yourself to MAINTAINERS

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120830/a12ce506/attachment.asc>