Mercurial > hg > orthanc
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);
