Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation
@ 2023-08-09 18:34 Michael Niedermayer
  2023-08-09 18:53 ` Paul B Mahol
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Niedermayer @ 2023-08-09 18:34 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Fixes: Timeout
Fixes: 60867/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MV30_fuzzer-6381933108527104
Fixes: 30147/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MV30_fuzzer-5549246684200960

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/mv30.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libavcodec/mv30.c b/libavcodec/mv30.c
index 0b19534b00..addf188c92 100644
--- a/libavcodec/mv30.c
+++ b/libavcodec/mv30.c
@@ -411,6 +411,8 @@ static int decode_intra(AVCodecContext *avctx, GetBitContext *gb, AVFrame *frame
     mgb = *gb;
     if (get_bits_left(gb) < s->mode_size * 8)
         return AVERROR_INVALIDDATA;
+    if (s->mode_size * 8 < (avctx->height + 15)/16 * ((avctx->width + 15)/16) * 12)
+        return AVERROR_INVALIDDATA;
 
     skip_bits_long(gb, s->mode_size * 8);
 
@@ -476,6 +478,9 @@ static int decode_inter(AVCodecContext *avctx, GetBitContext *gb,
     int ret, cnt = 0;
     int flags = 0;
 
+    if (get_bits_left(gb) < (mask_size + s->mode_size) * 8)
+        return AVERROR_INVALIDDATA;
+
     if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
         return ret;
 
-- 
2.17.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation
  2023-08-09 18:34 [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation Michael Niedermayer
@ 2023-08-09 18:53 ` Paul B Mahol
  2023-08-09 19:30   ` Michael Niedermayer
  0 siblings, 1 reply; 11+ messages in thread
From: Paul B Mahol @ 2023-08-09 18:53 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

This is not correct, and please stop writing such patches. Thanks.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation
  2023-08-09 18:53 ` Paul B Mahol
@ 2023-08-09 19:30   ` Michael Niedermayer
  2023-08-09 21:20     ` Paul B Mahol
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Niedermayer @ 2023-08-09 19:30 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 559 bytes --]

Hi Paul

On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:
> This is not correct, and please stop writing such patches. Thanks.

If there is a problem in the bugfix, please explain what the problem is.
If you do not like the specific fix, you can fix it differently too or
tell me what you prefer.
Simply saying "no" with no explanation repeatedly is rude

thank you

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation
  2023-08-09 21:20     ` Paul B Mahol
@ 2023-08-09 21:14       ` James Almer
  2023-08-09 21:34         ` Paul B Mahol
  2023-08-10  9:34       ` Michael Niedermayer
  1 sibling, 1 reply; 11+ messages in thread
From: James Almer @ 2023-08-09 21:14 UTC (permalink / raw)
  To: ffmpeg-devel

On 8/9/2023 6:20 PM, Paul B Mahol wrote:
> On Wed, Aug 9, 2023 at 9:30 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
>> Hi Paul
>>
>> On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:
>>> This is not correct, and please stop writing such patches. Thanks.
>>
>> If there is a problem in the bugfix, please explain what the problem is.
>> If you do not like the specific fix, you can fix it differently too or
>> tell me what you prefer.
>> Simply saying "no" with no explanation repeatedly is rude
>>
> 
> Patch breaks valid files.

Can those files be added to FATE alongside a test?

> 
> 
>>
>> thank you
>>
>> [...]
>> --
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> I have often repented speaking, but never of holding my tongue.
>> -- Xenocrates
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation
  2023-08-09 19:30   ` Michael Niedermayer
@ 2023-08-09 21:20     ` Paul B Mahol
  2023-08-09 21:14       ` James Almer
  2023-08-10  9:34       ` Michael Niedermayer
  0 siblings, 2 replies; 11+ messages in thread
From: Paul B Mahol @ 2023-08-09 21:20 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Wed, Aug 9, 2023 at 9:30 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> Hi Paul
>
> On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:
> > This is not correct, and please stop writing such patches. Thanks.
>
> If there is a problem in the bugfix, please explain what the problem is.
> If you do not like the specific fix, you can fix it differently too or
> tell me what you prefer.
> Simply saying "no" with no explanation repeatedly is rude
>

Patch breaks valid files.


>
> thank you
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I have often repented speaking, but never of holding my tongue.
> -- Xenocrates
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation
  2023-08-09 21:14       ` James Almer
@ 2023-08-09 21:34         ` Paul B Mahol
  0 siblings, 0 replies; 11+ messages in thread
From: Paul B Mahol @ 2023-08-09 21:34 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Wed, Aug 9, 2023 at 11:15 PM James Almer <jamrial@gmail.com> wrote:

> On 8/9/2023 6:20 PM, Paul B Mahol wrote:
> > On Wed, Aug 9, 2023 at 9:30 PM Michael Niedermayer <
> michael@niedermayer.cc>
> > wrote:
> >
> >> Hi Paul
> >>
> >> On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:
> >>> This is not correct, and please stop writing such patches. Thanks.
> >>
> >> If there is a problem in the bugfix, please explain what the problem is.
> >> If you do not like the specific fix, you can fix it differently too or
> >> tell me what you prefer.
> >> Simply saying "no" with no explanation repeatedly is rude
> >>
> >
> > Patch breaks valid files.
>
> Can those files be added to FATE alongside a test?
>

Sure, once SDR files have been removed.


>
> >
> >
> >>
> >> thank you
> >>
> >> [...]
> >> --
> >> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >>
> >> I have often repented speaking, but never of holding my tongue.
> >> -- Xenocrates
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >>
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation
  2023-08-09 21:20     ` Paul B Mahol
  2023-08-09 21:14       ` James Almer
@ 2023-08-10  9:34       ` Michael Niedermayer
  2023-08-10 10:12         ` Paul B Mahol
  1 sibling, 1 reply; 11+ messages in thread
From: Michael Niedermayer @ 2023-08-10  9:34 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1406 bytes --]

On Wed, Aug 09, 2023 at 11:20:43PM +0200, Paul B Mahol wrote:
> On Wed, Aug 9, 2023 at 9:30 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > Hi Paul
> >
> > On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:
> > > This is not correct, and please stop writing such patches. Thanks.
> >
> > If there is a problem in the bugfix, please explain what the problem is.
> > If you do not like the specific fix, you can fix it differently too or
> > tell me what you prefer.
> > Simply saying "no" with no explanation repeatedly is rude
> >
> 
> Patch breaks valid files.

Does the patch break files you create intentionally or files
pre-existing ?
The check can fail if 2 data segments overlap, one can craft
a file with that. The previous patches are implemented differently
and dont have that behavior, you rejected them too and at the time
you did call them "hacky" and did not mention that they break anything
and also ignored all further questions

I just implemented this one differently as the other way was rejected
by you with no comment

Also please provide the files this breaks so the issue can be
fixed

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation
  2023-08-10  9:34       ` Michael Niedermayer
@ 2023-08-10 10:12         ` Paul B Mahol
  2023-08-10 15:46           ` Michael Niedermayer
  0 siblings, 1 reply; 11+ messages in thread
From: Paul B Mahol @ 2023-08-10 10:12 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Thu, Aug 10, 2023 at 11:34 AM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Wed, Aug 09, 2023 at 11:20:43PM +0200, Paul B Mahol wrote:
> > On Wed, Aug 9, 2023 at 9:30 PM Michael Niedermayer <
> michael@niedermayer.cc>
> > wrote:
> >
> > > Hi Paul
> > >
> > > On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:
> > > > This is not correct, and please stop writing such patches. Thanks.
> > >
> > > If there is a problem in the bugfix, please explain what the problem
> is.
> > > If you do not like the specific fix, you can fix it differently too or
> > > tell me what you prefer.
> > > Simply saying "no" with no explanation repeatedly is rude
> > >
> >
> > Patch breaks valid files.
>
> Does the patch break files you create intentionally or files
> pre-existing ?
> The check can fail if 2 data segments overlap, one can craft
> a file with that. The previous patches are implemented differently
> and dont have that behavior, you rejected them too and at the time
> you did call them "hacky" and did not mention that they break anything
> and also ignored all further questions
>
> I just implemented this one differently as the other way was rejected
> by you with no comment
>
> Also please provide the files this breaks so the issue can be
> fixed
>
>
Why not same thing for AV1 codec?
Just reduce max resolutions for mv30 to 32x32 and be done.



> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation
  2023-08-10 10:12         ` Paul B Mahol
@ 2023-08-10 15:46           ` Michael Niedermayer
  2023-08-10 15:58             ` Paul B Mahol
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Niedermayer @ 2023-08-10 15:46 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 2121 bytes --]

On Thu, Aug 10, 2023 at 12:12:51PM +0200, Paul B Mahol wrote:
> On Thu, Aug 10, 2023 at 11:34 AM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > On Wed, Aug 09, 2023 at 11:20:43PM +0200, Paul B Mahol wrote:
> > > On Wed, Aug 9, 2023 at 9:30 PM Michael Niedermayer <
> > michael@niedermayer.cc>
> > > wrote:
> > >
> > > > Hi Paul
> > > >
> > > > On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:
> > > > > This is not correct, and please stop writing such patches. Thanks.
> > > >
> > > > If there is a problem in the bugfix, please explain what the problem
> > is.
> > > > If you do not like the specific fix, you can fix it differently too or
> > > > tell me what you prefer.
> > > > Simply saying "no" with no explanation repeatedly is rude
> > > >
> > >
> > > Patch breaks valid files.
> >
> > Does the patch break files you create intentionally or files
> > pre-existing ?
> > The check can fail if 2 data segments overlap, one can craft
> > a file with that. The previous patches are implemented differently
> > and dont have that behavior, you rejected them too and at the time
> > you did call them "hacky" and did not mention that they break anything
> > and also ignored all further questions
> >
> > I just implemented this one differently as the other way was rejected
> > by you with no comment
> >
> > Also please provide the files this breaks so the issue can be
> > fixed
> >
> >
> Why not same thing for AV1 codec?
> Just reduce max resolutions for mv30 to 32x32 and be done.

Limiting the resolution to max 32x32 would break real samples
for example V-codecs/mv30.avi

if you suggest to limit it only for the fuzzer, well, that would not
fix the timeout outside the fuzzer.
For some decoders limiting the resolution in the fuzzer is the only practical
option. But for mv30 this timeout really occurs because the input is not
checked/validated.

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation
  2023-08-10 15:46           ` Michael Niedermayer
@ 2023-08-10 15:58             ` Paul B Mahol
  2023-08-12 14:33               ` Michael Niedermayer
  0 siblings, 1 reply; 11+ messages in thread
From: Paul B Mahol @ 2023-08-10 15:58 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Thu, Aug 10, 2023 at 5:46 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Thu, Aug 10, 2023 at 12:12:51PM +0200, Paul B Mahol wrote:
> > On Thu, Aug 10, 2023 at 11:34 AM Michael Niedermayer <
> michael@niedermayer.cc>
> > wrote:
> >
> > > On Wed, Aug 09, 2023 at 11:20:43PM +0200, Paul B Mahol wrote:
> > > > On Wed, Aug 9, 2023 at 9:30 PM Michael Niedermayer <
> > > michael@niedermayer.cc>
> > > > wrote:
> > > >
> > > > > Hi Paul
> > > > >
> > > > > On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:
> > > > > > This is not correct, and please stop writing such patches.
> Thanks.
> > > > >
> > > > > If there is a problem in the bugfix, please explain what the
> problem
> > > is.
> > > > > If you do not like the specific fix, you can fix it differently
> too or
> > > > > tell me what you prefer.
> > > > > Simply saying "no" with no explanation repeatedly is rude
> > > > >
> > > >
> > > > Patch breaks valid files.
> > >
> > > Does the patch break files you create intentionally or files
> > > pre-existing ?
> > > The check can fail if 2 data segments overlap, one can craft
> > > a file with that. The previous patches are implemented differently
> > > and dont have that behavior, you rejected them too and at the time
> > > you did call them "hacky" and did not mention that they break anything
> > > and also ignored all further questions
> > >
> > > I just implemented this one differently as the other way was rejected
> > > by you with no comment
> > >
> > > Also please provide the files this breaks so the issue can be
> > > fixed
> > >
> > >
> > Why not same thing for AV1 codec?
> > Just reduce max resolutions for mv30 to 32x32 and be done.
>
> Limiting the resolution to max 32x32 would break real samples
> for example V-codecs/mv30.avi
>
> if you suggest to limit it only for the fuzzer, well, that would not
> fix the timeout outside the fuzzer.
> For some decoders limiting the resolution in the fuzzer is the only
> practical
> option. But for mv30 this timeout really occurs because the input is not
> checked/validated.
>

But mv30 bitstream is not so trivial and can not be checked that easily
with your patches.

Also whole fuzzing idea of your work is flawed.


>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> In a rich man's house there is no place to spit but his face.
> -- Diogenes of Sinope
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation
  2023-08-10 15:58             ` Paul B Mahol
@ 2023-08-12 14:33               ` Michael Niedermayer
  0 siblings, 0 replies; 11+ messages in thread
From: Michael Niedermayer @ 2023-08-12 14:33 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 3789 bytes --]

On Thu, Aug 10, 2023 at 05:58:24PM +0200, Paul B Mahol wrote:
> On Thu, Aug 10, 2023 at 5:46 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > On Thu, Aug 10, 2023 at 12:12:51PM +0200, Paul B Mahol wrote:
> > > On Thu, Aug 10, 2023 at 11:34 AM Michael Niedermayer <
> > michael@niedermayer.cc>
> > > wrote:
> > >
> > > > On Wed, Aug 09, 2023 at 11:20:43PM +0200, Paul B Mahol wrote:
> > > > > On Wed, Aug 9, 2023 at 9:30 PM Michael Niedermayer <
> > > > michael@niedermayer.cc>
> > > > > wrote:
> > > > >
> > > > > > Hi Paul
> > > > > >
> > > > > > On Wed, Aug 09, 2023 at 08:53:03PM +0200, Paul B Mahol wrote:
> > > > > > > This is not correct, and please stop writing such patches.
> > Thanks.
> > > > > >
> > > > > > If there is a problem in the bugfix, please explain what the
> > problem
> > > > is.
> > > > > > If you do not like the specific fix, you can fix it differently
> > too or
> > > > > > tell me what you prefer.
> > > > > > Simply saying "no" with no explanation repeatedly is rude
> > > > > >
> > > > >
> > > > > Patch breaks valid files.
> > > >
> > > > Does the patch break files you create intentionally or files
> > > > pre-existing ?
> > > > The check can fail if 2 data segments overlap, one can craft
> > > > a file with that. The previous patches are implemented differently
> > > > and dont have that behavior, you rejected them too and at the time
> > > > you did call them "hacky" and did not mention that they break anything
> > > > and also ignored all further questions
> > > >
> > > > I just implemented this one differently as the other way was rejected
> > > > by you with no comment
> > > >
> > > > Also please provide the files this breaks so the issue can be
> > > > fixed
> > > >
> > > >
> > > Why not same thing for AV1 codec?
> > > Just reduce max resolutions for mv30 to 32x32 and be done.
> >
> > Limiting the resolution to max 32x32 would break real samples
> > for example V-codecs/mv30.avi
> >
> > if you suggest to limit it only for the fuzzer, well, that would not
> > fix the timeout outside the fuzzer.
> > For some decoders limiting the resolution in the fuzzer is the only
> > practical
> > option. But for mv30 this timeout really occurs because the input is not
> > checked/validated.
> >
> 
> But mv30 bitstream is not so trivial and can not be checked that easily
> with your patches.

for decode_intra the loop reads from mgb look like this:

    for (int y = 0; y < avctx->height; y += 16) {
...
        for (int x = 0; x < avctx->width; x += 16) {
...
            for (int b = 0; b < 6; b++) {
                int mode = get_bits_le(&mgb, 2);
...
            }
        }

the only way to escape this loop is through error exit
so 2*6* number of 16x16 blocks is read unconditionally
and its read from a place that has s->mode_size * 8 bits
alternatively get_bits_left() can be used


for decode inter:
there is this at the begin
    mask = *gb;
    skip_bits_long(gb, mask_size * 8);
    mgb = *gb;
    skip_bits_long(gb, s->mode_size * 8);

so we have at minimum (mask_size + s->mode_size) * 8 bits

It does seem reasonable to me to check the input to contain enough bits
for what is consumed
If it breaks some real files, i very much would like to know what these
files are doing


> 
> Also whole fuzzing idea of your work is flawed.

what do you mean by "whole fuzzing idea" ?

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-08-12 14:33 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-09 18:34 [FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation Michael Niedermayer
2023-08-09 18:53 ` Paul B Mahol
2023-08-09 19:30   ` Michael Niedermayer
2023-08-09 21:20     ` Paul B Mahol
2023-08-09 21:14       ` James Almer
2023-08-09 21:34         ` Paul B Mahol
2023-08-10  9:34       ` Michael Niedermayer
2023-08-10 10:12         ` Paul B Mahol
2023-08-10 15:46           ` Michael Niedermayer
2023-08-10 15:58             ` Paul B Mahol
2023-08-12 14:33               ` Michael Niedermayer

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git