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

[Paul B Mahol]

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.

Thanks