Fix CVE 2023 5841 (#1627)

* enable deep file checks for core Signed-off-by: Kimball Thurston <[email protected]> * fix possible int overflow Signed-off-by: Kimball Thurston <[email protected]> * fix validation of deep sample counts Addresses CVE-2023-5841, fixing sample count check to not only check against 0 but previous sample as well. Signed-off-by: Kimball Thurston <[email protected]> * add clarifying comment Signed-off-by: Kimball Thurston <[email protected]> --------- Signed-off-by: Kimball Thurston <[email protected]>
AcademySoftwareFoundation · Feb 4, 2024 · 46944c3 · 46944c3
1 parent 8743203
commit 46944c3
Show file tree

Hide file tree

Showing 3 changed files with 175 additions and 51 deletions.
diff --git a/src/lib/OpenEXRCore/decoding.c b/src/lib/OpenEXRCore/decoding.c
@@ -288,6 +288,9 @@ default_decompress_chunk (exr_decode_pipeline_t* decode)
         uint64_t sampsize =
             (((uint64_t) decode->chunk.width) *
              ((uint64_t) decode->chunk.height));
+
+        if ((decode->decode_flags & EXR_DECODE_SAMPLE_COUNTS_AS_INDIVIDUAL))
+            sampsize += 1;
         sampsize *= sizeof (int32_t);
 
         rv = decompress_data (
@@ -340,7 +343,7 @@ unpack_sample_table (
     exr_result_t rv           = EXR_ERR_SUCCESS;
     int32_t      w            = decode->chunk.width;
     int32_t      h            = decode->chunk.height;
-    int32_t      totsamp      = 0;
+    uint64_t     totsamp      = 0;
     int32_t*     samptable    = decode->sample_count_table;
     size_t       combSampSize = 0;
 
@@ -351,38 +354,44 @@ unpack_sample_table (
     {
         for (int32_t y = 0; y < h; ++y)
         {
+            int32_t *cursampline = samptable + y * w;
             int32_t prevsamp = 0;
             for (int32_t x = 0; x < w; ++x)
             {
                 int32_t nsamps =
-                    (int32_t) one_to_native32 ((uint32_t) samptable[y * w + x]);
-                if (nsamps < 0) return EXR_ERR_INVALID_SAMPLE_DATA;
-                samptable[y * w + x] = nsamps - prevsamp;
-                prevsamp             = nsamps;
+                    (int32_t) one_to_native32 ((uint32_t) cursampline[x]);
+                if (nsamps < prevsamp) return EXR_ERR_INVALID_SAMPLE_DATA;
+
+                cursampline[x] = nsamps - prevsamp;
+                prevsamp       = nsamps;
             }
-            totsamp += prevsamp;
+            totsamp += (uint64_t)prevsamp;
         }
-        samptable[w * h] = totsamp;
+        if (totsamp >= (uint64_t)INT32_MAX)
+            return EXR_ERR_INVALID_SAMPLE_DATA;
+        samptable[w * h] = (int32_t)totsamp;
     }
     else
     {
         for (int32_t y = 0; y < h; ++y)
         {
+            int32_t *cursampline = samptable + y * w;
             int32_t prevsamp = 0;
             for (int32_t x = 0; x < w; ++x)
             {
                 int32_t nsamps =
-                    (int32_t) one_to_native32 ((uint32_t) samptable[y * w + x]);
-                if (nsamps < 0) return EXR_ERR_INVALID_SAMPLE_DATA;
-                samptable[y * w + x] = nsamps;
-                prevsamp             = nsamps;
+                    (int32_t) one_to_native32 ((uint32_t) cursampline[x]);
+                if (nsamps < prevsamp) return EXR_ERR_INVALID_SAMPLE_DATA;
+
+                cursampline[x] = nsamps;
+                prevsamp = nsamps;
             }
-            totsamp += prevsamp;
+
+            totsamp += (uint64_t)prevsamp;
         }
     }
 
-    if (totsamp < 0 ||
-        (((uint64_t) totsamp) * combSampSize) > decode->chunk.unpacked_size)
+    if ((totsamp * combSampSize) > decode->chunk.unpacked_size)
     {
         rv = pctxt->report_error (
             pctxt, EXR_ERR_INVALID_SAMPLE_DATA, "Corrupt sample count table");

diff --git a/src/lib/OpenEXRCore/unpack.c b/src/lib/OpenEXRCore/unpack.c
@@ -1196,9 +1196,10 @@ generic_unpack_deep_pointers (exr_decode_pipeline_t* decode)
                 if (outpix)
                 {
                     uint8_t* cdata = outpix;
+
                     UNPACK_SAMPLES (samps)
                 }
-                srcbuffer += bpc * samps;
+                srcbuffer += ((size_t) bpc) * ((size_t) samps);
             }
         }
         sampbuffer += w;
@@ -1242,12 +1243,14 @@ generic_unpack_deep (exr_decode_pipeline_t* decode)
                 }
                 else
                     prevsamps = sampbuffer[w - 1];
+
                 srcbuffer += ((size_t) bpc) * ((size_t) prevsamps);
 
                 if (incr_tot) totsamps += (size_t) prevsamps;
 
                 continue;
             }
+
             cdata += totsamps * ((size_t) ubpc);
 
             for (int x = 0; x < w; ++x)
@@ -1263,7 +1266,7 @@ generic_unpack_deep (exr_decode_pipeline_t* decode)
 
                 UNPACK_SAMPLES (samps)
 
-                srcbuffer += bpc * samps;
+                srcbuffer += ((size_t) bpc) * ((size_t) samps);
                 if (incr_tot) totsamps += (size_t) samps;
             }
         }
@@ -1301,7 +1304,7 @@ internal_exr_match_decode (
 
     if (isdeep)
     {
-        if ((decode->decode_flags & EXR_DECODE_SAMPLE_COUNTS_AS_INDIVIDUAL))
+        if ((decode->decode_flags & EXR_DECODE_NON_IMAGE_DATA_AS_POINTERS))
             return &generic_unpack_deep_pointers;
         return &generic_unpack_deep;
     }