Opened 6 years ago

Closed 6 years ago

#7250 closed defect (needs_more_info)

There are several potential out-of-bounds access vulnerabilities because of missing check for avctx->height and avctx->width

Reported by: Yooooooha Owned by:
Priority: normal Component: avcodec
Version: git-master Keywords: crash
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:
There are several potential out-of-bounds access vulnerabilities because of missing check for avctx->height and avctx->width.

Files and its function that should be patches.
libavcodec/smc.c: smc_decode_init
libavcodec/tiertexseqv.c: seqvideo_decode_init
libavcodec/qtrle.c: qtrle_decode_init
libavcodec/cinepak.c: cinepak_decode_init
libavcodec/msrle.c: msrle_decode_init
libavcodec/dsicinvideo.c: cinvideo_decode_init
libavcodec/msvideo1.c: msvideo1_decode_init

Below are the proposal patches for each file above.
libavcodec/smc.c: smc_decode_init

static av_cold int smc_decode_init(AVCodecContext *avctx)
{
    SmcContext *s = avctx->priv_data;

    s->avctx = avctx;
    avctx->pix_fmt = AV_PIX_FMT_PAL8;

+    if (!avctx->width || !avctx->height ||
+        (avctx->width & 1) || (avctx->height & 1)) {
+        av_log(avctx, AV_LOG_ERROR, "Invalid video dimensions: %dx%d\n",
+               avctx->width, avctx->height);
+        return AVERROR(EINVAL);
+    }

    s->frame = av_frame_alloc();
    if (!s->frame)
        return AVERROR(ENOMEM);

    return 0;
}

libavcodec/tiertexseqv.c: seqvideo_decode_init

static av_cold int seqvideo_decode_init(AVCodecContext *avctx)
{
    SeqVideoContext *seq = avctx->priv_data;
    int ret;

    seq->avctx = avctx;
    avctx->pix_fmt = AV_PIX_FMT_PAL8;

+    if (!avctx->width || !avctx->height ||
+        (avctx->width & 1) || (avctx->height & 1)) {
+        av_log(avctx, AV_LOG_ERROR, "Invalid video dimensions: %dx%d\n",
+               avctx->width, avctx->height);
+        return AVERROR(EINVAL);
+    }

    ret = ff_set_dimensions(avctx, 256, 128);
    if (ret < 0)
        return ret;

    seq->frame = av_frame_alloc();
    if (!seq->frame)
        return AVERROR(ENOMEM);

    return 0;
}

libavcodec/qtrle.c: qtrle_decode_init

static av_cold int qtrle_decode_init(AVCodecContext *avctx)
{
    QtrleContext *s = avctx->priv_data;

    s->avctx = avctx;
    switch (avctx->bits_per_coded_sample) {
    case 1:
    case 2:
    case 4:
    case 8:
    case 33:
    case 34:
    case 36:
    case 40:
        avctx->pix_fmt = AV_PIX_FMT_PAL8;
        break;

    case 16:
        avctx->pix_fmt = AV_PIX_FMT_RGB555;
        break;

    case 24:
        avctx->pix_fmt = AV_PIX_FMT_RGB24;
        break;

    case 32:
        avctx->pix_fmt = AV_PIX_FMT_RGB32;
        break;

    default:
        av_log (avctx, AV_LOG_ERROR, "Unsupported colorspace: %d bits/sample?\n",
            avctx->bits_per_coded_sample);
        return AVERROR_INVALIDDATA;
    }

+    if (!avctx->width || !avctx->height ||
+        (avctx->width & 1) || (avctx->height & 1)) {
+        av_log(avctx, AV_LOG_ERROR, "Invalid video dimensions: %dx%d\n",
+               avctx->width, avctx->height);
+        return AVERROR(EINVAL);
+    }


    s->frame = av_frame_alloc();
    if (!s->frame)
        return AVERROR(ENOMEM);

    return 0;
}

libavcodec/cinepak.c: cinepak_decode_init

static av_cold int cinepak_decode_init(AVCodecContext *avctx)
{
    CinepakContext *s = avctx->priv_data;

    s->avctx = avctx;
    s->width = (avctx->width + 3) & ~3;
    s->height = (avctx->height + 3) & ~3;

    s->sega_film_skip_bytes = -1;  /* uninitialized state */

    // check for paletted data
    if (avctx->bits_per_coded_sample != 8) {
        s->palette_video = 0;
        avctx->pix_fmt = AV_PIX_FMT_RGB24;
    } else {
        s->palette_video = 1;
        avctx->pix_fmt = AV_PIX_FMT_PAL8;
    }

+    if (!avctx->width || !avctx->height ||
+        (avctx->width & 1) || (avctx->height & 1)) {
+        av_log(avctx, AV_LOG_ERROR, "Invalid video dimensions: %dx%d\n",
+               avctx->width, avctx->height);
+        return AVERROR(EINVAL);
+    }


    s->frame = av_frame_alloc();
    if (!s->frame)
        return AVERROR(ENOMEM);

    return 0;
}

libavcodec/msrle.c: msrle_decode_init

static av_cold int msrle_decode_init(AVCodecContext *avctx)
{
    MsrleContext *s = avctx->priv_data;
    int i;

    s->avctx = avctx;

    switch (avctx->bits_per_coded_sample) {
    case 1:
        avctx->pix_fmt = AV_PIX_FMT_MONOWHITE;
        break;
    case 4:
    case 8:
        avctx->pix_fmt = AV_PIX_FMT_PAL8;
        break;
    case 24:
        avctx->pix_fmt = AV_PIX_FMT_BGR24;
        break;
    default:
        av_log(avctx, AV_LOG_ERROR, "unsupported bits per sample\n");
        return AVERROR_INVALIDDATA;
    }

+    if (!avctx->width || !avctx->height ||
+        (avctx->width & 1) || (avctx->height & 1)) {
+        av_log(avctx, AV_LOG_ERROR, "Invalid video dimensions: %dx%d\n",
+               avctx->width, avctx->height);
+        return AVERROR(EINVAL);
+    }


    s->frame = av_frame_alloc();
    if (!s->frame)
        return AVERROR(ENOMEM);

    if (avctx->extradata_size >= 4)
        for (i = 0; i < FFMIN(avctx->extradata_size, AVPALETTE_SIZE)/4; i++)
            s->pal[i] = 0xFFU<<24 | AV_RL32(avctx->extradata+4*i);

    return 0;
}

libavcodec/dsicinvideo.c: cinvideo_decode_init

static av_cold int cinvideo_decode_init(AVCodecContext *avctx)
{
    CinVideoContext *cin = avctx->priv_data;

    cin->avctx = avctx;
    avctx->pix_fmt = AV_PIX_FMT_PAL8;

+    if (!avctx->width || !avctx->height ||
+        (avctx->width & 1) || (avctx->height & 1)) {
+        av_log(avctx, AV_LOG_ERROR, "Invalid video dimensions: %dx%d\n",
+               avctx->width, avctx->height);
+        return AVERROR(EINVAL);
+    }


    cin->frame = av_frame_alloc();
    if (!cin->frame)
        return AVERROR(ENOMEM);

    cin->bitmap_size = avctx->width * avctx->height;
    if (allocate_buffers(cin))
        return AVERROR(ENOMEM);

    return 0;
}

libavcodec/msvideo1.c: msvideo1_decode_init

static av_cold int msvideo1_decode_init(AVCodecContext *avctx)
{
    Msvideo1Context *s = avctx->priv_data;

    s->avctx = avctx;

    /* figure out the colorspace based on the presence of a palette */
    if (s->avctx->bits_per_coded_sample == 8) {
        s->mode_8bit = 1;
        avctx->pix_fmt = AV_PIX_FMT_PAL8;
        if (avctx->extradata_size >= AVPALETTE_SIZE)
            memcpy(s->pal, avctx->extradata, AVPALETTE_SIZE);
    } else {
        s->mode_8bit = 0;
        avctx->pix_fmt = AV_PIX_FMT_RGB555;
    }

+    if (!avctx->width || !avctx->height ||
+        (avctx->width & 1) || (avctx->height & 1)) {
+        av_log(avctx, AV_LOG_ERROR, "Invalid video dimensions: %dx%d\n",
+               avctx->width, avctx->height);
+        return AVERROR(EINVAL);
+    }


    s->frame = av_frame_alloc();
    if (!s->frame)
        return AVERROR(ENOMEM);

    return 0;
}

Attachments (1)

patch_ffmpeg.tar.gz (1.6 KB ) - added by Yooooooha 6 years ago.
proposal patch for several files

Download all attachments as: .zip

Change History (4)

comment:1 by Carl Eugen Hoyos, 6 years ago

Keywords: crash added; out-of-bounds removed

Patches and suggestions for patches are unfortunately ignored here.
Please either send your patches - made with git format-patch - to the development mailing list or provide input files that trigger the issues you are describing.

by Yooooooha, 6 years ago

Attachment: patch_ffmpeg.tar.gz added

proposal patch for several files

comment:2 by Carl Eugen Hoyos, 6 years ago

When you send your patches to the FFmpeg development mailing list, please do not forget to use a non-default name for the author field.

comment:3 by Carl Eugen Hoyos, 6 years ago

Resolution: needs_more_info
Status: newclosed

It appears that several of the provided patches break legitimate input files. Please reopen this ticket if you can provide samples that allow to reproduce the vulnerabilities that you tried to fix.

Note: See TracTickets for help on using tickets.