[FFmpeg-devel] [PATCH 0/2] Origin Wing Commander IV video decoder

Ronald S. Bultje rsbultje
Fri Feb 4 22:42:15 CET 2011


Hi,

On Thu, Feb 3, 2011 at 4:32 AM, Kostya <kostya.shishkov at gmail.com> wrote:
> +    const int pic_size = avctx->width * avctx->height;

You calculate that in various places, and it never changes. Just store
it in the context.

> +static int xan_decode_chroma(AVCodecContext *avctx, AVPacket *avpkt)
[..]
+    chroma_off = AV_RL32(buf + 4);
+    if (!chroma_off)
+        return 0;
+    if (chroma_off >= avpkt->size) {

if (chroma_off + 10 >= avpkt->size) {, because below you read various
more, if you want to be exact.

> +    src    = avpkt->data + 4 + chroma_off;
> +    table  = src + 2;
> +    mode   = bytestream_get_le16(&src);
> +    offset = bytestream_get_le16(&src) * 2;

> +    dec_size = xan_unpack(s->scratch_buffer, pic_size, src + offset,
> +                          avpkt->size - offset - (src - avpkt->data));

src+offset can overread but is never checked in xan_unpack() before
reading opcode.

> +    if (mode > 1)
> +        av_log(avctx, AV_LOG_WARNING, "Unknown chroma coding mode %d\n", mode);

But no error? Most likely damaged file, right?

> +    if (mode) {
> +        for (j = 0; j < avctx->height / 2; j++) {
> +            for (i = 0; i < avctx->width / 2; i++) {

(unimportant) / 2 -> >> 1.

> +        for (j = 0; j < avctx->height / 4; j++) {
> +            for (i = 0; i < avctx->width / 2; i += 2) {

Same.

> +static int xan_decode_frame_type0(AVCodecContext *avctx, AVPacket *avpkt)
[..]
> +    ybuf = s->y_buffer;
> +    last = *src++;
> +    ybuf[0] = last << 1;
> +    for (j = 1; j < avctx->width - 1; j += 2) {
> +        cur = (last + *src++) & 0x1F;
> +        ybuf[j]   = last + cur;
> +        ybuf[j+1] = cur << 1;
> +        last = cur;
> +    }
> +    ybuf[j] = last << 1;
> +    prev_buf = ybuf;
> +    ybuf += avctx->width;
> +
> +    for (i = 1; i < avctx->height; i++) {
> +        last = ((prev_buf[0] >> 1) + *src++) & 0x1F;
> +        ybuf[0] = last << 1;
> +        for (j = 1; j < avctx->width - 1; j += 2) {
> +            cur = ((prev_buf[j + 1] >> 1) + *src++) & 0x1F;
> +            ybuf[j]   = last + cur;
> +            ybuf[j+1] = cur << 1;
> +            last = cur;
> +        }
> +        ybuf[j] = last << 1;
> +        prev_buf = ybuf;
> +        ybuf += avctx->width;
[..]
> +    src = s->y_buffer;
> +    ybuf = s->pic.data[0];
> +    for (j = 0; j < avctx->height; j++) {
> +        for (i = 0; i < avctx->width; i++)
> +            ybuf[i] = (src[i] << 2) | (src[i] >> 3);
> +        src  += avctx->width;
> +        ybuf += s->pic.linesize[0];
> +    }
[..]
> +static int xan_decode_frame_type1(AVCodecContext *avctx, AVPacket *avpkt)
[..]
> +    ybuf = s->y_buffer;
> +    for (i = 0; i < avctx->height; i++) {
> +        last = (ybuf[0] + (*src++ << 1)) & 0x3F;
> +        ybuf[0] = last;
> +        for (j = 1; j < avctx->width - 1; j += 2) {
> +            cur = (ybuf[j + 1] + (*src++ << 1)) & 0x3F;
> +            ybuf[j]   = (last + cur) >> 1;
> +            ybuf[j+1] = cur;
> +            last = cur;
> +        }
> +        ybuf[j] = last;
> +        ybuf += avctx->width;
> +    }
> +
> +    src = s->y_buffer;
> +    ybuf = s->pic.data[0];
> +    for (j = 0; j < avctx->height; j++) {
> +        for (i = 0; i < avctx->width; i++)
> +            ybuf[i] = (src[i] << 2) | (src[i] >> 3);
> +        src  += avctx->width;
> +        ybuf += s->pic.linesize[0];
> +    }

Can src ever overread here?

Ronald



More information about the ffmpeg-devel mailing list