Skip to content
Branch: master
Find file History
Pull request Compare This branch is 1 commit ahead, 3 commits behind mudongliang:master.
Fetching latest commit…
Cannot retrieve the latest commit at this time.
Permalink
Type Name Latest commit message Commit time
..
Failed to load latest commit information.
CVE-2015-8396-trigger.cpp
CVE-2015-8396.dcm
README.md

README.md

CVE-2015-8396

Experiment Environment

Ubuntu 14.04LTS

INSTALL & Configuration

wget https://github.com/mudongliang/source-packages/raw/master/CVE-2015-8396/v2.6.0.tar.gz
tar -xvf v2.6.0.tar.gz
cd gdcm
mkdir build
cd build
cmake ../
make
sudo make install

Problems in Installation & Configuration

How to trigger vulnerability

g++ -I/usr/include/gdcm-2.6 -o CVE-2015-8396-trigger CVE-2015-8396-trigger.cpp -lgdcmCommon -lgdcmMSFF -lgdcmDSED
./CVE-2015-8396-trigger CVE-2015-8396.dcm

PoCs

Grassroots DICOM (GDCM) 2.6.0 and 2.6.1 - ImageRegionReader::ReadIntoBuffer Buffer Overflow

GDCM buffer overflow in ImageRegionReader :: ReadIntoBuffer

Vulnerability Details & Patch

Root Cause

The integer overflow takes place when the application attempts to calculate the DICOM image size. In file gdcmBoxRegion.cxx, line 85 we have:

size_t BoxRegion::Area() const
{
  return (Internals->YMax — Internals->YMin + 1)*
         (Internals->XMax — Internals->XMin + 1)*
         (Internals->ZMax — Internals->ZMin + 1);
}

The above variables represent the dimensions of the DICOM image region that is to be copied, and are set via a call to gdcm::ImageRegionReader::SetRegion(). In the most common case the region covers the entire image and is therefore controlled by the input file's DICOM headers, where the image dimensions are specified. Specially crafted dimensions can cause the multiplication to wrap around zero, thus making the return value smaller than the real size requirements.

The return value is eventually saved in variable 'thelen' and then used in the buffer length check of gdcmImageRegionReader.cxx, line 445:

bool ImageRegionReader::ReadIntoBuffer(char *buffer, size_t buflen)
{
  size_t thelen = ComputeBufferLength();
  if( buflen < thelen )
    {
    gdcmDebugMacro( "buffer cannot be smaller than computed buffer length" );
    return false;
    }

  assert( Internals->GetFileOffset() != std::streampos(-1) );
  gdcmDebugMacro( "Using FileOffset: " << Internals->GetFileOffset() );
  std::istream* theStream = GetStreamPtr();
  theStream->seekg( Internals->GetFileOffset() );

  bool success = false;
  if( !success ) success = ReadRAWIntoBuffer(buffer, buflen);
  if( !success ) success = ReadRLEIntoBuffer(buffer, buflen);
  if( !success ) success = ReadJPEGIntoBuffer(buffer, buflen);
  if( !success ) success = ReadJPEGLSIntoBuffer(buffer, buflen);
  if( !success ) success = ReadJPEG2000IntoBuffer(buffer, buflen);

  return success;
}

As long as the length check is passed, all of the decoding functions (ReadRAWIntoBuffer, etc.) will assume that the input buffer is long enough so they will copy the image data into the buffer without further checks.

The image copy operations are executed by a number of memcpy() calls, such as the following one from gdcmJPEGLSCodec.cxx, line 514:

  memcpy(&(buffer[((z-zmin)*rowsize*colsize + (y-ymin)*rowsize)*bytesPerPixel]),
         tmpBuffer1, rowsize*bytesPerPixel);

Stack Trace

Patch

--- a/Source/MediaStorageAndFileFormat/gdcmJPEGLSCodec.cxx
+++ b/Source/MediaStorageAndFileFormat/gdcmJPEGLSCodec.cxx
@@ -454,6 +454,12 @@
     const unsigned int colsize = ymax - ymin + 1;
     const unsigned int bytesPerPixel = pf.GetPixelSize();
 
+    if( outv.size() != dimensions[0] * dimensions[1] * bytesPerPixel )
+    {
+       gdcmDebugMacro( "Inconsistant buffer size. Giving up" );
+       return false;
+    }
+
     const unsigned char *tmpBuffer1 = raw;
     unsigned int z = 0;
     for (unsigned int y = ymin; y <= ymax; ++y)
@@ -509,6 +515,12 @@
       const unsigned int rowsize = xmax - xmin + 1;
       const unsigned int colsize = ymax - ymin + 1;
       const unsigned int bytesPerPixel = pf.GetPixelSize();
+
+      if( outv.size() != dimensions[0] * dimensions[1] * bytesPerPixel )
+      {
+         gdcmDebugMacro( "Inconsistant buffer size. Giving up" );
+         return false;
+      }
 
       const unsigned char *tmpBuffer1 = raw;
       for (unsigned int y = ymin; y <= ymax; ++y)

Details are in https://sourceforge.net/p/gdcm/gdcm/ci/e547b1ded3fd21e0b0ad149f13045aa12d4b9b7c/

References

You can’t perform that action at this time.