From fccfe0286b93a9f730f2815bd60c9a248cd4c8f4 Mon Sep 17 00:00:00 2001 From: Sebastian Rasmussen Date: Mon, 12 Jul 2021 03:38:26 +0200 Subject: [PATCH] opj_j2k_is_imf_compliant: Fix out of bounds access. Previously when mainlevel was parsed == 12 openjpeg would generate a warning, but then the sublevel value would be compared to an out of bounds element in the tabMaxSubLevelFromMainLevel array. Judging by the code, what commit 84f3bebbff515f2b00ccf0c817930ebb10b91760 probably meant to do was to use early return whenever a non-IMF compliant stream was detected. This commit uses that approach, thus avoiding the out of bounds access after the initial warning. --- src/lib/openjp2/j2k.c | 77 +++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/src/lib/openjp2/j2k.c b/src/lib/openjp2/j2k.c index c3696edbf..b197a947d 100644 --- a/src/lib/openjp2/j2k.c +++ b/src/lib/openjp2/j2k.c @@ -7090,7 +7090,6 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, const int NL = parameters->numresolution - 1; const OPJ_UINT32 XTsiz = parameters->tile_size_on ? (OPJ_UINT32) parameters->cp_tdx : image->x1; - OPJ_BOOL ret = OPJ_TRUE; /* Validate mainlevel */ if (mainlevel > OPJ_IMF_MAINLEVEL_MAX) { @@ -7099,7 +7098,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> %d is thus not compliant\n" "-> Non-IMF codestream will be generated\n", mainlevel); - ret = OPJ_FALSE; + return OPJ_FALSE; } /* Validate sublevel */ @@ -7113,7 +7112,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, tabMaxSubLevelFromMainLevel[mainlevel], mainlevel, sublevel); - ret = OPJ_FALSE; + return OPJ_FALSE; } /* Number of components */ @@ -7123,7 +7122,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of components of input image (%d) is not compliant\n" "-> Non-IMF codestream will be generated\n", image->numcomps); - ret = OPJ_FALSE; + return OPJ_FALSE; } if (image->x0 != 0 || image->y0 != 0) { @@ -7132,7 +7131,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> %d,%d is not compliant\n" "-> Non-IMF codestream will be generated\n", image->x0, image->y0 != 0); - ret = OPJ_FALSE; + return OPJ_FALSE; } if (parameters->cp_tx0 != 0 || parameters->cp_ty0 != 0) { @@ -7141,7 +7140,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> %d,%d is not compliant\n" "-> Non-IMF codestream will be generated\n", parameters->cp_tx0, parameters->cp_ty0); - ret = OPJ_FALSE; + return OPJ_FALSE; } if (parameters->tile_size_on) { @@ -7158,7 +7157,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, parameters->cp_tdy, image->x1, image->y1); - ret = OPJ_FALSE; + return OPJ_FALSE; } } else { if ((OPJ_UINT32)parameters->cp_tdx >= image->x1 && @@ -7186,7 +7185,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Non-IMF codestream will be generated\n", parameters->cp_tdx, parameters->cp_tdy); - ret = OPJ_FALSE; + return OPJ_FALSE; } } } @@ -7203,7 +7202,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> At least component %d of input image (%d bits, %s) is not compliant\n" "-> Non-IMF codestream will be generated\n", i, image->comps[i].bpp, tmp_str); - ret = OPJ_FALSE; + return OPJ_FALSE; } } @@ -7214,14 +7213,14 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "IMF profiles require XRSiz1 == 1. Here it is set to %d.\n" "-> Non-IMF codestream will be generated\n", image->comps[i].dx); - ret = OPJ_FALSE; + return OPJ_FALSE; } if (i == 1 && image->comps[i].dx != 1 && image->comps[i].dx != 2) { opj_event_msg(p_manager, EVT_WARNING, "IMF profiles require XRSiz2 == 1 or 2. Here it is set to %d.\n" "-> Non-IMF codestream will be generated\n", image->comps[i].dx); - ret = OPJ_FALSE; + return OPJ_FALSE; } if (i > 1 && image->comps[i].dx != image->comps[i - 1].dx) { opj_event_msg(p_manager, EVT_WARNING, @@ -7229,7 +7228,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "Here it is set to %d instead of %d.\n" "-> Non-IMF codestream will be generated\n", i + 1, image->comps[i].dx, image->comps[i - 1].dx); - ret = OPJ_FALSE; + return OPJ_FALSE; } if (image->comps[i].dy != 1) { opj_event_msg(p_manager, EVT_WARNING, @@ -7237,7 +7236,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "Here it is set to %d for component %d.\n" "-> Non-IMF codestream will be generated\n", image->comps[i].dy, i); - ret = OPJ_FALSE; + return OPJ_FALSE; } } @@ -7252,7 +7251,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Input image size %d x %d is not compliant\n" "-> Non-IMF codestream will be generated\n", image->comps[0].w, image->comps[0].h); - ret = OPJ_FALSE; + return OPJ_FALSE; } break; case OPJ_PROFILE_IMF_4K: @@ -7264,7 +7263,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Input image size %d x %d is not compliant\n" "-> Non-IMF codestream will be generated\n", image->comps[0].w, image->comps[0].h); - ret = OPJ_FALSE; + return OPJ_FALSE; } break; case OPJ_PROFILE_IMF_8K: @@ -7276,7 +7275,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Input image size %d x %d is not compliant\n" "-> Non-IMF codestream will be generated\n", image->comps[0].w, image->comps[0].h); - ret = OPJ_FALSE; + return OPJ_FALSE; } break; default : @@ -7289,7 +7288,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "IMF profile forbid RGN / region of interest marker.\n" "-> Compression parameters specify a ROI\n" "-> Non-IMF codestream will be generated\n"); - ret = OPJ_FALSE; + return OPJ_FALSE; } if (parameters->cblockw_init != 32 || parameters->cblockh_init != 32) { @@ -7299,7 +7298,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Non-IMF codestream will be generated\n", parameters->cblockw_init, parameters->cblockh_init); - ret = OPJ_FALSE; + return OPJ_FALSE; } if (parameters->prog_order != OPJ_CPRL) { @@ -7308,7 +7307,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Compression parameters set it to %d.\n" "-> Non-IMF codestream will be generated\n", parameters->prog_order); - ret = OPJ_FALSE; + return OPJ_FALSE; } if (parameters->numpocs != 0) { @@ -7317,7 +7316,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Compression parameters set %d POC.\n" "-> Non-IMF codestream will be generated\n", parameters->numpocs); - ret = OPJ_FALSE; + return OPJ_FALSE; } /* Codeblock style: no mode switch enabled */ @@ -7327,7 +7326,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Compression parameters set code block style to %d.\n" "-> Non-IMF codestream will be generated\n", parameters->mode); - ret = OPJ_FALSE; + return OPJ_FALSE; } if (profile == OPJ_PROFILE_IMF_2K || @@ -7339,7 +7338,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "IMF 2K/4K/8K profiles require 9-7 Irreversible Transform.\n" "-> Compression parameters set it to reversible.\n" "-> Non-IMF codestream will be generated\n"); - ret = OPJ_FALSE; + return OPJ_FALSE; } } else { /* Expect 5-3 transform */ @@ -7348,7 +7347,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "IMF 2K/4K/8K profiles require 5-3 reversible Transform.\n" "-> Compression parameters set it to irreversible.\n" "-> Non-IMF codestream will be generated\n"); - ret = OPJ_FALSE; + return OPJ_FALSE; } } @@ -7359,7 +7358,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of layers is %d.\n" "-> Non-IMF codestream will be generated\n", parameters->tcp_numlayers); - ret = OPJ_FALSE; + return OPJ_FALSE; } /* Decomposition levels */ @@ -7371,7 +7370,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of decomposition levels is %d.\n" "-> Non-IMF codestream will be generated\n", NL); - ret = OPJ_FALSE; + return OPJ_FALSE; } break; case OPJ_PROFILE_IMF_4K: @@ -7381,7 +7380,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of decomposition levels is %d.\n" "-> Non-IMF codestream will be generated\n", NL); - ret = OPJ_FALSE; + return OPJ_FALSE; } break; case OPJ_PROFILE_IMF_8K: @@ -7391,7 +7390,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of decomposition levels is %d.\n" "-> Non-IMF codestream will be generated\n", NL); - ret = OPJ_FALSE; + return OPJ_FALSE; } break; case OPJ_PROFILE_IMF_2K_R: { @@ -7402,7 +7401,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of decomposition levels is %d.\n" "-> Non-IMF codestream will be generated\n", NL); - ret = OPJ_FALSE; + return OPJ_FALSE; } } else if (XTsiz >= 1024) { if (!(NL >= 1 && NL <= 4)) { @@ -7411,7 +7410,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of decomposition levels is %d.\n" "-> Non-IMF codestream will be generated\n", NL); - ret = OPJ_FALSE; + return OPJ_FALSE; } } break; @@ -7424,7 +7423,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of decomposition levels is %d.\n" "-> Non-IMF codestream will be generated\n", NL); - ret = OPJ_FALSE; + return OPJ_FALSE; } } else if (XTsiz >= 2048) { if (!(NL >= 1 && NL <= 5)) { @@ -7433,7 +7432,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of decomposition levels is %d.\n" "-> Non-IMF codestream will be generated\n", NL); - ret = OPJ_FALSE; + return OPJ_FALSE; } } else if (XTsiz >= 1024) { if (!(NL >= 1 && NL <= 4)) { @@ -7442,7 +7441,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of decomposition levels is %d.\n" "-> Non-IMF codestream will be generated\n", NL); - ret = OPJ_FALSE; + return OPJ_FALSE; } } break; @@ -7455,7 +7454,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of decomposition levels is %d.\n" "-> Non-IMF codestream will be generated\n", NL); - ret = OPJ_FALSE; + return OPJ_FALSE; } } else if (XTsiz >= 4096) { if (!(NL >= 1 && NL <= 6)) { @@ -7464,7 +7463,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of decomposition levels is %d.\n" "-> Non-IMF codestream will be generated\n", NL); - ret = OPJ_FALSE; + return OPJ_FALSE; } } else if (XTsiz >= 2048) { if (!(NL >= 1 && NL <= 5)) { @@ -7473,7 +7472,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of decomposition levels is %d.\n" "-> Non-IMF codestream will be generated\n", NL); - ret = OPJ_FALSE; + return OPJ_FALSE; } } else if (XTsiz >= 1024) { if (!(NL >= 1 && NL <= 4)) { @@ -7482,7 +7481,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "-> Number of decomposition levels is %d.\n" "-> Non-IMF codestream will be generated\n", NL); - ret = OPJ_FALSE; + return OPJ_FALSE; } } break; @@ -7499,7 +7498,7 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "IMF profiles require PPx = PPy = 7 for NLLL band, else 8.\n" "-> Supplied values are different from that.\n" "-> Non-IMF codestream will be generated\n"); - ret = OPJ_FALSE; + return OPJ_FALSE; } } else { int i; @@ -7510,12 +7509,12 @@ static OPJ_BOOL opj_j2k_is_imf_compliant(opj_cparameters_t *parameters, "IMF profiles require PPx = PPy = 7 for NLLL band, else 8.\n" "-> Supplied values are different from that.\n" "-> Non-IMF codestream will be generated\n"); - ret = OPJ_FALSE; + return OPJ_FALSE; } } } - return ret; + return OPJ_TRUE; }