[FFmpeg-devel] [PATCH 4/4] avcodec/hqx: Check the input data against the image size

[Michael Niedermayer]

On Tue, Jul 23, 2019 at 09:52:59AM +0200, Paul B Mahol wrote:
> On 7/23/19, Michael Niedermayer <michael at niedermayer.cc> wrote:
> > On Tue, Jul 23, 2019 at 09:03:32AM +0200, Paul B Mahol wrote:
> >> On 7/23/19, Michael Niedermayer <michael at niedermayer.cc> wrote:
> >> > On Mon, Jul 22, 2019 at 08:20:54AM +0200, Paul B Mahol wrote:
> >> >> On 7/21/19, Michael Niedermayer <michael at niedermayer.cc> wrote:
> >> >> > On Sun, Jul 21, 2019 at 10:48:26AM +0200, Paul B Mahol wrote:
> >> >> >> On 7/21/19, Michael Niedermayer <michael at niedermayer.cc> wrote:
> >> >> >> > Fixes: Timeout (22 -> 7 sec)
> >> >> >> > Fixes:
> >> >> >> > 15173/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HQX_fuzzer-5662556846292992
> >> >> >> >
> >> >> >> > 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/hqx.c | 4 ++++
> >> >> >> >  1 file changed, 4 insertions(+)
> >> >> >> >
> >> >> >> > diff --git a/libavcodec/hqx.c b/libavcodec/hqx.c
> >> >> >> > index bc24ba91d1..8639d77a41 100644
> >> >> >> > --- a/libavcodec/hqx.c
> >> >> >> > +++ b/libavcodec/hqx.c
> >> >> >> > @@ -471,6 +471,10 @@ static int hqx_decode_frame(AVCodecContext
> >> >> >> > *avctx,
> >> >> >> > void
> >> >> >> > *data,
> >> >> >> >      avctx->height              = ctx->height;
> >> >> >> >      avctx->bits_per_raw_sample = 10;
> >> >> >> >
> >> >> >> > +    if (avctx->coded_width / 16 * (avctx->coded_height / 16) *
> >> >> >> > +        (100 - avctx->discard_damaged_percentage) / 100 > 8LL *
> >> >> >> > avpkt->size)
> >> >> >> > +        return AVERROR_INVALIDDATA;
> >> >> >>
> >> >> >> Why just this change and not something better?
> >> >> >
> >> >> > What would you prefer exactly ?
> >> >>
> >> >> Something that works with pure black video.
> >> >
> >> > Can you share the failing video file ?
> >> > I thought theres a minimum size of 1 vlc code (2 bit seem the smallest)
> >> > per 16x16 block. But quite possibly i might have missed something
> >> >
> >>
> >> This is very disappointing. There is no freely available encoder for HQX.
> >> And the one who commits stuff need to make sure it does not introduce
> >> regressions.
> >
> > The reviewer just has to explain how the problem he speaks of can
> > occur.
> 
> No, its other way around.
> The patch author just has to explain how the problem he tries to solve
> is valid solution by given patch.

I have explained that in the very mail you just replied to.

anyway, no problem ill find a way to encode a pure black hqx video
or will otherwise do a more complete proof of correctness.
(or of course if an issue is found by doing so improve the patch)

I just thought if you know of a issue and told me what it is exactly
that would be much simpler and quicker.

Thanks


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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- 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/20190723/8a522c67/attachment.sig>