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

Ronald S. Bultje rsbultje
Mon Feb 7 14:05:32 CET 2011


Hi,

On Mon, Feb 7, 2011 at 3:09 AM, Kostya <kostya.shishkov at gmail.com> wrote:
> On Sun, Feb 06, 2011 at 11:57:39AM +0100, Reimar D?ffinger wrote:
>> On Sun, Feb 06, 2011 at 11:19:53AM +0100, Kostya wrote:
>> > On Sat, Feb 05, 2011 at 11:03:38PM -0500, Ronald S. Bultje wrote:
>> > > Hi,
>> > >
>> > > On Fri, Feb 4, 2011 at 5:47 PM, Kostya <kostya.shishkov at gmail.com> wrote:
>> > > > +static int xan_decode_chroma(AVCodecContext *avctx, AVPacket *avpkt)
>> > > [..]
>> > > > + ? ?src ? ?= avpkt->data + 4 + chroma_off;
>> > > > + ? ?table ?= src + 2;
>> > > > + ? ?mode ? = bytestream_get_le16(&src);
>> > > > + ? ?offset = bytestream_get_le16(&src) * 2;
>> > > > +
>> > > > + ? ?if (src + offset >= avpkt->data + avpkt->size) {
>> > >
>> > > This can still overflow, instead use src - avpkt->data >= avpkt->size
>> > > - offset. Rest looks OK.
>> >
>> > Chroma offset is checked earlier so the only way for it to overflow is when
>> > data_size ~= 2^32 - 2^17 (offset is 16 bit shifted by one).
>>
>> src + offset can overflow.
>> I think we assumed that chroma_off is already validated, I don't think
>> it would be possible to validate both in a single if.
>
> Here it is with change

Thanks, looks great. Queued. Can you (or someone) add a sample to
FATE? I'll look at patch #2 shortly.

Ronald



More information about the ffmpeg-devel mailing list