Talos Vulnerability Report

SUMMARY

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.

CONFIRMED VULNERABLE VERSIONS

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

PRODUCT URLS

Grassroot DICOM - https://sourceforge.net/projects/gdcm/

CVSSv3 SCORE

6.5 - CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:N/A:L

CWE

CWE-125 - Out-of-bounds Read

DETAILS

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.

Crash Information

=================================================================
==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

VENDOR RESPONSE

The vendor has fixed the code on their sourceforge site.

TIMELINE

2024-02-15 - Initial Vendor Contact
2024-02-20 - Vendor Disclosure
2024-02-21 - Vendor Patch Release
2024-04-25 - Public Release