diff mbox

[FFmpeg-devel,1/5] avcodec/mpeg4videodec: Move decode_studiovisualobject() parsing in the branch for visual object parsing

Message ID 20180429191918.2915-1-michael@niedermayer.cc
State Accepted
Commit e03bf251d8784f4d1df2c22381c902087e151e31
Headers show

Commit Message

Michael Niedermayer April 29, 2018, 7:19 p.m. UTC
Fixes: runtime error: shift exponent -1 is negative
Fixes: 7510/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG4_fuzzer-5024523356209152

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

Comments

Kieran Kunhya April 29, 2018, 8:23 p.m. UTC | #1
On Sun, 29 Apr 2018 at 20:20 Michael Niedermayer <michael@niedermayer.cc>
wrote:

> Fixes: runtime error: shift exponent -1 is negative
> Fixes:
> 7510/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG4_fuzzer-5024523356209152
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by
> <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>:
> Michael Niedermayer <michael@niedermayer.cc>
>

No, this is wrong, extension_and_user_data( 0 ) may precede
StudioVisualObject.

Kieran
Michael Niedermayer April 30, 2018, 4:43 p.m. UTC | #2
On Sun, Apr 29, 2018 at 08:23:39PM +0000, Kieran Kunhya wrote:
> On Sun, 29 Apr 2018 at 20:20 Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > Fixes: runtime error: shift exponent -1 is negative
> > Fixes:
> > 7510/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG4_fuzzer-5024523356209152
> >
> > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by
> > <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>:
> > Michael Niedermayer <michael@niedermayer.cc>
> >
> 
> No, this is wrong, extension_and_user_data( 0 ) may precede
> StudioVisualObject.

The code before this patch calls a single extension_and_user_data(0)
between VOS_STARTCODE and VISUAL_OBJ_STARTCODE

the code after this patch still does this.

The implementation for extension_and_user_data(0) does nothing though
everything the code does is behind a
if ((id == 2 || id == 4) && ...
so id == 0 does nothing

extension_and_user_data(0) should parse user data.
This was not done before this patch, but should work after the patch
as there is a entry in the main loop to parse user data.
The code before bypassed the main loop.

If there are no further objections i intend to apply this in a day or 2

Thanks

[...]
diff mbox

Patch

diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
index 32eb3d1ca8..27602e8542 100644
--- a/libavcodec/mpeg4videodec.c
+++ b/libavcodec/mpeg4videodec.c
@@ -2982,14 +2982,9 @@  static int decode_studio_vop_header(Mpeg4DecContext *ctx, GetBitContext *gb)
 
 static int decode_studiovisualobject(Mpeg4DecContext *ctx, GetBitContext *gb)
 {
-    uint32_t startcode;
     MpegEncContext *s = &ctx->m;
     int visual_object_type, width, height;
 
-    startcode = get_bits_long(gb, 32);
-
-    /* StudioVisualObject() */
-    if (startcode == VISUAL_OBJ_STARTCODE) {
         skip_bits(gb, 4); /* visual_object_verid */
         visual_object_type = get_bits(gb, 4);
 
@@ -3069,7 +3064,6 @@  static int decode_studiovisualobject(Mpeg4DecContext *ctx, GetBitContext *gb)
             next_start_code_studio(gb);
             extension_and_user_data(s, gb, 2);
         }
-    }
 
     return 0;
 }
@@ -3192,13 +3186,14 @@  int ff_mpeg4_decode_picture_header(Mpeg4DecContext *ctx, GetBitContext *gb)
                 s->studio_profile = 1;
                 next_start_code_studio(gb);
                 extension_and_user_data(s, gb, 0);
-
+            }
+        } else if (startcode == VISUAL_OBJ_STARTCODE) {
+            if (s->studio_profile) {
                 if ((ret = decode_studiovisualobject(ctx, gb)) < 0)
                     return ret;
                 break;
-            }
-        } else if (startcode == VISUAL_OBJ_STARTCODE) {
-            mpeg4_decode_visual_object(s, gb);
+            } else
+                mpeg4_decode_visual_object(s, gb);
         } else if (startcode == VOP_STARTCODE) {
             break;
         }