Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opj_t1_ht_decode_cblk(): avoid out-of-bounds read on ds0_ht_02_b11.j2k #2

Merged

Conversation

rouault
Copy link

@rouault rouault commented Sep 23, 2021

Avoids the following issue:

$ valgrind bin/opj_decompress -i ~/OpenHTJ2K/conformance_data/ds0_ht_02_b11.j2k -o out.ppm -threads 0

==4037690== Invalid read of size 1
==4037690== at 0x48589FA: opj_t1_ht_decode_cblk (fbc_dec.c:1262)
==4037690== by 0x48B28E5: opj_t1_clbl_decode_processor (t1.c:1690)
==4037690== by 0x4854A33: opj_thread_pool_submit_job (thread.c:835)
==4037690== by 0x48B37C3: opj_t1_decode_cblks (t1.c:1943)
==4037690== by 0x48BD668: opj_tcd_t1_decode (tcd.c:2000)
==4037690== by 0x48BCADF: opj_tcd_decode_tile (tcd.c:1654)
==4037690== by 0x487D348: opj_j2k_decode_tile (j2k.c:9759)
==4037690== by 0x4881CDA: opj_j2k_decode_tiles (j2k.c:11566)
==4037690== by 0x487B333: opj_j2k_exec (j2k.c:8903)
==4037690== by 0x4882AD1: opj_j2k_decode (j2k.c:11912)
==4037690== by 0x488EF5C: opj_decode (openjpeg.c:494)
==4037690== by 0x1103AC: main (opj_decompress.c:1547)
==4037690== Address 0x52884ef is 1 bytes before a block of size 2 alloc'd
==4037690== at 0x483B723: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==4037690== by 0x483E017: realloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==4037690== by 0x48C0676: opj_realloc (opj_malloc.c:244)
==4037690== by 0x48584E6: opj_t1_ht_decode_cblk (fbc_dec.c:1123)
==4037690== by 0x48B28E5: opj_t1_clbl_decode_processor (t1.c:1690)
==4037690== by 0x4854A33: opj_thread_pool_submit_job (thread.c:835)
==4037690== by 0x48B37C3: opj_t1_decode_cblks (t1.c:1943)
==4037690== by 0x48BD668: opj_tcd_t1_decode (tcd.c:2000)
==4037690== by 0x48BCADF: opj_tcd_decode_tile (tcd.c:1654)
==4037690== by 0x487D348: opj_j2k_decode_tile (j2k.c:9759)
==4037690== by 0x4881CDA: opj_j2k_decode_tiles (j2k.c:11566)
==4037690== by 0x487B333: opj_j2k_exec (j2k.c:8903)

I've also simplified a bit the allocation of the concatenated code block
buffer, to remove the OPJ_COMMON_CBLK_DATA_EXTRA that I believe is a trick only
needed for regular code block decoding, not HT.

Avoids the following issue:

$ valgrind bin/opj_decompress -i ~/OpenHTJ2K/conformance_data/ds0_ht_02_b11.j2k -o out.ppm -threads 0

==4037690== Invalid read of size 1
==4037690==    at 0x48589FA: opj_t1_ht_decode_cblk (fbc_dec.c:1262)
==4037690==    by 0x48B28E5: opj_t1_clbl_decode_processor (t1.c:1690)
==4037690==    by 0x4854A33: opj_thread_pool_submit_job (thread.c:835)
==4037690==    by 0x48B37C3: opj_t1_decode_cblks (t1.c:1943)
==4037690==    by 0x48BD668: opj_tcd_t1_decode (tcd.c:2000)
==4037690==    by 0x48BCADF: opj_tcd_decode_tile (tcd.c:1654)
==4037690==    by 0x487D348: opj_j2k_decode_tile (j2k.c:9759)
==4037690==    by 0x4881CDA: opj_j2k_decode_tiles (j2k.c:11566)
==4037690==    by 0x487B333: opj_j2k_exec (j2k.c:8903)
==4037690==    by 0x4882AD1: opj_j2k_decode (j2k.c:11912)
==4037690==    by 0x488EF5C: opj_decode (openjpeg.c:494)
==4037690==    by 0x1103AC: main (opj_decompress.c:1547)
==4037690==  Address 0x52884ef is 1 bytes before a block of size 2 alloc'd
==4037690==    at 0x483B723: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==4037690==    by 0x483E017: realloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==4037690==    by 0x48C0676: opj_realloc (opj_malloc.c:244)
==4037690==    by 0x48584E6: opj_t1_ht_decode_cblk (fbc_dec.c:1123)
==4037690==    by 0x48B28E5: opj_t1_clbl_decode_processor (t1.c:1690)
==4037690==    by 0x4854A33: opj_thread_pool_submit_job (thread.c:835)
==4037690==    by 0x48B37C3: opj_t1_decode_cblks (t1.c:1943)
==4037690==    by 0x48BD668: opj_tcd_t1_decode (tcd.c:2000)
==4037690==    by 0x48BCADF: opj_tcd_decode_tile (tcd.c:1654)
==4037690==    by 0x487D348: opj_j2k_decode_tile (j2k.c:9759)
==4037690==    by 0x4881CDA: opj_j2k_decode_tiles (j2k.c:11566)
==4037690==    by 0x487B333: opj_j2k_exec (j2k.c:8903)

I've also simplified a bit the allocation of the concatenated code block
buffer, to remove the OPJ_COMMON_CBLK_DATA_EXTRA that I believe is a trick only
needed for regular code block decoding, not HT.
@aous72
Copy link
Owner

aous72 commented Sep 24, 2021

I agree that this is an important issue and has to be addressed. These issues did not come to my mind earlier. Good that you spotted them.

I think there might be a better option of achieving the same outcome. For the sake of documentation, a problem can occur if the

  1. lengths1 < 2.
  2. lcup > cblk_len.
  3. lengths1 + lengths2 > cblk_len.

I hope these are all the cases.

@aous72 aous72 merged commit a219d5d into aous72:master Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants