diff mbox series

[FFmpeg-devel,v1,01/10] return value check for init_get_bits in wmv2dec.c

Message ID PAXP193MB1262B8246FB49C8DA844B949B6F99@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

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Maryam Ebrahimzadeh Aug. 12, 2021, 4:48 a.m. UTC
As the second argument for init_get_bits can be crafted, a return value check for this function call is necessary  so replace init_get_bits with init_get_bits8.

---
 libavcodec/wmv2dec.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Hendrik Leppkes Aug. 12, 2021, 5:53 a.m. UTC | #1
On Thu, Aug 12, 2021 at 6:48 AM maryam ebrahimzadeh <me22bee@outlook.com> wrote:
>
> As the second argument for init_get_bits can be crafted, a return value check for this function call is necessary  so replace init_get_bits with init_get_bits8.
>
> ---
>  libavcodec/wmv2dec.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/wmv2dec.c b/libavcodec/wmv2dec.c
> index c500e3e779..73da73c02c 100644
> --- a/libavcodec/wmv2dec.c
> +++ b/libavcodec/wmv2dec.c
> @@ -101,12 +101,14 @@ static int decode_ext_header(Wmv2Context *w)
>      GetBitContext gb;
>      int fps;
>      int code;
> +    int ret;
>
>      if (s->avctx->extradata_size < 4)
>          return AVERROR_INVALIDDATA;
>
> -    init_get_bits(&gb, s->avctx->extradata, 32);
> -
> +    ret = init_get_bits8(&gb, s->avctx->extradata, 4);
> +    if (ret < 0)
> +        return ret;

This is a fixed size, the buffer size is checked right above, what
exactly would the error condition be here?

- Hendrik
Maryam Ebrahimzadeh Aug. 12, 2021, 6 a.m. UTC | #2
On Aug 12, 2021, at 10:23 AM, Hendrik Leppkes <h.leppkes@gmail.com<mailto:h.leppkes@gmail.com>> wrote:

On Thu, Aug 12, 2021 at 6:48 AM maryam ebrahimzadeh <me22bee@outlook.com<mailto:me22bee@outlook.com>> wrote:

As the second argument for init_get_bits can be crafted, a return value check for this function call is necessary  so replace init_get_bits with init_get_bits8.

---
libavcodec/wmv2dec.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavcodec/wmv2dec.c b/libavcodec/wmv2dec.c
index c500e3e779..73da73c02c 100644
--- a/libavcodec/wmv2dec.c
+++ b/libavcodec/wmv2dec.c
@@ -101,12 +101,14 @@ static int decode_ext_header(Wmv2Context *w)
    GetBitContext gb;
    int fps;
    int code;
+    int ret;

    if (s->avctx->extradata_size < 4)
        return AVERROR_INVALIDDATA;

-    init_get_bits(&gb, s->avctx->extradata, 32);
-
+    ret = init_get_bits8(&gb, s->avctx->extradata, 4);
+    if (ret < 0)
+        return ret;

This is a fixed size, the buffer size is checked right above, what
exactly would the error condition be here?

Init_get_bits8 checks for overflow and some other conditions (in init_get_bits_xe ) too.


- Hendrik
Hendrik Leppkes Aug. 12, 2021, 6:05 a.m. UTC | #3
On Thu, Aug 12, 2021 at 8:01 AM Maryam Ebrahimzadeh <me22bee@outlook.com> wrote:
>
> Init_get_bits8 checks for overflow and some other conditions (in init_get_bits_xe ) too.
>

What overflow? I'm not sure you understand the concept here. 32 can't
overflow, neither can 8*4.

Please also make sure to quote and respond properly, as it makes it
hard to read otherwise.

- Hendrik
diff mbox series

Patch

diff --git a/libavcodec/wmv2dec.c b/libavcodec/wmv2dec.c
index c500e3e779..73da73c02c 100644
--- a/libavcodec/wmv2dec.c
+++ b/libavcodec/wmv2dec.c
@@ -101,12 +101,14 @@  static int decode_ext_header(Wmv2Context *w)
     GetBitContext gb;
     int fps;
     int code;
+    int ret;
 
     if (s->avctx->extradata_size < 4)
         return AVERROR_INVALIDDATA;
 
-    init_get_bits(&gb, s->avctx->extradata, 32);
-
+    ret = init_get_bits8(&gb, s->avctx->extradata, 4);
+    if (ret < 0)
+        return ret;
     fps                 = get_bits(&gb, 5);
     s->bit_rate         = get_bits(&gb, 11) * 1024;
     w->mspel_bit        = get_bits1(&gb);