Mercurial > hg > orthanc
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, ¶meters, &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) {
