CVE-2024-25569
An out-of-bounds read vulnerability exists in the RAWCodec::DecodeBytes functionality of Mathieu Malaterre Grassroot DICOM 3.0.23. A specially crafted DICOM file can lead to an out-of-bounds read. An attacker can provide a malicious file to trigger this vulnerability.
The versions below were either tested or verified to be vulnerable by Talos or confirmed to be vulnerable by the vendor.
Grassroot DICOM 3.0.23
Grassroot DICOM - https://sourceforge.net/projects/gdcm/
6.5 - CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:N/A:L
CWE-125 - Out-of-bounds Read
Grassroots DiCoM is a C++ library for DICOM medical files. It is accessible from Python, C#, Java and PHP. It supports RAW, JPEG, JPEG 2000, JPEG-LS, RLE and deflated transfer syntax. It comes with a super fast scanner implementation to quickly scan hundreds of DICOM files. It supports SCU network operations (C-ECHO, C-FIND, C-STORE, C-MOVE). PS 3.3 & 3.6 are distributed as XML files. It also provides PS 3.15 certificates and password based mecanism to anonymize and de-identify DICOM dataset
A specially-crafted DICOM file can lead to an out-of-bounds in gdcm::RAWCodec::DecodeBytes, due to a memcpy by a missing size check of the source memory buffer.
Below some extract of the function and the crash is happening in LINE70 :
LINE1 bool RAWCodec::DecodeBytes(const char* inBytes, size_t inBufferLength,
LINE2 char* outBytes, size_t inOutBufferLength)
LINE3 {
LINE4 // First let's see if we can do a fast-path:
LINE5 if( !NeedByteSwap &&
LINE6 !RequestPaddedCompositePixelCode &&
LINE7 PI != PhotometricInterpretation::YBR_FULL_422 &&
LINE8 /*!PlanarConfiguration &&*/ !RequestPlanarConfiguration &&
LINE9 GetPixelFormat().GetBitsAllocated() != 12 &&
LINE10 !NeedOverlayCleanup )
LINE11 {
[...]
LINE35 }
LINE36 // else
LINE37 assert( inBytes );
LINE38 assert( outBytes );
LINE39 std::stringstream is;
LINE40 is.write(inBytes, inBufferLength);
LINE41 std::stringstream os;
LINE42 bool r = DecodeByStreams(is, os);
LINE43 assert( r );
LINE44 if(!r) return false;
LINE45
LINE46 std::string str = os.str();
LINE47 //std::string::size_type check = str.size();//unused
LINE48
LINE49
LINE50 if( this->GetPixelFormat() == PixelFormat::UINT12 ||
LINE51 this->GetPixelFormat() == PixelFormat::INT12 )
LINE52 {
[...]
LINE64 }
LINE65 else
LINE66 {
LINE67 // DermaColorLossLess.dcm
LINE68 //assert (check == inOutBufferLength || check == inOutBufferLength + 1);
LINE69 // problem with: SIEMENS_GBS_III-16-ACR_NEMA_1.acr
LINE70 memcpy(outBytes, str.c_str(), inOutBufferLength);
LINE71 }
LINE72
LINE73 return r;
LINE74 }
Through the process up to LINE70 , there is no length checks performed between str and inOutBufferLength.
inOutBufferLength is passed as argument and come from the function Bitmap::TryRAWCodec
LINE75 bool Bitmap::TryRAWCodec(char *buffer, bool &lossyflag) const
LINE76 {
LINE77 RAWCodec codec;
LINE78 const TransferSyntax &ts = GetTransferSyntax();
[...]
LINE93 const ByteValue *bv = PixelData.GetByteValue();
LINE94 if( bv )
LINE95 {
LINE96 unsigned long len = GetBufferLength();
LINE97 if( !codec.CanDecode( ts ) ) return false;
LINE98 codec.SetPlanarConfiguration( GetPlanarConfiguration() );
LINE99 codec.SetPhotometricInterpretation( GetPhotometricInterpretation() );
LINE100 codec.SetLUT( GetLUT() );
LINE101 codec.SetPixelFormat( GetPixelFormat() );
LINE102 codec.SetNeedByteSwap( GetNeedByteSwap() );
LINE103 codec.SetNeedOverlayCleanup( AreOverlaysInPixelData() || UnusedBitsPresentInPixelData() );
LINE104 DataElement out;
LINE105 //bool r = codec.Decode(PixelData, out);
LINE106 bool r = codec.DecodeBytes(bv->GetPointer(), bv->GetLength(),
LINE107 buffer, len);
[...]
LINE144 }
LINE106-LINE107 is the call to RAWCodec::DecodeBytes with a len variable length. This len is computed from GetBufferLength LINE96 and is directly driven by values coming from the DICOM file.
Below for an overview the function Bitmap::GetBufferLength which is computing the len value is relying over a couple of DICOM tags :
LINE145 unsigned long Bitmap::GetBufferLength() const
LINE146 {
[...]
LINE156 unsigned long len = 0;
LINE157 unsigned int mul = 1;
LINE158 // First multiply the dimensions:
LINE159 std::vector<unsigned int>::const_iterator it = Dimensions.begin();
LINE160 for(; it != Dimensions.end(); ++it)
LINE161 {
LINE162 if( *it == 0 ) { gdcmWarningMacro("Dimension has been found to be zero" ); }
LINE163 mul *= *it;
LINE164 }
LINE165 // Multiply by the pixel size:
LINE166 // Special handling of packed format:
LINE167 if( PF == PixelFormat::UINT12 || PF == PixelFormat::INT12 )
LINE168 {
LINE169 #if 1
LINE170 mul *= PF.GetPixelSize();
LINE171 #else
LINE172 assert( PF.GetSamplesPerPixel() == 1 );
LINE173 unsigned int save = mul;
LINE174 save *= 12;
LINE175 save /= 8;
LINE176 assert( save * 8 / 12 == mul );
LINE177 mul = save;
LINE178 #endif
LINE179 }
LINE180 else if( PF == PixelFormat::SINGLEBIT )
LINE181 {
LINE182 assert( PF.GetSamplesPerPixel() == 1 );
LINE183 const size_t bytesPerRow = Dimensions[0] / 8 + (Dimensions[0] % 8 != 0 ? 1 : 0);
LINE184 size_t save = bytesPerRow * Dimensions[1];
LINE185 if( NumberOfDimensions > 2 )
LINE186 save *= Dimensions[2];
LINE187 if(Dimensions[0] % 8 == 0 )
LINE188 assert( save * 8 == mul );
LINE189 mul = (unsigned int)save;
LINE190 }
LINE191 else if( PF.GetBitsAllocated() % 8 != 0 )
LINE192 {
LINE193 // gdcmDataExtra/gdcmSampleData/images_of_interest/USBitsAllocated14.dcm
LINE194 // BitsAllocated :14
LINE195 // BitsStored :14
LINE196 // HighBit :13
LINE197 assert( PF.GetSamplesPerPixel() == 1 );
LINE198 const ByteValue *bv = PixelData.GetByteValue();
LINE199 assert( bv );
LINE200 unsigned int ref = bv->GetLength() / mul;
LINE201 if( !GetTransferSyntax().IsEncapsulated() )
LINE202 assert( bv->GetLength() % mul == 0 );
LINE203 mul *= ref;
LINE204 }
LINE205 else
LINE206 {
LINE207 mul *= PF.GetPixelSize();
LINE208 }
LINE209 len = mul;
LINE210
LINE211 //assert( len != 0 );
LINE212 return len;
LINE213 }
LINE209 is a local len result returned through the result corresponding to mul, derived from Dimensions LINE160, and depending of the file can be the result again by the multiplication to PF.GetPixelSize() LINE207. Below the code corresponding to GetPixelSize
LINE214 uint8_t PixelFormat::GetPixelSize() const
LINE215 {
LINE216 uint8_t pixelsize = (uint8_t)(BitsAllocated / 8);
LINE217 if( BitsAllocated == 12 )
LINE218 {
LINE219 pixelsize = 2; // fake a short value
LINE220 }
LINE221 else
LINE222 {
LINE223 assert( !(BitsAllocated % 8) );
LINE224 }
LINE225 pixelsize *= SamplesPerPixel;
LINE226
LINE227 return pixelsize;
LINE228 }
Basically PF.GetPixelSize() is depending of BitsAllocated LINE216 and SamplesPerPixel LINE225. Thus the len passed in argument LINE96 is totally under control and dependant of tags DICOM of the malformed file. str at LINE70 length is also dependant of the content of the file and is correponding to the DICOM tag Pixel Data length, making this out-of-bounds read possible by controlling all lengths involved in the memcpy.
=================================================================
==3151==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x621000007501 at pc 0x563c6ec43647 bp 0x7ffed4b84950 sp 0x7ffed4b84120
READ of size 524288 at 0x621000007501 thread T0
#0 0x563c6ec43646 in __asan_memcpy (/home/manu/gdcm_3_0_23_builds/asan/bin/CompressImage+0xa1646) (BuildId: 185df4d685b8d63866de5c20436d6884ca503318)
#1 0x7fe43e79c320 in gdcm::RAWCodec::DecodeBytes(char const*, unsigned long, char*, unsigned long) /home/manu/gdcm-3.0.23/Source/MediaStorageAndFileFormat/gdcmRAWCodec.cxx:138:5
#2 0x7fe43e5ed6e2 in gdcm::Bitmap::TryRAWCodec(char*, bool&) const /home/manu/gdcm-3.0.23/Source/MediaStorageAndFileFormat/gdcmBitmap.cxx:351:20
#3 0x7fe43e5f1587 in gdcm::Bitmap::GetBufferInternal(char*, bool&) const /home/manu/gdcm-3.0.23/Source/MediaStorageAndFileFormat/gdcmBitmap.cxx:999:28
#4 0x7fe43e5f3311 in gdcm::Bitmap::GetBuffer(char*) const /home/manu/gdcm-3.0.23/Source/MediaStorageAndFileFormat/gdcmBitmap.cxx:993:10
#5 0x7fe43e61c4c4 in gdcm::ImageChangeTransferSyntax::Change() /home/manu/gdcm-3.0.23/Source/MediaStorageAndFileFormat/gdcmImageChangeTransferSyntax.cxx:420:21
#6 0x563c6ec81954 in main /home/manu/gdcm-3.0.23/Examples/Cxx/CompressImage.cxx:63:19
#7 0x7fe43d429d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#8 0x7fe43d429e3f in __libc_start_main csu/../csu/libc-start.c:392:3
#9 0x563c6ebc14d4 in _start (/home/manu/gdcm_3_0_23_builds/asan/bin/CompressImage+0x1f4d4) (BuildId: 185df4d685b8d63866de5c20436d6884ca503318)
0x621000007501 is located 0 bytes to the right of 4097-byte region [0x621000006500,0x621000007501)
allocated by thread T0 here:
#0 0x563c6ec7f0ed in operator new(unsigned long) (/home/manu/gdcm_3_0_23_builds/asan/bin/CompressImage+0xdd0ed) (BuildId: 185df4d685b8d63866de5c20436d6884ca503318)
#1 0x7fe43d94c0bd in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_mutate(unsigned long, unsigned long, char const*, unsigned long) (/lib/x86_64-linux-gnu/libstdc++.so.6+0x14c0bd) (BuildId: e37fe1a879783838de78cbc8c80621fa685d58a2)
SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/manu/gdcm_3_0_23_builds/asan/bin/CompressImage+0xa1646) (BuildId: 185df4d685b8d63866de5c20436d6884ca503318) in __asan_memcpy
Shadow bytes around the buggy address:
0x0c427fff8e50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c427fff8e60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c427fff8e70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c427fff8e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c427fff8e90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c427fff8ea0:[01]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c427fff8eb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c427fff8ec0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c427fff8ed0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c427fff8ee0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c427fff8ef0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==3151==ABORTING
The vendor has fixed the code on their sourceforge site.
2024-02-15 - Initial Vendor Contact
2024-02-20 - Vendor Disclosure
2024-02-21 - Vendor Patch Release
2024-04-25 - Public Release
Discovered by Emmanuel Tacheau of Cisco Talos.