openjpeg: Prevent an integer overflow in opj_jp2_apply_pclr.

third_party/libopenjpeg20/jp2.c

 /*
  * The copyright in this software is being made available under the 2-clauses 
  * BSD License, included below. This software may be subject to other third 
  * party and contributor rights, including patent rights, and no such rights
  * are granted under this license.
  *
  * Copyright (c) 2002-2014, Universite catholique de Louvain (UCL), Belgium
  * Copyright (c) 2002-2014, Professor Benoit Macq
  * Copyright (c) 2001-2003, David Janssens
  * Copyright (c) 2002-2003, Yannick Verschueren
@@ skipping 954 matching lines @@
         OPJ_UINT16 i, nr_channels, cmp, pcol;
         OPJ_INT32 k, top_k;
 
         channel_size = color->jp2_pclr->channel_size;
         channel_sign = color->jp2_pclr->channel_sign;
         entries = color->jp2_pclr->entries;
         cmap = color->jp2_pclr->cmap;
         nr_channels = color->jp2_pclr->nr_channels;
 
         old_comps = image->comps;
+    /* Overflow check: prevent integer overflow */
+    for (i = 0; i < nr_channels; ++i) {

[Oliver Chang, 2016/08/25 01:02:20]

I know indentation is all over the place in this file, but could you make this
match the indentation in the previous lines?

+        cmp = cmap[i].cmp;
+        if (old_comps[cmp].h == 0 || old_comps[cmp].w > UINT_MAX / sizeof(OPJ_INT32) / old_comps[cmp].h) {

[Oliver Chang, 2016/08/25 01:02:20]

let's be explicit here and use ((OPJ_UINT32)-1) instead of UINT_MAX, since
"unsigned int" isn't actually defined to be 32 bits.

+            return;
+        }
+    }
+
         new_comps = (opj_image_comp_t*)
                         opj_malloc(nr_channels * sizeof(opj_image_comp_t));
         if (!new_comps) {
                 /* FIXME no error code for opj_jp2_apply_pclr */
                 /* FIXME event manager error callback */
                 return;
         }
         for(i = 0; i < nr_channels; ++i) {
                 pcol = cmap[i].pcol; cmp = cmap[i].cmp;
 
@@ skipping 19 matching lines @@
                 new_comps[i].prec = channel_size[i];
                 new_comps[i].sgnd = channel_sign[i];
         }
 
         top_k = color->jp2_pclr->nr_entries - 1;
 
         for(i = 0; i < nr_channels; ++i) {
                 /* Palette mapping: */
                 cmp = cmap[i].cmp; pcol = cmap[i].pcol;
                 src = old_comps[cmp].data;
-    assert( src );
+        dst = new_comps[i].data;
                 max = new_comps[i].w * new_comps[i].h;
 
+        /* Prevent null pointer access */
+        if (!src || !dst) {
+            for (j = 0; j < nr_channels; ++j) {
+                if (new_comps[j].data) {

[Oliver Chang, 2016/08/25 01:02:20]

you don't need to check if the pointer is null before passing it to free()

+                    opj_free(new_comps[j].data);
+                }
+            }
+            opj_free(new_comps);
+            new_comps = NULL;
+            return;
+        }
+
                 /* Direct use: */
     if(cmap[i].mtyp == 0) {
       assert( cmp == 0 ); // probably wrong.
-      dst = new_comps[i].data;
-      assert( dst );
       for(j = 0; j < max; ++j) {
         dst[j] = src[j];
       }
     }
     else {
       assert( i == pcol ); // probably wrong?
-      dst = new_comps[i].data;
-      assert( dst );
       for(j = 0; j < max; ++j) {
         /* The index */
         if((k = src[j]) < 0) k = 0; else if(k > top_k) k = top_k;
 
         /* The colour */
         dst[j] = (OPJ_INT32)entries[k * nr_channels + pcol];
         }
     }
         }
 
@@ skipping 2114 matching lines @@
   len = opj_stream_tell(cio)-lenp;
   opj_stream_skip(cio, lenp, p_manager);
   opj_write_bytes(l_data_header,len,4);/* L              */
   opj_stream_write_data(cio,l_data_header,4,p_manager);
   opj_stream_seek(cio, lenp+len,p_manager);
 
   return len;
 }
 #endif
 #endif /* USE_JPIP */