[FFmpeg-devel] [PATCH] avcodec/jpeg2000: Check that codsty->log2_prec_widths/heights has been initialized

[Michael Niedermayer]

Hi

On Mon, Sep 04, 2017 at 06:45:02PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Mon, Sep 4, 2017 at 6:04 PM, Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > Fixes: OOM
> > Fixes: 2225/clusterfuzz-testcase-minimized-5505632079708160
> >
> > 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/jpeg2000.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/libavcodec/jpeg2000.c b/libavcodec/jpeg2000.c
> > index 94efc94c4d..9e1bbc2ec4 100644
> > --- a/libavcodec/jpeg2000.c
> > +++ b/libavcodec/jpeg2000.c
> > @@ -506,6 +506,10 @@ int ff_jpeg2000_init_component(Jpeg2000Component
> > *comp,
> >          // update precincts size: 2^n value
> >          reslevel->log2_prec_width  = codsty->log2_prec_widths[
> > reslevelno];
> >          reslevel->log2_prec_height = codsty->log2_prec_heights[
> > reslevelno];
> > +        if (!reslevel->log2_prec_width || !reslevel->log2_prec_height) {
> > +            av_log(avctx, AV_LOG_ERROR, "COD/COC is missing\n");
> > +            return AVERROR_INVALIDDATA;
> > +        }
> 
> 
> Please change it to ff_tlog().

that would make the message unavailable to the user, so the user
would not know why a decoding failure occured.

It would also make it unavailable in bug reports as the message is
not in the compiled binary. Even at highest verbosity and debug levels
it would not show up not even with debug builds. Only in special trace
builds would it show up.

Users would not be able to find existing bug reports based on the error
message, would not be able to google it, would not be able to refer to
it in a specific way "a issue with missing COC/COD".

This is not a obscure detail of bitstream parsing, its a error in
the headers that will lead to the loss of a frame.

Lets also look at what other software does
picking lena converted to jpeg2000 and a damaged COD with a hex editor

    j2k_to_image -i lena-noco.jp2 -o image.pgm

    [ERROR] Error decoding component 0.
    The number of resolutions is too big: 256 vs max= 33. Truncating.

    [ERROR] Error decoding component 1.
    The number of resolutions is too big: 256 vs max= 33. Truncating.

    [ERROR] Error decoding component 2.
    The number of resolutions is too big: 256 vs max= 33. Truncating.

    [ERROR] Failed to decode J2K image
    ERROR -> j2k_to_image: failed to decode image!

You can see openjpeg shows detailed error messages
Lets try the clusterfuzz testcase directly:

    j2k_to_image -i clusterfuzz-testcase-minimized-5505632079708160.jp2 -o image.pnm

    [ERROR] Integer overflow in box->length
    [ERROR] Failed to read boxhdr
    [ERROR] Failed to decode jp2 structure
    ERROR -> j2k_to_image: failed to decode image!

again, a detailed error message

lets try jasper

    jasper --input lena-noco.jp2 --output file.pnm
    cannot get marker segment
    error: cannot decode code stream
    error: cannot load image data

and the  testcase directly:
    jasper --input clusterfuzz-testcase-minimized-5505632079708160 --output image.pnm
    cannot get marker segment
    error: cannot load image data

and jasper also shows more than just a generic error

Thats by default. no debug build, no trace build, no verbosity, no
debug options.

just for completeness lets run jasper with debug level 99
jasper --debug-level 99 --input clusterfuzz-testcase-minimized-5505632079708160.jp2 --output image.pnm
    type = 0xff4f (SOC);
    type = 0xff51 (SIZ); len = 41;caps = 0x2020;
    width = 25632; height = 32; xoff = 0; yoff = 0;
    tilewidth = 538976288; tileheight = 538976288; tilexoff = 0; tileyoff = 0;
    prec[0] = 8; sgnd[0] = 0; hsamp[0] = 1; vsamp[0] = 1
    type = 0xff52 (COD); len = 13;csty = 0x01;
    numdlvls = 0; qmfbid = 0; mctrans = 0
    prg = 32; numlyrs = 8224;
    cblkwidthval = 32; cblkheightval = 32; cblksty = 0x20;
    prcwidth[0] = 0, prcheight[0] = 0
    type = 0xff90 (SOT); len = 10;tileno = 0; len = 0; partno = 0; numparts = 32
    cannot get marker segment
    error: cannot load image data

You can again see, theres lots of details, which may be critically
important in a bug report.

More so users may have bug report samples that are not sharable
for all kinds of contractual reasons. Having detailed information
available is the only chance to debug such issues.
Requiring the user to build his own binary FFmpeg with custom build
flags is a large hurdle for reporting a bug

Thanks

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

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170905/10d70dac/attachment.sig>