| Message ID | PAXP193MB1262B312824C9BBFBDFD4CE3B6F99@PAXP193MB1262.EURP193.PROD.OUTLOOK.COM |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v1,01/10] return value check for init_get_bits in wmv2dec.c | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | fail | Make failed |
| andriy/PPC64_make | warning | Make failed |
On Thu, Aug 12, 2021 at 6:53 AM maryam ebrahimzadeh <me22bee@outlook.com> wrote: > > --- > libavcodec/vc1dec.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c > index 1fb1950ade..07d60294f2 100644 > --- a/libavcodec/vc1dec.c > +++ b/libavcodec/vc1dec.c > @@ -444,7 +444,9 @@ static av_cold int vc1_decode_init(AVCodecContext *avctx) > // the last byte of the extradata is a version number, 1 for the > // samples we can decode > > - init_get_bits(&gb, avctx->extradata, avctx->extradata_size*8); > + ret = init_get_bits8(&gb, avctx->extradata, avctx->extradata_size); > + if (ret < 0) > + return ret; > > if ((ret = ff_vc1_decode_sequence_header(avctx, v, &gb)) < 0) > return ret; > @@ -771,7 +773,9 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data, > } > init_get_bits(&s->gb, buf2, buf_size2*8); > } else > - init_get_bits(&s->gb, buf, buf_size*8); > + ret = init_get_bits8(&s->gb, buf, buf_size); > + if (ret < 0) > + return ret; > > if (v->res_sprite) { > v->new_sprite = !get_bits1(&s->gb); There is a whole bunch of other cases in vc1dec.c, I can even see one in the patch context there. Any reason you picked only these two to change? - Hendrik
I choose them because their second argument can be crafted.
On Aug 12, 2021, at 10:29 AM, Hendrik Leppkes <h.leppkes@gmail.com<mailto:h.leppkes@gmail.com>> wrote:
On Thu, Aug 12, 2021 at 6:53 AM maryam ebrahimzadeh <me22bee@outlook.com<mailto:me22bee@outlook.com>> wrote:
---
libavcodec/vc1dec.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index 1fb1950ade..07d60294f2 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -444,7 +444,9 @@ static av_cold int vc1_decode_init(AVCodecContext *avctx)
// the last byte of the extradata is a version number, 1 for the
// samples we can decode
- init_get_bits(&gb, avctx->extradata, avctx->extradata_size*8);
+ ret = init_get_bits8(&gb, avctx->extradata, avctx->extradata_size);
+ if (ret < 0)
+ return ret;
if ((ret = ff_vc1_decode_sequence_header(avctx, v, &gb)) < 0)
return ret;
@@ -771,7 +773,9 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data,
}
init_get_bits(&s->gb, buf2, buf_size2*8);
} else
- init_get_bits(&s->gb, buf, buf_size*8);
+ ret = init_get_bits8(&s->gb, buf, buf_size);
+ if (ret < 0)
+ return ret;
if (v->res_sprite) {
v->new_sprite = !get_bits1(&s->gb);
There is a whole bunch of other cases in vc1dec.c, I can even see one
in the patch context there. Any reason you picked only these two to
change?
- Hendrik
On Thu, Aug 12, 2021 at 12:53:31AM -0400, maryam ebrahimzadeh wrote: > --- > libavcodec/vc1dec.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) This breaks fate-vc1_sa20021 --- ./tests/ref/fate/vc1_sa20021 2021-08-06 11:56:29.601679428 +0200 +++ tests/data/fate/vc1_sa20021 2021-08-12 16:01:47.610015840 +0200 @@ -8,58 +8,30 @@ 0, 3, 3, 1, 506880, 0x195cbee1 0, 4, 4, 1, 506880, 0xc8141e28 0, 5, 5, 1, 506880, 0xb170c49b -0, 6, 6, 1, 506880, 0x2782268a -0, 7, 7, 1, 506880, 0x2782268a -0, 8, 8, 1, 506880, 0x2782268a 0, 9, 9, 1, 506880, 0x2782268a -0, 10, 10, 1, 506880, 0xe6803b32 0, 11, 11, 1, 506880, 0xe6803b32 -0, 12, 12, 1, 506880, 0xa5ef9baf 0, 13, 13, 1, 506880, 0xa5ef9baf 0, 14, 14, 1, 506880, 0x46e8cbcb 0, 15, 15, 1, 506880, 0x28a2239b -0, 16, 16, 1, 506880, 0x7667af2f 0, 17, 17, 1, 506880, 0x7667af2f 0, 18, 18, 1, 506880, 0x8011bcaf [...]
On 8/12/2021 1:53 AM, maryam ebrahimzadeh wrote: > --- > libavcodec/vc1dec.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c > index 1fb1950ade..07d60294f2 100644 > --- a/libavcodec/vc1dec.c > +++ b/libavcodec/vc1dec.c > @@ -444,7 +444,9 @@ static av_cold int vc1_decode_init(AVCodecContext *avctx) > // the last byte of the extradata is a version number, 1 for the > // samples we can decode > > - init_get_bits(&gb, avctx->extradata, avctx->extradata_size*8); > + ret = init_get_bits8(&gb, avctx->extradata, avctx->extradata_size); > + if (ret < 0) > + return ret; > > if ((ret = ff_vc1_decode_sequence_header(avctx, v, &gb)) < 0) > return ret; > @@ -771,7 +773,9 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data, > } > init_get_bits(&s->gb, buf2, buf_size2*8); > } else > - init_get_bits(&s->gb, buf, buf_size*8); > + ret = init_get_bits8(&s->gb, buf, buf_size); > + if (ret < 0) > + return ret; The check is not covered by the else. > > if (v->res_sprite) { > v->new_sprite = !get_bits1(&s->gb); >
diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c index 1fb1950ade..07d60294f2 100644 --- a/libavcodec/vc1dec.c +++ b/libavcodec/vc1dec.c @@ -444,7 +444,9 @@ static av_cold int vc1_decode_init(AVCodecContext *avctx) // the last byte of the extradata is a version number, 1 for the // samples we can decode - init_get_bits(&gb, avctx->extradata, avctx->extradata_size*8); + ret = init_get_bits8(&gb, avctx->extradata, avctx->extradata_size); + if (ret < 0) + return ret; if ((ret = ff_vc1_decode_sequence_header(avctx, v, &gb)) < 0) return ret; @@ -771,7 +773,9 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data, } init_get_bits(&s->gb, buf2, buf_size2*8); } else - init_get_bits(&s->gb, buf, buf_size*8); + ret = init_get_bits8(&s->gb, buf, buf_size); + if (ret < 0) + return ret; if (v->res_sprite) { v->new_sprite = !get_bits1(&s->gb);