Crashes due to internal bad memory references when using reduce on a truncated file.

[John-Nagle]

Expected behavior and actual behavior.

Decompressing a truncated file at lower resolution should work. In about 1 of 250 tries, it doesn't. The result can be either an error return or a bad memory access.

Possibly related to Issue #1427 , where a similar problem was observed in multi-thread mode.

This was originally discovered when a metaverse client called the Rust crate jpeg2k, which called the Rust foreign function interface crate jpeg2000-sys, which called OpenJPEG, which crashed. Here's the history of that bug, including Valgrind output.

Steps to reproduce the problem.

The problem can be reproduced with opj-decompress under Valgrind, which shows OpenJPEG referencing un-initialized memory.
The problem may be intermittent.

To generate the test-case:

wget -O full_image.j2k http://asset-cdn.glb.agni.lindenlab.com/?texture_id=36b68663-b68d-9923-bf10-0c55c52426b5
dd if=./full_image.j2k of=partial_0_44236.j2k bs=44237 count=1

Here is the command to trigger the uninitialized reads:

OPJ_NUM_THREADS=1 valgrind opj_decompress -allow-partial -i ./partial_0_44236.j2k -o test.png -r 2

Note that the file is 44237 bytes long (http range requests are inclusive on both ends).

Operating system

Ubuntu 22.04 LTS

openjpeg version

OpenJPEG 2.5.0, from Github.

[John-Nagle]

t2.c, line 1150, looks suspicious. Valgrind shows trouble where that value is used.
l_remaining_length = (OPJ_UINT32)(p_src_data + p_max_length - l_header_data);

I'm not sure what's going on in that code, but it seems to me that there should be some kind of check there for going off the end of the input data. Since this occurs with truncated files, that's a likely source of trouble.

[John-Nagle]

I've been looking at opj_t2_read_packet_header in t2.c. I don't see what's wrong, but it's clear that it's very vulnerable to any of those offsets being wrong. I'd suggest adding sanity checks in there.

Valgrind detects problems in both the C and Rust version in this area:

  if (l_cp->ppm == 1) { /* PPM */
        l_header_data_start = &l_cp->ppm_data;
        l_header_data = *l_header_data_start;
        l_modified_length_ptr = &(l_cp->ppm_len);

    } else if (p_tcp->ppt == 1) { /* PPT */
        l_header_data_start = &(p_tcp->ppt_data);
        l_header_data = *l_header_data_start;
        l_modified_length_ptr = &(p_tcp->ppt_len);
    } else { /* Normal Case */
        l_header_data_start = &(l_current_data);
        l_header_data = *l_header_data_start;
        l_remaining_length = (OPJ_UINT32)(p_src_data + p_max_length - l_header_data);
        l_modified_length_ptr = &(l_remaining_length);
    }

Something in there has a reference to un-initialized memory.