Message ID | 1539612240-27031-2-git-send-email-mypopydev@gmail.com |
---|---|
State | Accepted |
Headers | show |
On Mon, Oct 15, 2018 at 10:03:59PM +0800, Jun Zhao wrote: > case 1: > use the hexdump -C SMM0005.rcv get: > size skip (size - 4) > | | > V V > 00000000 18 00 00 c5 05 00 00 00 4d f1 0a 11 00 e0 01 00 > 00000010 00 d0 02 00 00 0c 00 00 00 88 13 00 00 c0 65 52 > ^ > | > size + 16 > case 2: > same the command for SMM0015.rcv get: > size > | > V > 00000000 19 00 00 c5 04 00 00 00 41 f3 80 01 40 02 00 00 > 00000010 d0 02 00 00 0c 00 00 00 00 00 00 10 00 00 00 00 > ^ > | > size + 16 > > There are different the RCV file format for VC-1, vc1test > just handle the case 2 now, this fix will support the case 1. > (Both of test clips come from: SMPTE Recommended Practice - > VC-1 Decoder and Bitstream Conformance). And I think I got > a older VC-1 test clip in the case 1. > > Reviewed-by: Carl Eugen Hoyos <ceffmpeg@gmail.com> > Reviewed-by: Jerome Borsboom <jerome.borsboom@carpalis.nl> > Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > Signed-off-by: Yan, FengX <fengx.yan@intel.com> > --- > libavformat/vc1test.c | 12 ++++++++++-- > 1 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c > index a801f4b..2427660 100644 > --- a/libavformat/vc1test.c > +++ b/libavformat/vc1test.c > @@ -34,9 +34,14 @@ > > static int vc1t_probe(AVProbeData *p) > { > + int size; > + > if (p->buf_size < 24) > return 0; > - if (p->buf[3] != 0xC5 || AV_RL32(&p->buf[4]) != 4 || AV_RL32(&p->buf[20]) != 0xC) > + > + size = AV_RL32(&p->buf[4]); > + if (p->buf[3] != 0xC5 || size < 4 || size+16 > p->buf_size || size + 16 is undefined here as it can overflow the int range [...]
On Wed, Oct 17, 2018 at 7:23 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Mon, Oct 15, 2018 at 10:03:59PM +0800, Jun Zhao wrote: > > case 1: > > use the hexdump -C SMM0005.rcv get: > > size skip (size - 4) > > | | > > V V > > 00000000 18 00 00 c5 05 00 00 00 4d f1 0a 11 00 e0 01 00 > > 00000010 00 d0 02 00 00 0c 00 00 00 88 13 00 00 c0 65 52 > > ^ > > | > > size + 16 > > case 2: > > same the command for SMM0015.rcv get: > > size > > | > > V > > 00000000 19 00 00 c5 04 00 00 00 41 f3 80 01 40 02 00 00 > > 00000010 d0 02 00 00 0c 00 00 00 00 00 00 10 00 00 00 00 > > ^ > > | > > size + 16 > > > > There are different the RCV file format for VC-1, vc1test > > just handle the case 2 now, this fix will support the case 1. > > (Both of test clips come from: SMPTE Recommended Practice - > > VC-1 Decoder and Bitstream Conformance). And I think I got > > a older VC-1 test clip in the case 1. > > > > Reviewed-by: Carl Eugen Hoyos <ceffmpeg@gmail.com> > > Reviewed-by: Jerome Borsboom <jerome.borsboom@carpalis.nl> > > Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> > > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > > Signed-off-by: Yan, FengX <fengx.yan@intel.com> > > --- > > libavformat/vc1test.c | 12 ++++++++++-- > > 1 files changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c > > index a801f4b..2427660 100644 > > --- a/libavformat/vc1test.c > > +++ b/libavformat/vc1test.c > > @@ -34,9 +34,14 @@ > > > > static int vc1t_probe(AVProbeData *p) > > { > > + int size; > > + > > if (p->buf_size < 24) > > return 0; > > - if (p->buf[3] != 0xC5 || AV_RL32(&p->buf[4]) != 4 || AV_RL32(&p->buf[20]) != 0xC) > > + > > + size = AV_RL32(&p->buf[4]); > > + if (p->buf[3] != 0xC5 || size < 4 || size+16 > p->buf_size || > > size + 16 is undefined here as it can overflow the int range > Yes, this is a offset-by-one issue and lead to overflow(right way is size + 15 > p->buf_size), in fact, I have found this issue after submitted this version patch, and fixed in local repo, so if don't have other comments, will apply with this offset-by-one fix. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > If you drop bombs on a foreign country and kill a hundred thousand > innocent people, expect your government to call the consequence > "unprovoked inhuman terrorist attacks" and use it to justify dropping > more bombs and killing more people. The technology changed, the idea is old. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Wed, Oct 17, 2018 at 7:23 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Mon, Oct 15, 2018 at 10:03:59PM +0800, Jun Zhao wrote: > > case 1: > > use the hexdump -C SMM0005.rcv get: > > size skip (size - 4) > > | | > > V V > > 00000000 18 00 00 c5 05 00 00 00 4d f1 0a 11 00 e0 01 00 > > 00000010 00 d0 02 00 00 0c 00 00 00 88 13 00 00 c0 65 52 > > ^ > > | > > size + 16 > > case 2: > > same the command for SMM0015.rcv get: > > size > > | > > V > > 00000000 19 00 00 c5 04 00 00 00 41 f3 80 01 40 02 00 00 > > 00000010 d0 02 00 00 0c 00 00 00 00 00 00 10 00 00 00 00 > > ^ > > | > > size + 16 > > > > There are different the RCV file format for VC-1, vc1test > > just handle the case 2 now, this fix will support the case 1. > > (Both of test clips come from: SMPTE Recommended Practice - > > VC-1 Decoder and Bitstream Conformance). And I think I got > > a older VC-1 test clip in the case 1. > > > > Reviewed-by: Carl Eugen Hoyos <ceffmpeg@gmail.com> > > Reviewed-by: Jerome Borsboom <jerome.borsboom@carpalis.nl> > > Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> > > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > > Signed-off-by: Yan, FengX <fengx.yan@intel.com> > > --- > > libavformat/vc1test.c | 12 ++++++++++-- > > 1 files changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c > > index a801f4b..2427660 100644 > > --- a/libavformat/vc1test.c > > +++ b/libavformat/vc1test.c > > @@ -34,9 +34,14 @@ > > > > static int vc1t_probe(AVProbeData *p) > > { > > + int size; > > + > > if (p->buf_size < 24) > > return 0; > > - if (p->buf[3] != 0xC5 || AV_RL32(&p->buf[4]) != 4 || AV_RL32(&p->buf[20]) != 0xC) > > + > > + size = AV_RL32(&p->buf[4]); > > + if (p->buf[3] != 0xC5 || size < 4 || size+16 > p->buf_size || > > size + 16 is undefined here as it can overflow the int range > And will check the int overflow issue. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > If you drop bombs on a foreign country and kill a hundred thousand > innocent people, expect your government to call the consequence > "unprovoked inhuman terrorist attacks" and use it to justify dropping > more bombs and killing more people. The technology changed, the idea is old. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Wed, Oct 17, 2018 at 7:23 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Mon, Oct 15, 2018 at 10:03:59PM +0800, Jun Zhao wrote: > > case 1: > > use the hexdump -C SMM0005.rcv get: > > size skip (size - 4) > > | | > > V V > > 00000000 18 00 00 c5 05 00 00 00 4d f1 0a 11 00 e0 01 00 > > 00000010 00 d0 02 00 00 0c 00 00 00 88 13 00 00 c0 65 52 > > ^ > > | > > size + 16 > > case 2: > > same the command for SMM0015.rcv get: > > size > > | > > V > > 00000000 19 00 00 c5 04 00 00 00 41 f3 80 01 40 02 00 00 > > 00000010 d0 02 00 00 0c 00 00 00 00 00 00 10 00 00 00 00 > > ^ > > | > > size + 16 > > > > There are different the RCV file format for VC-1, vc1test > > just handle the case 2 now, this fix will support the case 1. > > (Both of test clips come from: SMPTE Recommended Practice - > > VC-1 Decoder and Bitstream Conformance). And I think I got > > a older VC-1 test clip in the case 1. > > > > Reviewed-by: Carl Eugen Hoyos <ceffmpeg@gmail.com> > > Reviewed-by: Jerome Borsboom <jerome.borsboom@carpalis.nl> > > Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> > > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > > Signed-off-by: Yan, FengX <fengx.yan@intel.com> > > --- > > libavformat/vc1test.c | 12 ++++++++++-- > > 1 files changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c > > index a801f4b..2427660 100644 > > --- a/libavformat/vc1test.c > > +++ b/libavformat/vc1test.c > > @@ -34,9 +34,14 @@ > > > > static int vc1t_probe(AVProbeData *p) > > { > > + int size; > > + > > if (p->buf_size < 24) > > return 0; > > - if (p->buf[3] != 0xC5 || AV_RL32(&p->buf[4]) != 4 || AV_RL32(&p->buf[20]) != 0xC) > > + > > + size = AV_RL32(&p->buf[4]); > > + if (p->buf[3] != 0xC5 || size < 4 || size+16 > p->buf_size || > > size + 16 is undefined here as it can overflow the int range > > Update V5 patch
diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c index a801f4b..2427660 100644 --- a/libavformat/vc1test.c +++ b/libavformat/vc1test.c @@ -34,9 +34,14 @@ static int vc1t_probe(AVProbeData *p) { + int size; + if (p->buf_size < 24) return 0; - if (p->buf[3] != 0xC5 || AV_RL32(&p->buf[4]) != 4 || AV_RL32(&p->buf[20]) != 0xC) + + size = AV_RL32(&p->buf[4]); + if (p->buf[3] != 0xC5 || size < 4 || size+16 > p->buf_size || + AV_RL32(&p->buf[size+16]) != 0xC) return 0; return AVPROBE_SCORE_EXTENSION; @@ -48,9 +53,10 @@ static int vc1t_read_header(AVFormatContext *s) AVStream *st; int frames; uint32_t fps; + int size; frames = avio_rl24(pb); - if(avio_r8(pb) != 0xC5 || avio_rl32(pb) != 4) + if (avio_r8(pb) != 0xC5 || ((size = avio_rl32(pb)) < 4)) return AVERROR_INVALIDDATA; /* init video codec */ @@ -63,6 +69,8 @@ static int vc1t_read_header(AVFormatContext *s) if (ff_get_extradata(s, st->codecpar, pb, VC1_EXTRADATA_SIZE) < 0) return AVERROR(ENOMEM); + + avio_skip(pb, size - 4); st->codecpar->height = avio_rl32(pb); st->codecpar->width = avio_rl32(pb); if(avio_rl32(pb) != 0xC)