diff mbox

[FFmpeg-devel,3/5] avcodec/smacker: cleanup on errors in smka_decode_frame()

Message ID 20191010224011.5364-3-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer Oct. 10, 2019, 10:40 p.m. UTC 
Fixes: multiple memleaks
Fixes: 17660/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SMACKAUD_fuzzer-5689769928949760
Fixes: 18064/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SMACKAUD_fuzzer-5631086809317376

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/smacker.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

Comments

James Almer Oct. 13, 2019, 3:20 a.m. UTC | #1 
On 10/10/2019 7:40 PM, Michael Niedermayer wrote:
> Fixes: multiple memleaks
> Fixes: 17660/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SMACKAUD_fuzzer-5689769928949760
> Fixes: 18064/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SMACKAUD_fuzzer-5631086809317376
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/smacker.c | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/smacker.c b/libavcodec/smacker.c
> index a2950c455b..901cdb1fb1 100644
> --- a/libavcodec/smacker.c
> +++ b/libavcodec/smacker.c
> @@ -721,8 +721,10 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data,
>          for(i = 0; i <= stereo; i++)
>              *samples++ = pred[i];
>          for(; i < unp_size / 2; i++) {
> -            if(get_bits_left(&gb)<0)
> -                return AVERROR_INVALIDDATA;
> +            if(get_bits_left(&gb)<0) {

Take the opportunity to add white spaces here and the other changed case
below.

> +                ret = AVERROR_INVALIDDATA;
> +                goto error;
> +            }
>              if(i & stereo) {
>                  if(vlc[2].table)
>                      res = get_vlc2(&gb, vlc[2].table, SMKTREE_BITS, 3);
> @@ -730,7 +732,8 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data,
>                      res = 0;
>                  if (res < 0) {
>                      av_log(avctx, AV_LOG_ERROR, "invalid vlc\n");
> -                    return AVERROR_INVALIDDATA;
> +                    ret = AVERROR_INVALIDDATA;
> +                    goto error;
>                  }
>                  val  = h[2].values[res];
>                  if(vlc[3].table)
> @@ -739,7 +742,8 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data,
>                      res = 0;
>                  if (res < 0) {
>                      av_log(avctx, AV_LOG_ERROR, "invalid vlc\n");
> -                    return AVERROR_INVALIDDATA;
> +                    ret = AVERROR_INVALIDDATA;
> +                    goto error;
>                  }
>                  val |= h[3].values[res] << 8;
>                  pred[1] += sign_extend(val, 16);
> @@ -751,7 +755,8 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data,
>                      res = 0;
>                  if (res < 0) {
>                      av_log(avctx, AV_LOG_ERROR, "invalid vlc\n");
> -                    return AVERROR_INVALIDDATA;
> +                    ret = AVERROR_INVALIDDATA;
> +                    goto error;
>                  }
>                  val  = h[0].values[res];
>                  if(vlc[1].table)
> @@ -760,7 +765,8 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data,
>                      res = 0;
>                  if (res < 0) {
>                      av_log(avctx, AV_LOG_ERROR, "invalid vlc\n");
> -                    return AVERROR_INVALIDDATA;
> +                    ret = AVERROR_INVALIDDATA;
> +                    goto error;
>                  }
>                  val |= h[1].values[res] << 8;
>                  pred[0] += sign_extend(val, 16);
> @@ -773,8 +779,10 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data,
>          for(i = 0; i <= stereo; i++)
>              *samples8++ = pred[i];
>          for(; i < unp_size; i++) {
> -            if(get_bits_left(&gb)<0)
> -                return AVERROR_INVALIDDATA;
> +            if(get_bits_left(&gb)<0) {
> +                ret = AVERROR_INVALIDDATA;
> +                goto error;
> +            }
>              if(i & stereo){
>                  if(vlc[1].table)
>                      res = get_vlc2(&gb, vlc[1].table, SMKTREE_BITS, 3);
> @@ -782,7 +790,8 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data,
>                      res = 0;
>                  if (res < 0) {
>                      av_log(avctx, AV_LOG_ERROR, "invalid vlc\n");
> -                    return AVERROR_INVALIDDATA;
> +                    ret = AVERROR_INVALIDDATA;
> +                    goto error;
>                  }
>                  pred[1] += sign_extend(h[1].values[res], 8);
>                  *samples8++ = pred[1];
> @@ -793,7 +802,8 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data,
>                      res = 0;
>                  if (res < 0) {
>                      av_log(avctx, AV_LOG_ERROR, "invalid vlc\n");
> -                    return AVERROR_INVALIDDATA;
> +                    ret = AVERROR_INVALIDDATA;
> +                    goto error;
>                  }
>                  pred[0] += sign_extend(h[0].values[res], 8);
>                  *samples8++ = pred[0];
>
Michael Niedermayer Oct. 16, 2019, 4:46 p.m. UTC | #2 
On Sun, Oct 13, 2019 at 12:20:28AM -0300, James Almer wrote:
> On 10/10/2019 7:40 PM, Michael Niedermayer wrote:
> > Fixes: multiple memleaks
> > Fixes: 17660/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SMACKAUD_fuzzer-5689769928949760
> > Fixes: 18064/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SMACKAUD_fuzzer-5631086809317376
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/smacker.c | 30 ++++++++++++++++++++----------
> >  1 file changed, 20 insertions(+), 10 deletions(-)
> > 
> > diff --git a/libavcodec/smacker.c b/libavcodec/smacker.c
> > index a2950c455b..901cdb1fb1 100644
> > --- a/libavcodec/smacker.c
> > +++ b/libavcodec/smacker.c
> > @@ -721,8 +721,10 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data,
> >          for(i = 0; i <= stereo; i++)
> >              *samples++ = pred[i];
> >          for(; i < unp_size / 2; i++) {
> > -            if(get_bits_left(&gb)<0)
> > -                return AVERROR_INVALIDDATA;
> > +            if(get_bits_left(&gb)<0) {
> 
> Take the opportunity to add white spaces here and the other changed case
> below.

will apply with white space improved

thanks

[...]
diff mbox

Patch

diff --git a/libavcodec/smacker.c b/libavcodec/smacker.c
index a2950c455b..901cdb1fb1 100644
--- a/libavcodec/smacker.c
+++ b/libavcodec/smacker.c
@@ -721,8 +721,10 @@  static int smka_decode_frame(AVCodecContext *avctx, void *data,
         for(i = 0; i <= stereo; i++)
             *samples++ = pred[i];
         for(; i < unp_size / 2; i++) {
-            if(get_bits_left(&gb)<0)
-                return AVERROR_INVALIDDATA;
+            if(get_bits_left(&gb)<0) {
+                ret = AVERROR_INVALIDDATA;
+                goto error;
+            }
             if(i & stereo) {
                 if(vlc[2].table)
                     res = get_vlc2(&gb, vlc[2].table, SMKTREE_BITS, 3);
@@ -730,7 +732,8 @@  static int smka_decode_frame(AVCodecContext *avctx, void *data,
                     res = 0;
                 if (res < 0) {
                     av_log(avctx, AV_LOG_ERROR, "invalid vlc\n");
-                    return AVERROR_INVALIDDATA;
+                    ret = AVERROR_INVALIDDATA;
+                    goto error;
                 }
                 val  = h[2].values[res];
                 if(vlc[3].table)
@@ -739,7 +742,8 @@  static int smka_decode_frame(AVCodecContext *avctx, void *data,
                     res = 0;
                 if (res < 0) {
                     av_log(avctx, AV_LOG_ERROR, "invalid vlc\n");
-                    return AVERROR_INVALIDDATA;
+                    ret = AVERROR_INVALIDDATA;
+                    goto error;
                 }
                 val |= h[3].values[res] << 8;
                 pred[1] += sign_extend(val, 16);
@@ -751,7 +755,8 @@  static int smka_decode_frame(AVCodecContext *avctx, void *data,
                     res = 0;
                 if (res < 0) {
                     av_log(avctx, AV_LOG_ERROR, "invalid vlc\n");
-                    return AVERROR_INVALIDDATA;
+                    ret = AVERROR_INVALIDDATA;
+                    goto error;
                 }
                 val  = h[0].values[res];
                 if(vlc[1].table)
@@ -760,7 +765,8 @@  static int smka_decode_frame(AVCodecContext *avctx, void *data,
                     res = 0;
                 if (res < 0) {
                     av_log(avctx, AV_LOG_ERROR, "invalid vlc\n");
-                    return AVERROR_INVALIDDATA;
+                    ret = AVERROR_INVALIDDATA;
+                    goto error;
                 }
                 val |= h[1].values[res] << 8;
                 pred[0] += sign_extend(val, 16);
@@ -773,8 +779,10 @@  static int smka_decode_frame(AVCodecContext *avctx, void *data,
         for(i = 0; i <= stereo; i++)
             *samples8++ = pred[i];
         for(; i < unp_size; i++) {
-            if(get_bits_left(&gb)<0)
-                return AVERROR_INVALIDDATA;
+            if(get_bits_left(&gb)<0) {
+                ret = AVERROR_INVALIDDATA;
+                goto error;
+            }
             if(i & stereo){
                 if(vlc[1].table)
                     res = get_vlc2(&gb, vlc[1].table, SMKTREE_BITS, 3);
@@ -782,7 +790,8 @@  static int smka_decode_frame(AVCodecContext *avctx, void *data,
                     res = 0;
                 if (res < 0) {
                     av_log(avctx, AV_LOG_ERROR, "invalid vlc\n");
-                    return AVERROR_INVALIDDATA;
+                    ret = AVERROR_INVALIDDATA;
+                    goto error;
                 }
                 pred[1] += sign_extend(h[1].values[res], 8);
                 *samples8++ = pred[1];
@@ -793,7 +802,8 @@  static int smka_decode_frame(AVCodecContext *avctx, void *data,
                     res = 0;
                 if (res < 0) {
                     av_log(avctx, AV_LOG_ERROR, "invalid vlc\n");
-                    return AVERROR_INVALIDDATA;
+                    ret = AVERROR_INVALIDDATA;
+                    goto error;
                 }
                 pred[0] += sign_extend(h[0].values[res], 8);
                 *samples8++ = pred[0];