Added check to make sure: HighBit < BitsAllocated.

Added check to the image preprocessing to make sure that the value of HighBit is always less than the value of BitsAllocated. Before, this missing check could lead to memory corruption if an invalid combination of values was retrieved from a malformed DICOM dataset. Thanks to Emmanuel Tacheau from the Cisco Talos team <[email protected]> for the report, sample file (PoC) and detailed analysis. See TALOS-2024-2121 and CVE-2024-52333.
DCMTK · Jan 3, 2025 · 03e851b · 03e851b
1 parent 3827482
commit 03e851b
Showing 1 changed file with 11 additions and 5 deletions.
diff --git a/dcmimgle/libsrc/diimage.cc b/dcmimgle/libsrc/diimage.cc
@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 1996-2024, OFFIS e.V.
+ *  Copyright (C) 1996-2025, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -549,12 +549,18 @@ void DiImage::convertPixelData()
     {
         const unsigned long fsize = OFstatic_cast(unsigned long, Rows) * OFstatic_cast(unsigned long, Columns) *
             OFstatic_cast(unsigned long, SamplesPerPixel);
-        if ((BitsAllocated < 1) || (BitsStored < 1) || (BitsAllocated < BitsStored) ||
-            (BitsStored > OFstatic_cast(Uint16, HighBit + 1)))
+        if ((BitsAllocated < 1) || (BitsStored < 1))
         {
             ImageStatus = EIS_InvalidValue;
-            DCMIMGLE_ERROR("invalid values for 'BitsAllocated' (" << BitsAllocated << "), "
-                << "'BitsStored' (" << BitsStored << ") and/or 'HighBit' (" << HighBit << ")");
+            DCMIMGLE_ERROR("invalid value(s) for 'BitsAllocated' (" << BitsAllocated << "), "
+                << "and/or 'BitsStored' (" << BitsStored << ")");
+            return;
+        }
+        else if ((BitsAllocated < BitsStored) || (BitsAllocated <= HighBit) || ((BitsStored - 1) > HighBit))
+        {
+            ImageStatus = EIS_InvalidValue;
+            DCMIMGLE_ERROR("invalid combination of values for 'BitsAllocated' (" << BitsAllocated << "), "
+                << "'BitsStored' (" << BitsStored << ") and 'HighBit' (" << HighBit << ")");
             return;
         }
         else if ((evr == EVR_OB) && (BitsStored <= 8))