[FFmpeg-devel] [PATCH 2/4] avcodec/hevc_ps: Fix integer overflow with num_tile_rows

[Michael Niedermayer]

On Mon, Jun 17, 2019 at 07:55:45PM -0300, James Almer wrote:
> On 6/17/2019 6:54 PM, Michael Niedermayer wrote:
> > On Sun, Jun 16, 2019 at 11:10:43PM -0300, James Almer wrote:
> >> On 6/13/2019 3:32 PM, Michael Niedermayer wrote:
> >>> Fixes: signed integer overflow: -2147483648 - 1 cannot be represented in type 'int'
> >>> Fixes: 14880/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-5130977304641536
> >>>
> >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> >>> ---
> >>>  libavcodec/hevc_ps.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> >>> index 80df417e4f..0ed6682bb4 100644
> >>> --- a/libavcodec/hevc_ps.c
> >>> +++ b/libavcodec/hevc_ps.c
> >>> @@ -1596,7 +1596,7 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, AVCodecContext *avctx,
> >>>          if (pps->num_tile_rows <= 0 ||
> >>>              pps->num_tile_rows >= sps->height) {
> >>>              av_log(avctx, AV_LOG_ERROR, "num_tile_rows_minus1 out of range: %d\n",
> >>> -                   pps->num_tile_rows - 1);
> >>> +                   pps->num_tile_rows - 1U);
> >>
> >> The proper fix for this is making pps->num_tile_rows/cols unsigned. 
[...]
> > 
> > is this here ok if num_tile_rows is 0 ?
> > for (i = 0; i < pps->num_tile_rows - 1; i++) { (example line from ffmpeg git)
> > 
> > i would guess nearly everyone wold say yes without having seen the
> > discussion about the type. but of course if this is unsigned its not
> > going to be safe with it being 0. 
> 
> pps->num_tile_rows is set to a value returned by "get_ue_golomb_long() +
> 1", which will always be in the 1..UINT32_MAX range. It can't be 0, as
> it would be a bug. Int is definitely not the right type for it.

i dont think num_tile_rows of more than 2^31 (that is the negative when signed range)
makes much sense but sure i can change it to unsigned if preferred.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190619/55c9f272/attachment.sig>