subject:"\[FFmpeg\-devel\] \[PATCH 2\/3\] avcodec\/iff\: Check ham vs bpp"

Re: [FFmpeg-devel] [PATCH 2/3] avcodec/iff: Check ham vs bpp

2019-07-08 Thread Michael Niedermayer

On Sun, Jun 23, 2019 at 10:45:56AM +0200, Michael Niedermayer wrote:
> On Sun, Jun 23, 2019 at 06:33:02PM +1000, Peter Ross wrote:
> > On Sun, Jun 23, 2019 at 12:30:54AM +0200, Michael Niedermayer wrote:
> > > This checks the ham value much stricter and avoids hitting cases which 
> > > cannot be reached
> > > with data from the libavformat demuxer.
> > > 
> > > Fixes: out of array access
> > > Fixes: 
> > > 15320/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IFF_ILBM_fuzzer-5080476840099840
> > > Fixes: 
> > > 15423/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IFF_ILBM_fuzzer-5630765833912320
> > > 
> > > Found-by: continuous fuzzing process 
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/iff.c | 15 ---
> > >  1 file changed, 12 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/libavcodec/iff.c b/libavcodec/iff.c
> > > index b43bd507b3..b065bbe598 100644
> > > --- a/libavcodec/iff.c
> > > +++ b/libavcodec/iff.c
> > > @@ -280,6 +280,18 @@ static int extract_header(AVCodecContext *const 
> > > avctx,
> > >  for (i = 0; i < 16; i++)
> > >  s->tvdc[i] = bytestream_get_be16();
> > >  
> > > +if (s->ham) {
> > > +if (s->bpp > 8) {
> > > +av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits 
> > > for HAM: %u\n", s->ham);
> > > +s->ham = 0;
> > > +return AVERROR_INVALIDDATA;
> > > +} if (s->ham != (s->bpp > 6 ? 6 : 4)) {
> > > +av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits 
> > > for HAM: %u, BPP: %u\n", s->ham, s->bpp);
> > > +s->ham = 0;
> > > +return AVERROR_INVALIDDATA;
> > > +}
> > > +}
> > 
> > i am curious why we need to reset ham = 0? otherwise patch okay.
> 
> That was just so as not to leave an invalid value in the context. It is
> very likely not needed, I will remove it.

will apply without the ham = 0


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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 2/3] avcodec/iff: Check ham vs bpp

2019-06-23 Thread Michael Niedermayer

On Sun, Jun 23, 2019 at 06:33:02PM +1000, Peter Ross wrote:
> On Sun, Jun 23, 2019 at 12:30:54AM +0200, Michael Niedermayer wrote:
> > This checks the ham value much stricter and avoids hitting cases which 
> > cannot be reached
> > with data from the libavformat demuxer.
> > 
> > Fixes: out of array access
> > Fixes: 
> > 15320/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IFF_ILBM_fuzzer-5080476840099840
> > Fixes: 
> > 15423/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IFF_ILBM_fuzzer-5630765833912320
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/iff.c | 15 ---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavcodec/iff.c b/libavcodec/iff.c
> > index b43bd507b3..b065bbe598 100644
> > --- a/libavcodec/iff.c
> > +++ b/libavcodec/iff.c
> > @@ -280,6 +280,18 @@ static int extract_header(AVCodecContext *const avctx,
> >  for (i = 0; i < 16; i++)
> >  s->tvdc[i] = bytestream_get_be16();
> >  
> > +if (s->ham) {
> > +if (s->bpp > 8) {
> > +av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits 
> > for HAM: %u\n", s->ham);
> > +s->ham = 0;
> > +return AVERROR_INVALIDDATA;
> > +} if (s->ham != (s->bpp > 6 ? 6 : 4)) {
> > +av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits 
> > for HAM: %u, BPP: %u\n", s->ham, s->bpp);
> > +s->ham = 0;
> > +return AVERROR_INVALIDDATA;
> > +}
> > +}
> 
> i am curious why we need to reset ham = 0? otherwise patch okay.

That was just so as not to leave an invalid value in the context. It is
very likely not needed, I will remove it.


> 
> patch 1 and 3 okay.

will apply

thx


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

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin


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 2/3] avcodec/iff: Check ham vs bpp

2019-06-23 Thread Peter Ross

On Sun, Jun 23, 2019 at 12:30:54AM +0200, Michael Niedermayer wrote:
> This checks the ham value much stricter and avoids hitting cases which cannot 
> be reached
> with data from the libavformat demuxer.
> 
> Fixes: out of array access
> Fixes: 
> 15320/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IFF_ILBM_fuzzer-5080476840099840
> Fixes: 
> 15423/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IFF_ILBM_fuzzer-5630765833912320
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/iff.c | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/iff.c b/libavcodec/iff.c
> index b43bd507b3..b065bbe598 100644
> --- a/libavcodec/iff.c
> +++ b/libavcodec/iff.c
> @@ -280,6 +280,18 @@ static int extract_header(AVCodecContext *const avctx,
>  for (i = 0; i < 16; i++)
>  s->tvdc[i] = bytestream_get_be16();
>  
> +if (s->ham) {
> +if (s->bpp > 8) {
> +av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits for 
> HAM: %u\n", s->ham);
> +s->ham = 0;
> +return AVERROR_INVALIDDATA;
> +} if (s->ham != (s->bpp > 6 ? 6 : 4)) {
> +av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits for 
> HAM: %u, BPP: %u\n", s->ham, s->bpp);
> +s->ham = 0;
> +return AVERROR_INVALIDDATA;
> +}
> +}

i am curious why we need to reset ham = 0? otherwise patch okay.

patch 1 and 3 okay.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)


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".

[FFmpeg-devel] [PATCH 2/3] avcodec/iff: Check ham vs bpp

2019-06-22 Thread Michael Niedermayer

This checks the ham value much stricter and avoids hitting cases which cannot 
be reached
with data from the libavformat demuxer.

Fixes: out of array access
Fixes: 
15320/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IFF_ILBM_fuzzer-5080476840099840
Fixes: 
15423/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IFF_ILBM_fuzzer-5630765833912320

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

diff --git a/libavcodec/iff.c b/libavcodec/iff.c
index b43bd507b3..b065bbe598 100644
--- a/libavcodec/iff.c
+++ b/libavcodec/iff.c
@@ -280,6 +280,18 @@ static int extract_header(AVCodecContext *const avctx,
 for (i = 0; i < 16; i++)
 s->tvdc[i] = bytestream_get_be16();
 
+if (s->ham) {
+if (s->bpp > 8) {
+av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits for 
HAM: %u\n", s->ham);
+s->ham = 0;
+return AVERROR_INVALIDDATA;
+} if (s->ham != (s->bpp > 6 ? 6 : 4)) {
+av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits for 
HAM: %u, BPP: %u\n", s->ham, s->bpp);
+s->ham = 0;
+return AVERROR_INVALIDDATA;
+}
+}
+
 if (s->masking == MASK_HAS_MASK) {
 if (s->bpp >= 8 && !s->ham) {
 avctx->pix_fmt = AV_PIX_FMT_RGB32;
@@ -307,9 +319,6 @@ static int extract_header(AVCodecContext *const avctx,
 if (!s->bpp || s->bpp > 32) {
 av_log(avctx, AV_LOG_ERROR, "Invalid number of bitplanes: %u\n", 
s->bpp);
 return AVERROR_INVALIDDATA;
-} else if (s->ham >= 8) {
-av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits for HAM: 
%u\n", s->ham);
-return AVERROR_INVALIDDATA;
 }
 
 av_freep(>ham_buf);
-- 
2.22.0

___
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 2/3] avcodec/iff: Check ham vs bpp

Re: [FFmpeg-devel] [PATCH 2/3] avcodec/iff: Check ham vs bpp

Re: [FFmpeg-devel] [PATCH 2/3] avcodec/iff: Check ham vs bpp

[FFmpeg-devel] [PATCH 2/3] avcodec/iff: Check ham vs bpp

4 matches

Site Navigation

Mail list logo

Footer information