CVE-2018-21010: Out-of-bounds Write (CWE-787)
value: OpenJPEG before 2.3.1 has a heap buffer overflow in color_apply_icc_profile in bin/common/color.c.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 8.8
Published: 2019-09-05T13:15Z
Repository: https://github.com/uclouvain/openjpeg
Language: C
Files changed: src/bin/common/color.c
Commit date: 2018-11-27 23:31:30 +0100
Commit message: color_apply_icc_profile: avoid potential heap buffer overflow

Derived from a patch by Thuan Pham

Diff: diff --git a/src/bin/common/color.c b/src/bin/common/color.c
index a97d49f12..d3a2f38d7 100644
--- a/src/bin/common/color.c
+++ b/src/bin/common/color.c
@@ -595,86 +595,96 @@ void color_apply_icc_profile(opj_image_t *image)
 #endif
         return;
     }
 
     if (image->numcomps > 2) { /* RGB, RGBA */
-        if (prec <= 8) {
-            unsigned char *inbuf, *outbuf, *in, *out;
-
-            max = max_w * max_h;
-            nr_samples = (size_t)(max * 3U * sizeof(unsigned char));
-            in = inbuf = (unsigned char*)opj_image_data_alloc(nr_samples);
-            out = outbuf = (unsigned char*)opj_image_data_alloc(nr_samples);
-
-            if (inbuf == NULL || outbuf == NULL) {
-                goto fails0;
-            }
-
-            r = image->comps[0].data;
-            g = image->comps[1].data;
-            b = image->comps[2].data;
-
-            for (i = 0U; i < max; ++i) {
-                *in++ = (unsigned char) * r++;
-                *in++ = (unsigned char) * g++;
-                *in++ = (unsigned char) * b++;
-            }
-
-            cmsDoTransform(transform, inbuf, outbuf, (cmsUInt32Number)max);
-
-            r = image->comps[0].data;
-            g = image->comps[1].data;
-            b = image->comps[2].data;
-
-            for (i = 0U; i < max; ++i) {
-                *r++ = (int) * out++;
-                *g++ = (int) * out++;
-                *b++ = (int) * out++;
-            }
-            ok = 1;
+        if ((image->comps[0].w == image->comps[1].w &&
+                image->comps[0].w == image->comps[2].w) &&
+                (image->comps[0].h == image->comps[1].h &&
+                 image->comps[0].h == image->comps[2].h)) {
+            if (prec <= 8) {
+                unsigned char *inbuf, *outbuf, *in, *out;
+
+                max = max_w * max_h;
+                nr_samples = (size_t)(max * 3U * sizeof(unsigned char));
+                in = inbuf = (unsigned char*)opj_image_data_alloc(nr_samples);
+                out = outbuf = (unsigned char*)opj_image_data_alloc(nr_samples);
+
+                if (inbuf == NULL || outbuf == NULL) {
+                    goto fails0;
+                }
+
+                r = image->comps[0].data;
+                g = image->comps[1].data;
+                b = image->comps[2].data;
+
+                for (i = 0U; i < max; ++i) {
+                    *in++ = (unsigned char) * r++;
+                    *in++ = (unsigned char) * g++;
+                    *in++ = (unsigned char) * b++;
+                }
+
+                cmsDoTransform(transform, inbuf, outbuf, (cmsUInt32Number)max);
+
+                r = image->comps[0].data;
+                g = image->comps[1].data;
+                b = image->comps[2].data;
+
+                for (i = 0U; i < max; ++i) {
+                    *r++ = (int) * out++;
+                    *g++ = (int) * out++;
+                    *b++ = (int) * out++;
+                }
+                ok = 1;
 
 fails0:
-            opj_image_data_free(inbuf);
-            opj_image_data_free(outbuf);
-        } else { /* prec > 8 */
-            unsigned short *inbuf, *outbuf, *in, *out;
-
-            max = max_w * max_h;
-            nr_samples = (size_t)(max * 3U * sizeof(unsigned short));
-            in = inbuf = (unsigned short*)opj_image_data_alloc(nr_samples);
-            out = outbuf = (unsigned short*)opj_image_data_alloc(nr_samples);
-
-            if (inbuf == NULL || outbuf == NULL) {
-                goto fails1;
-            }
-
-            r = image->comps[0].data;
-            g = image->comps[1].data;
-            b = image->comps[2].data;
-
-            for (i = 0U  ; i < max; ++i) {
-                *in++ = (unsigned short) * r++;
-                *in++ = (unsigned short) * g++;
-                *in++ = (unsigned short) * b++;
-            }
-
-            cmsDoTransform(transform, inbuf, outbuf, (cmsUInt32Number)max);
-
-            r = image->comps[0].data;
-            g = image->comps[1].data;
-            b = image->comps[2].data;
-
-            for (i = 0; i < max; ++i) {
-                *r++ = (int) * out++;
-                *g++ = (int) * out++;
-                *b++ = (int) * out++;
-            }
-            ok = 1;
+                opj_image_data_free(inbuf);
+                opj_image_data_free(outbuf);
+            } else { /* prec > 8 */
+                unsigned short *inbuf, *outbuf, *in, *out;
+
+                max = max_w * max_h;
+                nr_samples = (size_t)(max * 3U * sizeof(unsigned short));
+                in = inbuf = (unsigned short*)opj_image_data_alloc(nr_samples);
+                out = outbuf = (unsigned short*)opj_image_data_alloc(nr_samples);
+
+                if (inbuf == NULL || outbuf == NULL) {
+                    goto fails1;
+                }
+
+                r = image->comps[0].data;
+                g = image->comps[1].data;
+                b = image->comps[2].data;
+
+                for (i = 0U  ; i < max; ++i) {
+                    *in++ = (unsigned short) * r++;
+                    *in++ = (unsigned short) * g++;
+                    *in++ = (unsigned short) * b++;
+                }
+
+                cmsDoTransform(transform, inbuf, outbuf, (cmsUInt32Number)max);
+
+                r = image->comps[0].data;
+                g = image->comps[1].data;
+                b = image->comps[2].data;
+
+                for (i = 0; i < max; ++i) {
+                    *r++ = (int) * out++;
+                    *g++ = (int) * out++;
+                    *b++ = (int) * out++;
+                }
+                ok = 1;
 
 fails1:
-            opj_image_data_free(inbuf);
-            opj_image_data_free(outbuf);
+                opj_image_data_free(inbuf);
+                opj_image_data_free(outbuf);
+            }
+        } else {
+            fprintf(stderr,
+                    "[ERROR] Image components should have the same width and height\n");
+            cmsDeleteTransform(transform);
+            return;
         }
     } else { /* image->numcomps <= 2 : GRAY, GRAYA */
         if (prec <= 8) {
             unsigned char *in, *inbuf, *out, *outbuf;
             opj_image_comp_t *new_comps;

Vulnerable code:         if (prec <= 8) {
            unsigned char *inbuf, *outbuf, *in, *out;

            max = max_w * max_h;
            nr_samples = (size_t)(max * 3U * sizeof(unsigned char));
            in = inbuf = (unsigned char*)opj_image_data_alloc(nr_samples);
            out = outbuf = (unsigned char*)opj_image_data_alloc(nr_samples);

            if (inbuf == NULL || outbuf == NULL) {
                goto fails0;
            }

            r = image->comps[0].data;
            g = image->comps[1].data;
            b = image->comps[2].data;

            for (i = 0U; i < max; ++i) {
                *in++ = (unsigned char) * r++;
                *in++ = (unsigned char) * g++;
                *in++ = (unsigned char) * b++;
            }

            cmsDoTransform(transform, inbuf, outbuf, (cmsUInt32Number)max);

            r = image->comps[0].data;
            g = image->comps[1].data;
            b = image->comps[2].data;

            for (i = 0U; i < max; ++i) {
                *r++ = (int) * out++;
                *g++ = (int) * out++;
                *b++ = (int) * out++;
            }
            ok = 1;
            opj_image_data_free(inbuf);
            opj_image_data_free(outbuf);
        } else { /* prec > 8 */
            unsigned short *inbuf, *outbuf, *in, *out;

            max = max_w * max_h;
            nr_samples = (size_t)(max * 3U * sizeof(unsigned short));
            in = inbuf = (unsigned short*)opj_image_data_alloc(nr_samples);
            out = outbuf = (unsigned short*)opj_image_data_alloc(nr_samples);

            if (inbuf == NULL || outbuf == NULL) {
                goto fails1;
            }

            r = image->comps[0].data;
            g = image->comps[1].data;
            b = image->comps[2].data;

            for (i = 0U  ; i < max; ++i) {
                *in++ = (unsigned short) * r++;
                *in++ = (unsigned short) * g++;
                *in++ = (unsigned short) * b++;
            }

            cmsDoTransform(transform, inbuf, outbuf, (cmsUInt32Number)max);

            r = image->comps[0].data;
            g = image->comps[1].data;
            b = image->comps[2].data;

            for (i = 0; i < max; ++i) {
                *r++ = (int) * out++;
                *g++ = (int) * out++;
                *b++ = (int) * out++;
            }
            ok = 1;
            opj_image_data_free(inbuf);
            opj_image_data_free(outbuf);
Fixed code:         if ((image->comps[0].w == image->comps[1].w &&
                image->comps[0].w == image->comps[2].w) &&
                (image->comps[0].h == image->comps[1].h &&
                 image->comps[0].h == image->comps[2].h)) {
            if (prec <= 8) {
                unsigned char *inbuf, *outbuf, *in, *out;

                max = max_w * max_h;
                nr_samples = (size_t)(max * 3U * sizeof(unsigned char));
                in = inbuf = (unsigned char*)opj_image_data_alloc(nr_samples);
                out = outbuf = (unsigned char*)opj_image_data_alloc(nr_samples);

                if (inbuf == NULL || outbuf == NULL) {
                    goto fails0;
                }

                r = image->comps[0].data;
                g = image->comps[1].data;
                b = image->comps[2].data;

                for (i = 0U; i < max; ++i) {
                    *in++ = (unsigned char) * r++;
                    *in++ = (unsigned char) * g++;
                    *in++ = (unsigned char) * b++;
                }

                cmsDoTransform(transform, inbuf, outbuf, (cmsUInt32Number)max);

                r = image->comps[0].data;
                g = image->comps[1].data;
                b = image->comps[2].data;

                for (i = 0U; i < max; ++i) {
                    *r++ = (int) * out++;
                    *g++ = (int) * out++;
                    *b++ = (int) * out++;
                }
                ok = 1;
                opj_image_data_free(inbuf);
                opj_image_data_free(outbuf);
            } else { /* prec > 8 */
                unsigned short *inbuf, *outbuf, *in, *out;

                max = max_w * max_h;
                nr_samples = (size_t)(max * 3U * sizeof(unsigned short));
                in = inbuf = (unsigned short*)opj_image_data_alloc(nr_samples);
                out = outbuf = (unsigned short*)opj_image_data_alloc(nr_samples);

                if (inbuf == NULL || outbuf == NULL) {
                    goto fails1;
                }

                r = image->comps[0].data;
                g = image->comps[1].data;
                b = image->comps[2].data;

                for (i = 0U  ; i < max; ++i) {
                    *in++ = (unsigned short) * r++;
                    *in++ = (unsigned short) * g++;
                    *in++ = (unsigned short) * b++;
                }

                cmsDoTransform(transform, inbuf, outbuf, (cmsUInt32Number)max);

                r = image->comps[0].data;
                g = image->comps[1].data;
                b = image->comps[2].data;

                for (i = 0; i < max; ++i) {
                    *r++ = (int) * out++;
                    *g++ = (int) * out++;
                    *b++ = (int) * out++;
                }
                ok = 1;
                opj_image_data_free(inbuf);
                opj_image_data_free(outbuf);
            }
        } else {
            fprintf(stderr,
                    "[ERROR] Image components should have the same width and height\n");
            cmsDeleteTransform(transform);
            return;