Found code similar to CVE-2016-7163

[RongxiYe]

Source code

In the fix for CVE-2016-7163 (issue https://github.com/uclouvain/openjpeg/issues/826), l_current_pi->include_size in the opj_pi_create_decode function gained a check to see if the multiplication caused an integer overflow.

/* step calculations */
    l_step_p = 1;
    l_step_c = l_max_prec * l_step_p;
    l_step_r = numcomps * l_step_c;
    l_step_l = l_max_res * l_step_r;

    /* set values for first packet iterator */
    l_current_pi = l_pi;

    /* memory allocation for include */
    /* prevent an integer overflow issue */
    /* 0 < l_tcp->numlayers < 65536 c.f. opj_j2k_read_cod in j2k.c */
    l_current_pi->include = 00;
->  if (l_step_l <= (UINT_MAX / (l_tcp->numlayers + 1U))) {
        l_current_pi->include_size = (l_tcp->numlayers + 1U) * l_step_l;
        l_current_pi->include = (OPJ_INT16*) opj_calloc(
                                    l_current_pi->include_size, sizeof(OPJ_INT16));
    }

However, in the opj_pi_initialise_encode function in the encoding part (in the same file, src/lib/openjp2/pi.c), there is exactly the same code logic, but it does not perform the check.

/* step calculations*/
    l_step_p = 1;
    l_step_c = l_max_prec * l_step_p;
    l_step_r = numcomps * l_step_c;
    l_step_l = l_max_res * l_step_r;

    /* set values for first packet iterator*/
    l_pi->tp_on = (OPJ_BYTE)p_cp->m_specific_param.m_enc.m_tp_on;
    l_current_pi = l_pi;

    /* memory allocation for include*/
    l_current_pi->include_size = l_tcp->numlayers * l_step_l;
    l_current_pi->include = (OPJ_INT16*) opj_calloc(l_current_pi->include_size,
                            sizeof(OPJ_INT16));

Should the same check be here? By the way, the two operands of the multiplication in the encoding part and the decoding part can be specified externally in the same way.

Assembly Code

In fact, the overflow is because the operands to be multiplied are all 32 bits, so the multiplication result is stored in a 32-bit register in the assembly code. Even if it is expanded to size_t during calloc, it does not change the fact that the multiplication has overflowed.

Possible Fix

Same as CVE-2016-7163.

Version

Latest version