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

[Michael Niedermayer]

On Tue, Sep 05, 2017 at 01:16:22PM +0200, Paul B Mahol wrote:
> On 9/5/17, Michael Niedermayer <michael at niedermayer.cc> wrote:
> > 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
> 
> FFmpeg is not meant to have every log for every error return.
> 
> COD/COC sounds funny.

COD/COC is how its called in most of the jpeg2000 spec
I can write the full terms out of course
"Coding style component" and "Coding style default"


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"Nothing to hide" only works if the folks in power share the values of
you and everyone you know entirely and always will -- Tom Scott

-------------- 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/204ad716/attachment.sig>