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

[Michael Niedermayer]

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. 

I dont think "unsigned int" is wise to use as type here, the semantics 
of unsigned ints are unexpected to many
especially making random subsets of "normal" fields unsigned will make
the codebase slowly "interresting".

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. 


> The
> minimum allowed value for num_tile_{rows,cols}_minus1 is 0.

yes


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

You can kill me, but you cannot change the truth.
-------------- 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/20190617/f6796cad/attachment.sig>