changeset 6661:be4a4f1ccaf4 machine-spirits

fix possible memory exhaustion via forged ZIP metadata
author Sebastien Jodogne <s.jodogne@gmail.com>
date Sun, 22 Mar 2026 11:05:50 +0100
parents 5d0d0d41e00a
children 344064be379d
files NEWS OrthancFramework/Sources/Compatibility.h OrthancFramework/Sources/Compression/ZipReader.cpp OrthancFramework/Sources/Compression/ZipReader.h OrthancFramework/Sources/MultiThreading/ReaderWriterLock.h OrthancServer/Resources/Configuration.json OrthancServer/Sources/main.cpp
diffstat 7 files changed, 207 insertions(+), 9 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Fri Mar 20 17:35:32 2026 +0100
+++ b/NEWS	Sun Mar 22 11:05:50 2026 +0100
@@ -57,13 +57,14 @@
 * Save the jobs registry in DB only if it has changed.
   https://discourse.orthanc-server.org/t/frequent-idle-messages-between-postgres-and-orthanc/6406
 * New CMake option: "USE_SYSTEM_MINIZIP" to use the system-wide version of minizip
-* Security fixes courtesy of Machine Spirits UG:
+* Fixes for security issues reported Machine Spirits UG:
   - Fix possible out-of-bound access when calling /tools/create-dicom with a PAM file.
   - Fix possible out-of-bound access when rows/columns DICOM tags exceed the maximum value for a US (65535).
   - Fix possible out-of-bound access in PMSCT_RLE1 encoded images.
   - Fix possible out-of-bound access in palette images.
   - Fix possible out-of-bound access when reading a DICOM file with invalid group length tag.
   - Fix possible memory exhaustion via very large "Content-Length"
+  - Fix possible memory exhaustion via forged ZIP metadata
 * Upgraded dependencies for static builds:
   - boost 1.89.0
   - dcmtk 3.7.0
--- a/OrthancFramework/Sources/Compatibility.h	Fri Mar 20 17:35:32 2026 +0100
+++ b/OrthancFramework/Sources/Compatibility.h	Sun Mar 22 11:05:50 2026 +0100
@@ -72,10 +72,12 @@
 // The override keyword (C++11) is enabled
 #  define ORTHANC_OVERRIDE  override 
 #  define ORTHANC_FINAL     final
+#  define ORTHANC_NOEXCEPT  noexcept
 #else
 // The override keyword (C++11) is not available
 #  define ORTHANC_OVERRIDE
 #  define ORTHANC_FINAL
+#  define ORTHANC_NOEXCEPT
 #endif
 
 
--- a/OrthancFramework/Sources/Compression/ZipReader.cpp	Fri Mar 20 17:35:32 2026 +0100
+++ b/OrthancFramework/Sources/Compression/ZipReader.cpp	Sun Mar 22 11:05:50 2026 +0100
@@ -41,7 +41,9 @@
 #endif
 
 
+#include "../ChunkedBuffer.h"
 #include "../OrthancException.h"
+#include "../MultiThreading/ReaderWriterLock.h"
 
 #if ORTHANC_SANDBOXED != 1
 #  include "../SystemToolbox.h"
@@ -66,6 +68,11 @@
 #include <string.h>
 
 
+static Orthanc::ReaderWriterLock mutex_;
+static bool hasMaximumUncompressedFileSize_ = false;
+static size_t maximumUncompressedFileSize_ = 0;
+
+
 namespace Orthanc
 {
   // ZPOS64_T corresponds to "uint64_t"
@@ -323,6 +330,62 @@
   }
 
 
+  static bool ReadInternal(std::string& content,
+                           unzFile& unzip,
+                           uint64_t size) ORTHANC_NOEXCEPT
+  {
+#if 0
+    /**
+     * This was the code using in Orthanc <= 1.12.10
+     **/
+    content.resize(static_cast<size_t>(size));
+    return (unzReadCurrentFile(unzip, &content[0], static_cast<uLong>(content.size())) != 0);
+
+#else
+    /**
+     * Read chunk by chunk to disarm ZIP bombs
+     **/
+    ChunkedBuffer buffer;
+
+    std::string chunk;
+    chunk.resize(10 * 1024 * 1024); // Read by chunks of 10MB
+
+    for (;;)
+    {
+      int r = unzReadCurrentFile(unzip, &chunk[0], chunk.size());
+
+      if (r == 0)
+      {
+        // We're done
+        if (buffer.GetNumBytes() == size)
+        {
+          buffer.Flatten(content);
+          return true;
+        }
+        else
+        {
+          return false;  // Presumably a malicious ZIP file
+        }
+      }
+      else if (r < 0)
+      {
+        // Error (might come from a malicious ZIP file)
+        return false;
+      }
+      else if (static_cast<uint64_t>(buffer.GetNumBytes()) + r > size)
+      {
+        // Presumably a ZIP bomb
+        return false;
+      }
+      else
+      {
+        buffer.AddChunk(&chunk[0], r);
+      }
+    }
+#endif
+  }
+
+
   bool ZipReader::ReadNextFile(std::string& filename,
                                std::string& content)
   {
@@ -340,6 +403,12 @@
         throw OrthancException(ErrorCode_BadFileFormat);
       }
 
+      if (static_cast<uint64_t>(static_cast<size_t>(info.uncompressed_size)) != info.uncompressed_size)
+      {
+        // Too large file for a 32bit architecture
+        throw OrthancException(ErrorCode_NotEnoughMemory);
+      }
+
       filename.resize(info.size_filename);
       if (!filename.empty() &&
           unzGetCurrentFileInfo64(pimpl_->unzip_, &info, &filename[0],
@@ -348,14 +417,28 @@
         throw OrthancException(ErrorCode_BadFileFormat);
       }
 
-      content.resize(info.uncompressed_size);
+      {
+        // Prevent ZIP bombs
+        ReaderWriterLock::ReadLock lock(mutex_);
 
-      if (!content.empty())
+        if (hasMaximumUncompressedFileSize_ &&
+            info.uncompressed_size > maximumUncompressedFileSize_)
+        {
+          char s[32];
+          sprintf(s, "%0.2f", static_cast<float>(info.uncompressed_size) / (1024.0f * 1024.0f));
+          throw OrthancException(ErrorCode_BadFileFormat, "Uncompressed size exceeds limit: " + std::string(s) + "MB");
+        }
+      }
+
+      if (info.uncompressed_size == 0)
+      {
+        content.clear();
+      }
+      else
       {
         if (unzOpenCurrentFile(pimpl_->unzip_) == 0)
         {
-          bool success = (unzReadCurrentFile(pimpl_->unzip_, &content[0],
-                                             static_cast<uLong>(content.size())) != 0);
+          bool success = ReadInternal(content, pimpl_->unzip_, info.uncompressed_size);
                           
           if (unzCloseCurrentFile(pimpl_->unzip_) != 0 ||
               !success)
@@ -442,4 +525,19 @@
     }
   }
 #endif
+
+
+  void ZipReader::SetMaximumUncompressedFileSize(uint64_t size)
+  {
+    if (static_cast<uint64_t>(static_cast<size_t>(size)) != size)
+    {
+      throw OrthancException(ErrorCode_NotEnoughMemory);
+    }
+    else
+    {
+      ReaderWriterLock::WriteLock lock(mutex_);
+      hasMaximumUncompressedFileSize_ = true;
+      maximumUncompressedFileSize_ = size;
+    }
+  }
 }
--- a/OrthancFramework/Sources/Compression/ZipReader.h	Fri Mar 20 17:35:32 2026 +0100
+++ b/OrthancFramework/Sources/Compression/ZipReader.h	Sun Mar 22 11:05:50 2026 +0100
@@ -87,5 +87,7 @@
 #if ORTHANC_SANDBOXED != 1
     static bool IsZipFile(const boost::filesystem::path& path);
 #endif
+
+    static void SetMaximumUncompressedFileSize(uint64_t size);
   };
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/OrthancFramework/Sources/MultiThreading/ReaderWriterLock.h	Sun Mar 22 11:05:50 2026 +0100
@@ -0,0 +1,76 @@
+/**
+ * Orthanc - A Lightweight, RESTful DICOM Store
+ * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics
+ * Department, University Hospital of Liege, Belgium
+ * Copyright (C) 2017-2023 Osimis S.A., Belgium
+ * Copyright (C) 2024-2026 Orthanc Team SRL, Belgium
+ * Copyright (C) 2021-2026 Sebastien Jodogne, ICTEAM UCLouvain, Belgium
+ *
+ * This program is free software: you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation, either version 3 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this program. If not, see
+ * <http://www.gnu.org/licenses/>.
+ **/
+
+
+#pragma once
+
+#include "../OrthancFramework.h"
+
+#if !defined(__EMSCRIPTEN__)
+// Multithreading is not supported in WebAssembly
+#  include <boost/thread/shared_mutex.hpp>
+#  include <boost/thread/lock_types.hpp>  // For boost::unique_lock<> and boost::shared_lock<>
+#endif
+
+
+namespace Orthanc
+{
+  class ORTHANC_PUBLIC ReaderWriterLock : public boost::noncopyable
+  {
+  private:
+    boost::shared_mutex mutex_;
+
+  public:
+    class ReadLock : public boost::noncopyable
+    {
+    private:
+#if !defined(__EMSCRIPTEN__)
+      boost::shared_lock<boost::shared_mutex> lock_;
+#endif
+
+    public:
+      explicit ReadLock(ReaderWriterLock& that)
+#if !defined(__EMSCRIPTEN__)
+        : lock_(that.mutex_)
+#endif
+      {
+      }
+    };
+
+    class WriteLock : public boost::noncopyable
+    {
+    private:
+#if !defined(__EMSCRIPTEN__)
+      boost::unique_lock<boost::shared_mutex> lock_;
+#endif
+
+    public:
+      explicit WriteLock(ReaderWriterLock& that)
+#if !defined(__EMSCRIPTEN__)
+        : lock_(that.mutex_)
+#endif
+      {
+      }
+    };
+  };
+}
--- a/OrthancServer/Resources/Configuration.json	Fri Mar 20 17:35:32 2026 +0100
+++ b/OrthancServer/Resources/Configuration.json	Sun Mar 22 11:05:50 2026 +0100
@@ -1108,11 +1108,17 @@
   // "/tools/create-archives" routes. (new in Orthanc 1.12.11)
   "ZipUseUtf8" : false,
 
-  // Maximum body size allowed in a HTTP request (POST or PUT) to prevent
-  // exhaustion of resources, expressed in MB. A value of "0" indicates
-  // no limit on the body size. (new in Orthanc 1.12.11)
+  // Maximum allowed size (in MB) of the body of an HTTP request (POST
+  // or PUT), to prevent resource exhaustion. A value of "0" means no
+  // limit (default in Orthanc <= 1.12.10). (new in Orthanc 1.12.11)
   "MaximumRequestBodySizeMB" : 2048,
 
+  // Maximum allowed size (in MB) of a file after decompression when
+  // it is contained in a ZIP or gzip archive, to prevent resource
+  // exhaustion by ZIP/gzip bombs. A value of "0" means no limit (this
+  // was the default in Orthanc <= 1.12.10). (new in Orthanc 1.12.11)
+  "MaximumFileSizeInArchiveMB" : 512,
+
   // When set to false, this option disables all /patients routes and
   // disables patients related sanity checks when performing resource
   // modification.  This is required e.g when your Orthanc stores
--- a/OrthancServer/Sources/main.cpp	Fri Mar 20 17:35:32 2026 +0100
+++ b/OrthancServer/Sources/main.cpp	Sun Mar 22 11:05:50 2026 +0100
@@ -25,6 +25,7 @@
 #include "OrthancRestApi/OrthancRestApi.h"
 
 #include "../../OrthancFramework/Sources/Compatibility.h"
+#include "../../OrthancFramework/Sources/Compression/ZipReader.h"
 #include "../../OrthancFramework/Sources/DicomFormat/DicomArray.h"
 #include "../../OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.h"
 #include "../../OrthancFramework/Sources/DicomNetworking/DicomServer.h"
@@ -47,8 +48,8 @@
 #include "OrthancWebDav.h"
 #include "ServerContext.h"
 #include "ServerEnumerations.h"
+#include "ServerJobs/DicomRetrieveScuBaseJob.h"
 #include "ServerJobs/StorageCommitmentScpJob.h"
-#include "ServerJobs/DicomRetrieveScuBaseJob.h"
 #include "ServerToolbox.h"
 #include "StorageCommitmentReports.h"
 
@@ -1139,6 +1140,18 @@
         LOG(WARNING) << "No limit on the maximum body size in HTTP requests";
       }
 
+      const unsigned int maxSizeInArchive = lock.GetConfiguration().GetUnsignedIntegerParameter("MaximumFileSizeInArchiveMB", 512);
+      if (maxSizeInArchive != 0)
+      {
+        LOG(WARNING) << "Limiting on the maximum file size uncompressed from ZIP/gzip archives to " << maxSizeInArchive << "MB";
+        ZipReader::SetMaximumUncompressedFileSize(static_cast<uint64_t>(maxSizeInArchive) *
+                                                  static_cast<uint64_t>(1024 * 1024));
+      }
+      else
+      {
+        LOG(WARNING) << "No limit on the maximum file size uncompressed from ZIP/gzip archives";
+      }
+
       // Let's assume that the HTTP server is secure
       context.SetHttpServerSecure(true);