opj_t1_encode_cblks: fix UBSAN signed integer overflow

Fixes #1053 / CVE-2018-5727

Note: I don't consider this issue to be a security vulnerability, in
practice.
At least with gcc or clang compilers on x86_64 which generate the same
assembly code with or without that fix.

diff --git a/src/lib/openjp2/t1.c b/src/lib/openjp2/t1.c
index ec04c682525fc96d6eaa73687ba65dd6e60fedb1..f6f7671190cd5bc5a40a8ccac9b349abc0489e43 100644 (file)
--- a/src/lib/openjp2/t1.c
+++ b/src/lib/openjp2/t1.c
@@ -2168,9 +2168,18 @@ OPJ_BOOL opj_t1_encode_cblks(opj_t1_t *t1,
                         t1->data = tiledp;
                         t1->data_stride = tile_w;
                         if (tccp->qmfbid == 1) {
                         t1->data = tiledp;
                         t1->data_stride = tile_w;
                         if (tccp->qmfbid == 1) {

+                            /* Do multiplication on unsigned type, even if the
+                             * underlying type is signed, to avoid potential
+                             * int overflow on large value (the output will be
+                             * incorrect in such situation, but whatever...)
+                             * This assumes complement-to-2 signed integer
+                             * representation
+                             * Fixes https://github.com/uclouvain/openjpeg/issues/1053
+                             */
+                            OPJ_UINT32* OPJ_RESTRICT tiledp_u = (OPJ_UINT32*) tiledp;

                             for (j = 0; j < cblk_h; ++j) {
                                 for (i = 0; i < cblk_w; ++i) {
                             for (j = 0; j < cblk_h; ++j) {
                                 for (i = 0; i < cblk_w; ++i) {

-                                    tiledp[tileIndex] *= (1 << T1_NMSEDEC_FRACBITS);
+                                    tiledp_u[tileIndex] <<= T1_NMSEDEC_FRACBITS;

                                     tileIndex++;
                                 }
                                 tileIndex += tileLineAdvance;
                                     tileIndex++;
                                 }
                                 tileIndex += tileLineAdvance;