On Mon, Feb 15, 2016 at 11:27:20AM -0800, Mark Harris wrote: > On Mon, Feb 15, 2016 at 11:02 AM, Michael Niedermayer > <mich...@niedermayer.cc> wrote: > > On Mon, Feb 15, 2016 at 09:57:51AM -0800, Mark Harris wrote: > >> Avoid invalid memory read/crash when ico offset >= 0xfffffff8. > >> Base64-encoded example: AAABADAwMDAwMAAAMAAwMDAw/P///w== > >> --- > >> libavformat/icodec.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/libavformat/icodec.c b/libavformat/icodec.c > >> index 6ddb901..8f84337 100644 > >> --- a/libavformat/icodec.c > >> +++ b/libavformat/icodec.c > >> @@ -60,7 +60,7 @@ static int probe(AVProbeData *p) > >> offset = AV_RL32(p->buf + 18 + i * 16); > >> if (offset < 22) > >> return FFMIN(i, AVPROBE_SCORE_MAX / 4); > >> - if (offset + 8 > p->buf_size) > >> + if (offset > p->buf_size - 8) > > > > buf_size - 8 can underflow or more precissely is not guranteed to be > > representable as unsigned while the compare is using unsigned > > > > If p->buf_size was less than 8, would it not have returned before > this? AV_RL32(p->buf + 14) would be 0 and offset = AV_RL32(p->buf + > 18) would be 0, due to the zero padding of the probe buffer.
you are correct, i didnt think about that but I would suggest to make the check work independant of that, who knows, the code could be factored out or the array might not be zero padded for whatever reason. If the code depends on such assumtations that should be clearly documented so one changing the zero padding would know that this needs updating. But its easier to use 8LL instead of 8, which should avoid the problem or to make offset uint64_t [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In a rich man's house there is no place to spit but his face. -- Diogenes of Sinope
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
