Added check to make sure: HighBit < BitsAllocated.
authorJoerg Riesmeier <dicom@jriesmeier.com>
Fri, 21 Mar 2025 11:45:44 +0000 (12:45 +0100)
committerMathieu Malaterre <malat@debian.org>
Fri, 21 Mar 2025 11:45:44 +0000 (12:45 +0100)
Forwarded: https://git.dcmtk.org/?p=dcmtk.git;a=commit;h=03e851b0586d05057c3268988e180ffb426b2e03
Bug-Debian: https://bugs.debian.org/1093047
Reviewed-By: Étienne Mollier <emollier@debian.org>
Last-Update: 2025-01-18

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
<vulndiscovery@external.cisco.com> for the report, sample file (PoC)
and detailed analysis. See TALOS-2024-2121 and CVE-2024-52333.

Gbp-Pq: Name 0008-CVE-2024-52333.patch

dcmimgle/libsrc/diimage.cc

index 480235e3b542c5bcbcccf50d92aac178a17a4e5f..1827ac68b48c56830d17e8c7e79de732942385c6 100644 (file)
@@ -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))