decode: block decoder generates 34 bit left shift about openjph HOT 4 CLOSED

Hi Aaron,

Good to see you still looking at the code, or perhaps developing your own.

I looked at this issue, and OpenJPH throws an exception for an error in the packet header on line 3462 of ojph_codestream.cpp. Kakadu's kdu_expand also throws an error about an illegal length in the header "Illegal length parameter found while parsing HT code-block contribution to Part-15 packet header". The issue is that the number of missing msbs is larger then K_max, which is illegal -- I am a bit blurry about this now.

If I were to disable this, line 1027 of block_decoder.cpp captures an error;

if (scup < 2 || scup > lcup || scup > 4079) //something is wrong
        return false;

because scup > lcup, which is illegal.

If I were to disable this, then I get an access error (seg. fault).

If you look at the area around 1202 of block_decoder, then the only way to have large m_n is if you have large U_q[0], but an error is generated when U_q[0] > missing_msbs; therefore, the only way to have a large m_n is if the missing_msbs is too large, but this is captured on line 1014

if (missing_msbs > 29) // p < 1
        return false;

Because of this, the highest m_n can be is 29, unless there is data corruption that changes missing_msbs.

Please let me know how this goes.

Cheers,
Aous.

from openjph.

Hi Aous,
Thank you for taking a look at the code.
This test file comes from the oss-fuzz fuzzer, by the way.

I think I found the bug:

        if (U_q[0] > missing_msbs && U_q[1] > missing_msbs)
          return false;

should read

        if (U_q[0] > missing_msbs || U_q[1] > missing_msbs)
          return false;

What do you think?

Cheers,
Aaron

from openjph.

Hi Aaron,

You are absolutely right. Thank you for discovering this bug.

I pushed the corrections.

oss-fuzzer seems like a lot of work, what are your thoughts.

Kind regards,
Aous.

from openjph.

Great, thank you.
oss-fuzz is a bit of work at the beginning, but it is well worth the effort.
Especially for codecs, which can have large attack surfaces.
I can help you set it up, if you're interested. It helps to have a simple C API for the decode functionality,
so the fuzzer can control the decode instead of the command line binary.
The C API will be good to have anyways, for people who want to integrate the library into their own code.

from openjph.