diff mbox

[FFmpeg-devel] 2 alternative ways to check in vp9 decode_tiles() if there is data remaining

Message ID 20181020104235.GB3743@michaelspb
State New
Headers show

Commit Message

Michael Niedermayer Oct. 20, 2018, 10:42 a.m. UTC 
Hi

2 alternative patchsets are attached to fix $SUBJ

The 2 alternatives should behave similar.

The first adds a function to check if the next range coder symbol read would
trigger the end of input case.
We then error out before reading in case the read would trigger this case

The second sets a flag if the end of input case triggered and subsequently
errors out

The second case should be slower as it requires additional checks in inner
loops, but i was asked to implement this with a flag, so i implemented both
ways.

Which version, if any, should i apply ?

Thanks

[...]

Comments

Michael Niedermayer Oct. 29, 2018, 8:30 p.m. UTC | #1 
On Sat, Oct 20, 2018 at 12:42:35PM +0200, Michael Niedermayer wrote:
> Hi
> 
> 2 alternative patchsets are attached to fix $SUBJ
> 
> The 2 alternatives should behave similar.
> 
> The first adds a function to check if the next range coder symbol read would
> trigger the end of input case.
> We then error out before reading in case the read would trigger this case
> 
> The second sets a flag if the end of input case triggered and subsequently
> errors out
> 
> The second case should be slower as it requires additional checks in inner
> loops, but i was asked to implement this with a flag, so i implemented both
> ways.
> 
> Which version, if any, should i apply ?

this also fixes 9775/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP9_fuzzer-5643845344690176
ill apply the one that avoids checks in the inner loop.
If people prefer the other iam happy to revert it and replace it by the
other solution. But i dont want to leave the issue open 

Thanks
[...]
diff mbox

Patch

From 72aa2377c7b401f1a0c2866bc1471f4c98310415 Mon Sep 17 00:00:00 2001
From: Michael Niedermayer <michael@niedermayer.cc>
Date: Sat, 4 Aug 2018 22:21:02 +0200
Subject: [PATCH 2/2] avcodec/vp9: Check in decode_tiles() if there is data
 remaining

Fixes: Timeout
Fixes: 9330/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP9_fuzzer-5707345857347584

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

diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index b1178c9c0c..dd5c8098c8 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -1308,6 +1308,9 @@  static int decode_tiles(AVCodecContext *avctx,
                     } else {
                         decode_sb(td, row, col, lflvl_ptr,
                                   yoff2, uvoff2, BL_64X64);
+                        if (td->c->is_end) {
+                            return AVERROR_INVALIDDATA;
+                        }
                     }
                 }
             }
-- 
2.19.1