changeset 6741:7318f5f73ace

Fixed various OOB read/write
author Alain Mazy <am@orthanc.team>
date Thu, 07 May 2026 16:24:20 +0200
parents 9551353f3e03
children b9752ef08934
files NEWS OrthancFramework/Sources/DicomParsing/Internals/DicomFrameIndex.cpp OrthancFramework/Sources/DicomParsing/Internals/DicomImageDecoder.cpp OrthancFramework/Sources/DicomParsing/ParsedDicomFile.cpp OrthancFramework/Sources/Images/JpegReader.cpp OrthancFramework/Sources/Images/PngReader.cpp
diffstat 6 files changed, 120 insertions(+), 21 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Wed May 06 16:55:22 2026 +0200
+++ b/NEWS	Thu May 07 16:24:20 2026 +0200
@@ -39,6 +39,7 @@
 * Fix a Denial of Service via Deeply Nested DICOM Sequences
   https://orthanc.uclouvain.be/bugs/show_bug.cgi?id=258
   Security issue reported by Jose Lopez Martinez (aka elpe_pinillo) from Deloitte.
+* Fixed various OOB read/write (TODO: finalize release notes)
 * Upgraded dependencies for static builds:
   - dcmtk 3.7.0 (hot-fix: https://github.com/DCMTK/dcmtk/commit/847d50e83ae5bbfbc731c99c142ee1410303d222)
 
--- a/OrthancFramework/Sources/DicomParsing/Internals/DicomFrameIndex.cpp	Wed May 06 16:55:22 2026 +0200
+++ b/OrthancFramework/Sources/DicomParsing/Internals/DicomFrameIndex.cpp	Thu May 07 16:24:20 2026 +0200
@@ -219,6 +219,10 @@
         }
 
         assert(offset + fragment->getLength() <= frame.size());
+        if (static_cast<uint64_t>(offset) + fragment->getLength() > static_cast<uint64_t>(frame.size()))
+        {
+          throw OrthancException(ErrorCode_CorruptedFile, "Fragment too large to fit in frame");
+        }
 
         memcpy(target + offset, content, fragment->getLength());
         offset += fragment->getLength();
--- a/OrthancFramework/Sources/DicomParsing/Internals/DicomImageDecoder.cpp	Wed May 06 16:55:22 2026 +0200
+++ b/OrthancFramework/Sources/DicomParsing/Internals/DicomImageDecoder.cpp	Thu May 07 16:24:20 2026 +0200
@@ -123,6 +123,11 @@
 #  define EXS_JPEGProcess14SV1  EXS_JPEGProcess14SV1TransferSyntax
 #endif
 
+static const uint64_t MAX_DECODED_FRAME_SIZE = (sizeof(void*) == 4
+                                                ? 1ull * 1024ull * 1024ull * 1024ull   // 1 GB on 32 bits system
+                                                : 4ull * 1024ull * 1024ull * 1024ull); // 4 GB on 64 bits system
+
+
 namespace Orthanc
 {
   static const Endianness ENDIANNESS = Toolbox::DetectEndianness();
@@ -528,11 +533,13 @@
 
       case PixelFormat_RGB48:
       {
+        const uint64_t expectedSize = static_cast<uint64_t>(target->GetWidth()) * target->GetHeight() * 2;
+
         if (r != "0\\0\\16" ||
             rc != 65536 ||
             gc != 65536 ||
             bc != 65536 ||
-            pixelLength != 2 * target->GetWidth() * target->GetHeight())
+            pixelLength != expectedSize)
         {
           throw OrthancException(ErrorCode_NotImplemented, std::string("Palette Color Lookup Table Descriptor not supported: '") + r.c_str() + "'");
         }
@@ -610,8 +617,26 @@
     {
       try
       {
-        size_t frameSize = info.GetHeight() * info.GetWidth() * GetBytesPerPixel(sourceFormat);
-        if ((frame + 1) * frameSize <= source.GetSize())
+        uint64_t frameSize = static_cast<uint64_t>(info.GetHeight()) * info.GetWidth() * GetBytesPerPixel(sourceFormat);
+
+        if (frameSize > MAX_DECODED_FRAME_SIZE ||
+            static_cast<uint64_t>(static_cast<size_t>(frameSize)) != frameSize)
+        {
+          std::ostringstream errorMessage;
+          errorMessage << "ImageDecoder: max decoded frame size overflow  (" << frameSize << " vs " << MAX_DECODED_FRAME_SIZE << ")";
+          throw OrthancException(ErrorCode_BadFileFormat, errorMessage.str());
+        }
+
+        uint64_t frameEnd = frameSize * (frame + 1);
+
+        if (static_cast<uint64_t>(static_cast<size_t>(frameEnd)) != frameEnd)
+        {
+          std::ostringstream errorMessage;
+          errorMessage << "ImageDecoder: frameEnd platform overflow (" << frameEnd << ")"; // can only happen on 32 bits
+          throw OrthancException(ErrorCode_BadFileFormat, errorMessage.str());
+        }
+
+        if (frameEnd <= static_cast<uint64_t>(source.GetSize()))
         {
           const uint8_t* buffer = reinterpret_cast<const uint8_t*>(source.GetAccessor().GetPixelData());
 
@@ -728,12 +753,11 @@
   }
 
 
-  ImageAccessor* DicomImageDecoder::ApplyCodec
-  (const DcmCodec& codec,
-   const DcmCodecParameter& parameters,
-   const DcmRepresentationParameter& representationParameter,
-   DcmDataset& dataset,
-   unsigned int frame)
+  ImageAccessor* DicomImageDecoder::ApplyCodec(const DcmCodec& codec,
+                                               const DcmCodecParameter& parameters,
+                                               const DcmRepresentationParameter& representationParameter,
+                                               DcmDataset& dataset,
+                                               unsigned int frame)
   {
     DcmPixelSequence* pixelSequence = FromDcmtkBridge::GetPixelSequence(dataset);
     if (pixelSequence == NULL)
@@ -757,7 +781,24 @@
         info.GetChannelCount() == 1)
     {
       std::string uncompressed;
-      uncompressed.resize(info.GetWidth() * info.GetHeight() * info.GetBytesPerValue());
+      uint64_t frameSize = static_cast<uint64_t>(info.GetWidth()) * info.GetHeight() * info.GetBytesPerValue();
+
+      if (frameSize > MAX_DECODED_FRAME_SIZE ||
+          static_cast<uint64_t>(static_cast<size_t>(frameSize)) != frameSize)
+      {
+        std::ostringstream errorMessage;
+        errorMessage << "ImageDecoder: max decoded frame size overflow  (" << frameSize << " vs " << MAX_DECODED_FRAME_SIZE << ")";
+        throw OrthancException(ErrorCode_BadFileFormat, errorMessage.str());
+      }
+
+      uncompressed.resize(frameSize);
+
+      if (static_cast<uint64_t>(static_cast<Uint32>(frameSize)) != frameSize) // in case, some day, MAX_DECODED_FRAME_SIZE gets larger than 4GB
+      {
+        std::ostringstream errorMessage;
+        errorMessage << "ImageDecoder: frameSize too large for DCMTK interface (" << frameSize << ")";
+        throw OrthancException(ErrorCode_BadFileFormat, errorMessage.str());
+      }
 
       if (uncompressed.size() == 0 ||
           !codec.decodeFrame(&representationParameter, 
@@ -775,6 +816,13 @@
     }
     else
     {
+      if (static_cast<uint64_t>(static_cast<Uint32>(target->GetSize())) != target->GetSize()) // in case, some day, MAX_DECODED_FRAME_SIZE gets larger than 4GB
+      {
+        std::ostringstream errorMessage;
+        errorMessage << "ImageDecoder: target->GetSize() too large for DCMTK interface (" << target->GetSize() << ")";
+        throw OrthancException(ErrorCode_BadFileFormat, errorMessage.str());
+      }
+
       if (!codec.decodeFrame(&representationParameter, 
                              pixelSequence, &parameters, 
                              &dataset, frame, startFragment, target->GetBuffer(), 
--- a/OrthancFramework/Sources/DicomParsing/ParsedDicomFile.cpp	Wed May 06 16:55:22 2026 +0200
+++ b/OrthancFramework/Sources/DicomParsing/ParsedDicomFile.cpp	Thu May 07 16:24:20 2026 +0200
@@ -146,6 +146,9 @@
 #  define EXS_JPEGProcess1      EXS_JPEGProcess1TransferSyntax
 #endif
 
+static const uint64_t MAX_OVERLAY_SIZE = (sizeof(void*) == 4
+                                          ? 1ull * 1024ull * 1024ull * 1024ull   // 1 GB on 32 bits system
+                                          : 4ull * 1024ull * 1024ull * 1024ull); // 4 GB on 64 bits system
 
 
 namespace Orthanc
@@ -2081,7 +2084,7 @@
   }
 
 
-  static unsigned int Ceiling(unsigned int a,
+  static unsigned int Ceiling(uint64_t a,
                               unsigned int b)
   {
     if (a % b == 0)
@@ -2135,8 +2138,25 @@
        * bytes, even if targeting WebAssembly.
        **/
 
-      unsigned int expectedSize = Ceiling(rows * columns, 8);
-      if (overlayElement->getLengthField() < expectedSize)
+      uint64_t expectedOverlaySize = Ceiling(static_cast<uint64_t>(rows) * columns, 8);
+
+      if (expectedOverlaySize > MAX_OVERLAY_SIZE ||
+          static_cast<uint64_t>(static_cast<size_t>(expectedOverlaySize)) != expectedOverlaySize)
+      {
+        std::ostringstream errorMessage;
+        errorMessage << "ParsedDicomFile: max overlay size overflow  (" << expectedOverlaySize << " vs " << MAX_OVERLAY_SIZE << ")";
+        throw OrthancException(ErrorCode_BadFileFormat, errorMessage.str());
+      }
+
+      if (static_cast<uint64_t>(static_cast<Uint32>(expectedOverlaySize)) != expectedOverlaySize) // in case, some day, MAX_OVERLAY_SIZE gets larger than 4GB
+      {
+        std::ostringstream errorMessage;
+        errorMessage << "ParsedDicomFile: expectedOverlaySize too large for DCMTK interface (" << expectedOverlaySize << ")";
+        throw OrthancException(ErrorCode_BadFileFormat, errorMessage.str());
+      }
+
+
+      if (overlayElement->getLengthField() < expectedOverlaySize)
       {
         throw OrthancException(ErrorCode_CorruptedFile, "Overlay doesn't have a valid number of bits");
       }
--- a/OrthancFramework/Sources/Images/JpegReader.cpp	Wed May 06 16:55:22 2026 +0200
+++ b/OrthancFramework/Sources/Images/JpegReader.cpp	Thu May 07 16:24:20 2026 +0200
@@ -33,6 +33,9 @@
 #  include "../SystemToolbox.h"
 #endif
 
+static const uint64_t MAX_DECODED_JPEG_IMAGE_SIZE = (sizeof(void*) == 4
+                                                     ? 1ull * 1024ull * 1024ull * 1024ull   // 1 GB on 32 bits system
+                                                     : 4ull * 1024ull * 1024ull * 1024ull); // 4 GB on 64 bits system
 
 namespace Orthanc
 {
@@ -62,7 +65,16 @@
       throw OrthancException(ErrorCode_NotImplemented);
     }
 
-    unsigned int pitch = cinfo.output_width * cinfo.output_components;
+    uint64_t pitch = static_cast<uint64_t>(cinfo.output_width) * cinfo.output_components;
+    uint64_t totalSize = pitch * cinfo.output_height;
+
+    if (totalSize > MAX_DECODED_JPEG_IMAGE_SIZE ||
+        static_cast<uint64_t>(static_cast<size_t>(totalSize)) != totalSize)
+    {
+      std::ostringstream errorMessage;
+      errorMessage << "JPEG IMAGE size overflow  (" << totalSize << " vs " << MAX_DECODED_JPEG_IMAGE_SIZE << ")";
+      throw OrthancException(ErrorCode_BadFileFormat, errorMessage.str());
+    }
 
     /* Make a one-row-high sample array that will go away when done with image */
     JSAMPARRAY buffer = (*cinfo.mem->alloc_sarray) ((j_common_ptr) &cinfo, JPOOL_IMAGE, pitch, 1);
--- a/OrthancFramework/Sources/Images/PngReader.cpp	Wed May 06 16:55:22 2026 +0200
+++ b/OrthancFramework/Sources/Images/PngReader.cpp	Thu May 07 16:24:20 2026 +0200
@@ -36,6 +36,11 @@
 #include <png.h>
 #include <string.h>  // For memcpy()
 
+static const uint64_t MAX_DECODED_PNG_IMAGE_SIZE = (sizeof(void*) == 4
+                                                    ? 1ull * 1024ull * 1024ull * 1024ull   // 1 GB on 32 bits system
+                                                    : 4ull * 1024ull * 1024ull * 1024ull); // 4 GB on 64 bits system
+
+
 namespace Orthanc
 {
 #if ORTHANC_SANDBOXED == 0
@@ -150,17 +155,17 @@
                  &compression_type, &filter_method);
 
     PixelFormat format;
-    unsigned int pitch;
+    uint64_t pitch;
 
     if (color_type == PNG_COLOR_TYPE_GRAY && bit_depth == 8)
     {
       format = PixelFormat_Grayscale8;
-      pitch = width;
+      pitch = static_cast<uint64_t>(width);
     }
     else if (color_type == PNG_COLOR_TYPE_GRAY && bit_depth == 16)
     {
       format = PixelFormat_Grayscale16;
-      pitch = 2 * width;
+      pitch = static_cast<uint64_t>(width) * 2;
 
       if (Toolbox::DetectEndianness() == Endianness_Little)
       {
@@ -170,17 +175,17 @@
     else if (color_type == PNG_COLOR_TYPE_RGB && bit_depth == 8)
     {
       format = PixelFormat_RGB24;
-      pitch = 3 * width;
+      pitch = static_cast<uint64_t>(width) * 3;
     }
     else if (color_type == PNG_COLOR_TYPE_RGBA && bit_depth == 8)
     {
       format = PixelFormat_RGBA32;
-      pitch = 4 * width;
+      pitch = static_cast<uint64_t>(width) * 4;
     }
     else if (color_type == PNG_COLOR_TYPE_RGBA && bit_depth == 16)
     {
       format = PixelFormat_RGBA64;
-      pitch = 8 * width;
+      pitch = static_cast<uint64_t>(width) * 8;
 
       if (Toolbox::DetectEndianness() == Endianness_Little)
       {
@@ -192,7 +197,16 @@
       throw OrthancException(ErrorCode_NotImplemented);
     }
 
-    data_.resize(height * pitch);
+    uint64_t totalSize = pitch * height;
+    if (totalSize > MAX_DECODED_PNG_IMAGE_SIZE ||
+        static_cast<uint64_t>(static_cast<size_t>(totalSize)) != totalSize)
+    {
+      std::ostringstream errorMessage;
+      errorMessage << "PNG IMAGE size overflow  (" << totalSize << " vs " << MAX_DECODED_PNG_IMAGE_SIZE << ")";
+      throw OrthancException(ErrorCode_BadFileFormat, errorMessage.str());
+    }
+
+    data_.resize(static_cast<size_t>(totalSize));
 
     if (height == 0 || width == 0)
     {