Re: [FFmpeg-devel] [PATCH 1/4] avcodec/prosumer: Check for bytestream eof in decompress()
On Wed, Oct 31, 2018 at 12:13:50PM +0100, Paul B Mahol wrote: > On 10/31/18, Michael Niedermayer 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 > > --- > > libavcodec/prosumer.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > patchset looks ok. will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship naturally arises out of democracy, and the most aggravated form of tyranny and slavery out of the most extreme liberty. -- Plato signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] avcodec/prosumer: Check for bytestream eof in decompress()
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
> >> 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
> >>> ---
> >>> 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@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
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] avcodec/prosumer: Check for bytestream eof in decompress()
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
>> 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
>>> ---
>>> 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.
In GetBitContext and PutBitContext they are properly called
get_bits_left() and put_bits_left() respectively, for example.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] avcodec/prosumer: Check for bytestream eof in decompress()
On Wed, Oct 31, 2018 at 12:06:28PM +0100, Hendrik Leppkes wrote:
> On Wed, Oct 31, 2018 at 12:03 PM Michael Niedermayer
> 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
> > ---
> > 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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Avoid a single point of failure, be that a person or equipment.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] avcodec/prosumer: Check for bytestream eof in decompress()
On 10/31/18, Michael Niedermayer 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 > --- > libavcodec/prosumer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > patchset looks ok. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] avcodec/prosumer: Check for bytestream eof in decompress()
On Wed, Oct 31, 2018 at 12:03 PM Michael Niedermayer
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
> ---
> 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?
- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/4] avcodec/prosumer: Check for bytestream eof in decompress()
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
---
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) {
--
2.19.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
