Added check to make sure: HighBit < BitsAllocated.
authorJoerg Riesmeier <dicom@jriesmeier.com>
Sat, 18 Jan 2025 15:30:29 +0000 (16:30 +0100)
committerÉtienne Mollier <emollier@debian.org>
Sat, 18 Jan 2025 15:30:29 +0000 (16:30 +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 f198bf58c2ca00c166376c3b0e965a812264167d..09f8201c7158b5468e9998f7204ae1221841df35 100644 (file)
@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 1996-2021, 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
@@ -548,12 +548,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))