From d801bd4e6287d13b65a48775ebd43fca350b21d9 Mon Sep 17 00:00:00 2001 From: Sebastian Rasmussen Date: Wed, 4 Sep 2019 01:18:37 +0200 Subject: [PATCH 1/2] openjp2/j2k: Make comments adhere to specification. The function is used to read both SPcod and SPcoc, so all comments should refer to both marker segments' parameter names. --- src/lib/openjp2/j2k.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/lib/openjp2/j2k.c b/src/lib/openjp2/j2k.c index 14f6ff41..59b2bbb7 100644 --- a/src/lib/openjp2/j2k.c +++ b/src/lib/openjp2/j2k.c @@ -9739,9 +9739,9 @@ static OPJ_BOOL opj_j2k_read_SPCod_SPCoc(opj_j2k_t *p_j2k, return OPJ_FALSE; } - opj_read_bytes(l_current_ptr, &l_tccp->numresolutions, - 1); /* SPcox (D) */ - ++l_tccp->numresolutions; /* tccp->numresolutions = read() + 1 */ + /* SPcod (D) / SPcoc (A) */ + opj_read_bytes(l_current_ptr, &l_tccp->numresolutions, 1); + ++l_tccp->numresolutions; /* tccp->numresolutions = read() + 1 */ if (l_tccp->numresolutions > OPJ_J2K_MAXRLVLS) { opj_event_msg(p_manager, EVT_ERROR, "Invalid value for numresolutions : %d, max value is set in openjpeg.h at %d\n", @@ -9762,11 +9762,13 @@ static OPJ_BOOL opj_j2k_read_SPCod_SPCoc(opj_j2k_t *p_j2k, return OPJ_FALSE; } - opj_read_bytes(l_current_ptr, &l_tccp->cblkw, 1); /* SPcoc (E) */ + /* SPcod (E) / SPcoc (B) */ + opj_read_bytes(l_current_ptr, &l_tccp->cblkw, 1); ++l_current_ptr; l_tccp->cblkw += 2; - opj_read_bytes(l_current_ptr, &l_tccp->cblkh, 1); /* SPcoc (F) */ + /* SPcod (F) / SPcoc (C) */ + opj_read_bytes(l_current_ptr, &l_tccp->cblkh, 1); ++l_current_ptr; l_tccp->cblkh += 2; @@ -9777,8 +9779,8 @@ static OPJ_BOOL opj_j2k_read_SPCod_SPCoc(opj_j2k_t *p_j2k, return OPJ_FALSE; } - - opj_read_bytes(l_current_ptr, &l_tccp->cblksty, 1); /* SPcoc (G) */ + /* SPcod (G) / SPcoc (D) */ + opj_read_bytes(l_current_ptr, &l_tccp->cblksty, 1); ++l_current_ptr; if (l_tccp->cblksty & 0xC0U) { /* 2 msb are reserved, assume we can't read */ opj_event_msg(p_manager, EVT_ERROR, @@ -9786,7 +9788,8 @@ static OPJ_BOOL opj_j2k_read_SPCod_SPCoc(opj_j2k_t *p_j2k, return OPJ_FALSE; } - opj_read_bytes(l_current_ptr, &l_tccp->qmfbid, 1); /* SPcoc (H) */ + /* SPcod (H) / SPcoc (E) */ + opj_read_bytes(l_current_ptr, &l_tccp->qmfbid, 1); ++l_current_ptr; *p_header_size = *p_header_size - 5; @@ -9798,8 +9801,9 @@ static OPJ_BOOL opj_j2k_read_SPCod_SPCoc(opj_j2k_t *p_j2k, return OPJ_FALSE; } + /* SPcod (I_i) / SPcoc (F_i) */ for (i = 0; i < l_tccp->numresolutions; ++i) { - opj_read_bytes(l_current_ptr, &l_tmp, 1); /* SPcoc (I_i) */ + opj_read_bytes(l_current_ptr, &l_tmp, 1); ++l_current_ptr; /* Precinct exponent 0 is only allowed for lowest resolution level (Table A.21) */ if ((i != 0) && (((l_tmp & 0xf) == 0) || ((l_tmp >> 4) == 0))) { From f3ee448815eb992b8d4746e32c05e8289f30415f Mon Sep 17 00:00:00 2001 From: Sebastian Rasmussen Date: Wed, 31 Oct 2018 15:56:11 +0100 Subject: [PATCH 2/2] openjp2/j2k: Validate all SGcod/SPcod/SPcoc parameter values. Previously the multiple component transformation SGcod(C) and wavelet transformation SPcod(H)/SPcoc(E) parameter values were never checked, allowing for out of range values. The lack of validation allowed the bit stream provided in issue #1158 through. After this commit an error message points to the marker segments' parameters as being out of range. input/nonregression/edf_c2_20.jp2 contains an SPcod(H) value of 17, but according to Table A-20 of the specification only values 0 and 1 are valid. input/nonregression/issue826.jp2 contains a SGcod(B) value of 2, but according to Table A-17 of the specification only values 0 and 1 are valid. input/nonregression/oss-fuzz2785.jp2 contains a SGcod(B) value of 32, but it is likewise limited to 0 or 1. These test cases have been updated to consistently fail to parse the headers since they contain out of bounds values. This fixes issue #1210. --- src/lib/openjp2/j2k.c | 12 ++++++++++++ tests/nonregression/CMakeLists.txt | 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/lib/openjp2/j2k.c b/src/lib/openjp2/j2k.c index 59b2bbb7..43be7677 100644 --- a/src/lib/openjp2/j2k.c +++ b/src/lib/openjp2/j2k.c @@ -2698,6 +2698,12 @@ static OPJ_BOOL opj_j2k_read_cod(opj_j2k_t *p_j2k, opj_read_bytes(p_header_data, &l_tcp->mct, 1); /* SGcod (C) */ ++p_header_data; + if (l_tcp->mct > 1) { + opj_event_msg(p_manager, EVT_ERROR, + "Invalid multiple component transformation\n"); + return OPJ_FALSE; + } + p_header_size -= 5; for (i = 0; i < l_image->numcomps; ++i) { l_tcp->tccps[i].csty = l_tcp->csty & J2K_CCP_CSTY_PRT; @@ -9792,6 +9798,12 @@ static OPJ_BOOL opj_j2k_read_SPCod_SPCoc(opj_j2k_t *p_j2k, opj_read_bytes(l_current_ptr, &l_tccp->qmfbid, 1); ++l_current_ptr; + if (l_tccp->qmfbid > 1) { + opj_event_msg(p_manager, EVT_ERROR, + "Error reading SPCod SPCoc element, Invalid transformation found\n"); + return OPJ_FALSE; + } + *p_header_size = *p_header_size - 5; /* use custom precinct size ? */ diff --git a/tests/nonregression/CMakeLists.txt b/tests/nonregression/CMakeLists.txt index 82eff3c3..9561fd65 100644 --- a/tests/nonregression/CMakeLists.txt +++ b/tests/nonregression/CMakeLists.txt @@ -34,7 +34,6 @@ set(BLACKLIST_JPEG2000_TMP edf_c2_1178956.jp2 edf_c2_1000290.jp2 #edf_c2_1000691.jp2 # ok - #edf_c2_20.jp2 #looks ok as per kdu_jp2info edf_c2_1377017.jp2 edf_c2_1002767.jp2 edf_c2_10025.jp2 @@ -61,6 +60,7 @@ set(BLACKLIST_JPEG2000 broken2.jp2 broken3.jp2 broken4.jp2 + edf_c2_20.jp2 #may look ok as per kdu_jp2info, but inspection it reveals that the transformation value is out of range gdal_fuzzer_assert_in_opj_j2k_read_SQcd_SQcc.patch.jp2 gdal_fuzzer_check_comp_dx_dy.jp2 gdal_fuzzer_check_number_of_tiles.jp2 @@ -82,6 +82,8 @@ set(BLACKLIST_JPEG2000 issue475.jp2 #kdu_jp2info ok issue413.jp2 #kdu_jp2info ok issue823.jp2 #kdu_jp2info ok + issue826.jp2 #inspection reveales that the transformation value is out of range + oss-fuzz2785.jp2 #inspection reveales that the transformation value is out of range ) file(GLOB_RECURSE OPJ_DATA_NR_LIST