[FFmpeg-devel] oggparsevorbis.c vorbis_comment: check for negative size
Michael Niedermayer
michaelni
Mon Oct 8 04:10:45 CEST 2007
Hi
On Sun, Oct 07, 2007 at 11:33:20AM -0400, Rich Felker wrote:
> On Sun, Oct 07, 2007 at 02:38:10PM +0200, matthieu castet wrote:
> > Attila Kinali wrote:
> > > On Sun, 7 Oct 2007 12:42:13 +0200
> > > Attila Kinali <attila at kinali.ch> wrote:
> > >
> > >
> > >> The segfault occures, because s is read from the file but only
> > >> checked to be smaller than the limit, but not whether it is
> > >> positive, resulting in an overflow when it is a big negative number.
> > >>
> > >> Patch attached
> > >
> > > Updated patch. Missed another occurence of the same problem.
> > Why doesn't you make s unsigned ?
>
> It won't solve the overflow issue. However checking to make sure s is
> not negative is just a hack to work around the problem of not writing
> overflow-safe unsigned arithmetic.
yes i agree
_maybe_ the following would fix it (ive not checked too carefull)
@@ -somewhere +somewhere @@
char *p = buf;
- int s, n, j;
+ int n, j;
+ unsigned int s;
- if (size < 4)
+ if (size < 8)
return -1;
s = AV_RL32(p);
p += 4;
size -= 4;
- if (size < s + 4)
+ if (size - 4 < s)
return -1;
p += s;
size -= s;
[...]
--
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: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071008/e61ec3c8/attachment.pgp>
More information about the ffmpeg-devel
mailing list