[FFmpeg-devel] [PATCH] avcodec/golomb: Prevent shift by negative number

[Michael Niedermayer]

On Tue, Jul 14, 2020 at 06:10:39PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Mon, Jul 13, 2020 at 09:04:30PM +0200, Tomas Härdin wrote:
> >> fre 2020-07-10 klockan 15:48 +0200 skrev Andreas Rheinhardt:
> >>> This happened in get_ue_golomb() if the cached bitstream reader was
> >>> in
> >>> use, because there was no check to handle the case of the read value
> >>> not being in the range 0..8190.
> >>>
> >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> >>> ---
> >>>  libavcodec/golomb.h | 4 ++++
> >>>  1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> >>> index 7fd46a91bd..5bfcfe085f 100644
> >>> --- a/libavcodec/golomb.h
> >>> +++ b/libavcodec/golomb.h
> >>> @@ -66,6 +66,10 @@ static inline int get_ue_golomb(GetBitContext *gb)
> >>>          return ff_ue_golomb_vlc_code[buf];
> >>>      } else {
> >>>          int log = 2 * av_log2(buf) - 31;
> >>> +        if (log < 0) {
> >>> +            av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> >>> +            return AVERROR_INVALIDDATA;
> >>> +        }
> >>
> >> How come invalid codes can even be present like this? Seems
> >> inefficient. Or is the decoder wrong? Maybe Michael wants to chime in
> >> here, since he wrote the original implementation.
> > 
> > The codepath of the non cached one does a check too. It should be
> > done consistently.
> 
> Does this imply that you want me to error out for log < 7 although the
> cached bitstream reader is able to read the number even when log is in
> the range of 0..6? (The reason for this is that one only gets 25 valid
> bits in the noncached version.)

is it intended to be valid for this function to be used for longer codes ?
if not it should fail because otherwise a bug in the code would be missed
also it would result in fewer differences and easier testing as both
would behave the same

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200714/65b4afc6/attachment.sig>