[FFmpeg-devel] [PATCH 4/4] Add MxPEG decoder

Michael Niedermayer michaelni at gmx.at
Thu Mar 31 15:22:08 CEST 2011


On Mon, Mar 28, 2011 at 12:03:58AM +0400, Anatoly Nenashev wrote:
[...]
> +static int mxpeg_decode_mxm(MXpegDecodeContext *s, const uint8_t *buf_ptr, int buf_size)
> +{
> +    unsigned bitmask_size, mb_count;
> +    int i;
> +
> +    s->mb_width  = AV_RL16(buf_ptr+4);
> +    s->mb_height = AV_RL16(buf_ptr+6);
> +    mb_count = s->mb_width * s->mb_height;
> +
> +    bitmask_size = (mb_count + 7) >> 3;
> +    if (bitmask_size > buf_size - 12) {
> +        av_log(s->jpg.avctx, AV_LOG_ERROR, "MXM bitmask is not complete\n");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    if (s->bitmask_size != bitmask_size) {
> +        av_freep(&s->mxm_bitmask);
> +        s->mxm_bitmask = av_malloc(bitmask_size);
> +        if (!s->mxm_bitmask) {
> +            av_log(s->jpg.avctx, AV_LOG_ERROR, "MXM bitmask memory allocation error\n");
> +            return AVERROR(ENOMEM);
> +        }
> +
> +        av_freep(&s->completion_bitmask);
> +        s->completion_bitmask = av_mallocz(bitmask_size);
> +        if (!s->completion_bitmask) {
> +            av_log(s->jpg.avctx, AV_LOG_ERROR, "Completion bitmask memory allocation error\n");
> +            return AVERROR(ENOMEM);
> +        }
> +
> +        s->bitmask_size = bitmask_size;
> +    }
> +
> +    memcpy(s->mxm_bitmask, buf_ptr + 12, bitmask_size);
> +    s->got_mxm_bitmask = 1;

This is another security issue.

Lets assume on the first frame this passes and both arrays are allocated
with 1000
2nd frame decreases bitmask_size to 500 and lets assume the 2nd
malloc() fails and the decoding of frame 2 fails as a result.
At this point s->bitmask_size is 1000 and the first array is 500,
the second NULL
3rd frame we assume that mb_width/height have returned to the first
frame values so s->bitmask_size == bitmask_size
and the memcpy now copies 500 bytes of user choosen data over the
end of the first array

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- 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/20110331/1fceda03/attachment.asc>


More information about the ffmpeg-devel mailing list