Re: [FFmpeg-devel] [PATCH 4/6] avcodec/pngdec: Check amount decoded

2020-01-28 Thread Michael Niedermayer 
On Sun, Aug 18, 2019 at 01:28:39AM +0200, Michael Niedermayer wrote:
> Fixes: Timeout (70sec -> 243ms)
> Fixes: 
> 16097/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APNG_fuzzer-5664690889293824
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/pngdec.c | 12 
>  1 file changed, 12 insertions(+)

There are now 4 testcases this patch fixes:
Fixes: 
16097/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APNG_fuzzer-5664690889293824
Fixes: 
16927/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APNG_fuzzer-5170612070252544
Fixes: 
16927/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APNG_fuzzer-5706325622784000
Fixes: 
18705/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APNG_fuzzer-5650989302677504

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 4/6] avcodec/pngdec: Check amount decoded

2019-09-02 Thread Michael Niedermayer 
On Sun, Aug 18, 2019 at 11:04:20AM +0200, Paul B Mahol wrote:
> On Sun, Aug 18, 2019 at 10:25 AM Michael Niedermayer 
> wrote:
> 
> > On Sun, Aug 18, 2019 at 09:21:25AM +0200, Paul B Mahol wrote:
> > > NAK
> >
> > What problem do you see in this patch ?
> >
> > What change do you suggest ?
> > or what alternative fix for the issue do you suggest ?
> >
> > a DOS issue in png will have to be fixed, otherwise major
> > users would have to use different libraries for *png and
> > disable it in their libavcodec.
> >
> 
> What other png libraries do this thing?

which libraries do you want me to check ?
libpng seems to support incremental decoding of images so that pixels
"sparkle in" as they are decoded. I dont think it rejects based on a
user parameter.
such a sparkle in feature doesnt exist in libavcodecs API in this
form

Iam not sure how this information above helps us with this patch

We can reject every image that has any pixel missing, this would
break decoding of real files probably which are truncated

We can reject no image based on missing pixels, this would make
the decoder more vulnerable to DOS attacks. 

We can honor the user parameter intended for this purpose and
reject images missing most pixels, this is what the patch does

We can hardcode some arbitrary threshold of how much can be
missing before rejection

We could do something completely different, but i need to know what
exactly is preferred

What does the community prefer ?

Thanks

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

Never trust a computer, one day, it may think you are the virus. -- Compn


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 4/6] avcodec/pngdec: Check amount decoded

2019-08-18 Thread Paul B Mahol 
On Sun, Aug 18, 2019 at 10:25 AM Michael Niedermayer 
wrote:

> On Sun, Aug 18, 2019 at 09:21:25AM +0200, Paul B Mahol wrote:
> > NAK
>
> What problem do you see in this patch ?
>
> What change do you suggest ?
> or what alternative fix for the issue do you suggest ?
>
> a DOS issue in png will have to be fixed, otherwise major
> users would have to use different libraries for *png and
> disable it in their libavcodec.
>

What other png libraries do this thing?


>
> Thanks
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> There will always be a question for which you do not know the correct
> answer.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] avcodec/pngdec: Check amount decoded

2019-08-18 Thread Michael Niedermayer 
On Sun, Aug 18, 2019 at 09:21:25AM +0200, Paul B Mahol wrote:
> NAK

What problem do you see in this patch ?

What change do you suggest ?
or what alternative fix for the issue do you suggest ?

a DOS issue in png will have to be fixed, otherwise major
users would have to use different libraries for *png and
disable it in their libavcodec.

Thanks

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

There will always be a question for which you do not know the correct answer.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 4/6] avcodec/pngdec: Check amount decoded

2019-08-18 Thread Paul B Mahol 
NAK

On Sun, Aug 18, 2019 at 1:36 AM Michael Niedermayer 
wrote:

> Fixes: Timeout (70sec -> 243ms)
> Fixes:
> 16097/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APNG_fuzzer-5664690889293824
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by
> :
> Michael Niedermayer 
> ---
>  libavcodec/pngdec.c | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> index 4ca4f7bdc1..6e6856ab3e 100644
> --- a/libavcodec/pngdec.c
> +++ b/libavcodec/pngdec.c
> @@ -320,6 +320,15 @@ static void deloco_ ## NAME(TYPE *dst, int size, int
> alpha) \
>  YUV2RGB(rgb8, uint8_t)
>  YUV2RGB(rgb16, uint16_t)
>
> +static int percent_missing(PNGDecContext *s)
> +{
> +if (s->interlace_type) {
> +return 100 - 100 * s->pass / (NB_PASSES - 1);
> +} else {
> +return 100 - 100 * s->y / s->cur_h;
> +}
> +}
> +
>  /* process exactly one decompressed row */
>  static void png_handle_row(PNGDecContext *s)
>  {
> @@ -1354,6 +1363,9 @@ exit_loop:
>  return 0;
>  }
>
> +if (percent_missing(s) > avctx->discard_damaged_percentage)
> +return AVERROR_INVALIDDATA;
> +
>  if (s->bits_per_pixel <= 4)
>  handle_small_bpp(s, p);
>
> --
> 2.22.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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/6] avcodec/pngdec: Check amount decoded

2019-08-17 Thread Michael Niedermayer 
Fixes: Timeout (70sec -> 243ms)
Fixes: 
16097/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APNG_fuzzer-5664690889293824

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/pngdec.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 4ca4f7bdc1..6e6856ab3e 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -320,6 +320,15 @@ static void deloco_ ## NAME(TYPE *dst, int size, int 
alpha) \
 YUV2RGB(rgb8, uint8_t)
 YUV2RGB(rgb16, uint16_t)
 
+static int percent_missing(PNGDecContext *s)
+{
+if (s->interlace_type) {
+return 100 - 100 * s->pass / (NB_PASSES - 1);
+} else {
+return 100 - 100 * s->y / s->cur_h;
+}
+}
+
 /* process exactly one decompressed row */
 static void png_handle_row(PNGDecContext *s)
 {
@@ -1354,6 +1363,9 @@ exit_loop:
 return 0;
 }
 
+if (percent_missing(s) > avctx->discard_damaged_percentage)
+return AVERROR_INVALIDDATA;
+
 if (s->bits_per_pixel <= 4)
 handle_small_bpp(s, p);
 
-- 
2.22.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-requ...@ffmpeg.org with subject "unsubscribe".