[FFmpeg-devel] [PATCH 1/4] avcodec/prosumer: Check for bytestream eof in decompress()

[Michael Niedermayer]

On Wed, Oct 31, 2018 at 09:59:04AM -0300, James Almer wrote:
> On 10/31/2018 9:18 AM, Michael Niedermayer wrote:
> > On Wed, Oct 31, 2018 at 12:06:28PM +0100, Hendrik Leppkes wrote:
> >> On Wed, Oct 31, 2018 at 12:03 PM Michael Niedermayer
> >> <michael at niedermayer.cc> wrote:
> >>>
> >>> Fixes: Infinite loop
> >>> Fixes: 10685/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_PROSUMER_fuzzer-5652236881887232
> >>>
> >>> 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/prosumer.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavcodec/prosumer.c b/libavcodec/prosumer.c
> >>> index 6e98677b55..2fd9880ee1 100644
> >>> --- a/libavcodec/prosumer.c
> >>> +++ b/libavcodec/prosumer.c
> >>> @@ -57,7 +57,7 @@ static int decompress(GetByteContext *gb, int size, PutByteContext *pb, const ui
> >>>      b = lut[2 * idx];
> >>>
> >>>      while (1) {
> >>> -        if (bytestream2_get_bytes_left_p(pb) <= 0)
> >>> +        if (bytestream2_get_bytes_left_p(pb) <= 0 || bytestream2_get_eof(pb))
> >>>              return 0;
> >>>          if (((b & 0xFF00u) != 0x8000u) || (b & 0xFFu)) {
> >>>              if ((b & 0xFF00u) != 0x8000u) {
> >>
> >> Why does bytestream2_get_bytes_left_p not return <= 0 at eof, where
> >> there certainly arent any bytes left?
> > 
> > there are 2 bytes left because a 4 byte write failed to fit in 2 bytes
> > 
> > maybe bytestream2_get_bytes_left_p() should be made to return 0 once eof is
> > set even if space remains
> 
> Unrelated to this, but seeing how all the main GetByteContext functions
> start with bytestream2_get and all the PutByteContext functions with
> bytestream2_put, the fact these two functions from the latter start with
> bytestream2_get is not ideal.
> You have bytestream2_get_bytes_left for GetByteContext and
> bytestream2_get_bytes_left_p with the suffix p hinting it's for
> PutByteContext, but then you have bytestream2_get_eof for PutByteContext
> without a suffix and nothing for GetByteContext. It's confusing and not
> exactly intuitive.

fully agree



> 
> In GetBitContext and PutBitContext they are properly called
> get_bits_left() and put_bits_left() respectively, for example.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- 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/20181031/debb6707/attachment.sig>