View Issue Details [ Jump to Notes ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0010529ITKpublic2010-04-09 10:302010-10-21 12:31
ReporterNicolas Savoire 
Assigned ToBradley Lowekamp 
PrioritynormalSeveritycrashReproducibilityalways
StatusclosedResolutionfixed 
PlatformLinuxOSUbuntu OS Version9.10
Product VersionITK-3-16 
Target VersionFixed in VersionITK-3-18 
Summary0010529: Reading a 2D image from a 3D leads to a crash
DescriptionWhen reading a 2D image from a 3D file sometimes leads to a crash depending on the ImageIO used. It happens when the requested image pixel type differs from the file pixel type and when using an ImageIO that use the default GenerateStreamableReadRegionFromRequestedRegion implementation or returns its largest possible region in GenerateStreamableReadRegionFromRequestedRegion.
The culprit is most probably itkImageFileReader.txx:414:
this->DoConvertBuffer(static_cast< void *>(loadBuffer), m_ActualIORegion.GetNumberOfPixels() );
When a type conversion is needed, the number of pixels of the actual IO region is used instead of number of pixels of the requested region, as a result a buffer overflow happens when the actual image io is larger than the requested region.
Steps To ReproduceAttached is a test file that triggers the crash.
TagsNo tags attached.
Resolution Date
Sprint
Sprint Status
Attached Filescxx file icon itkImageFileReaderBug.cxx [^] (775 bytes) 2010-04-09 10:30
patch file icon itkImageFileReader.patch [^] (665 bytes) 2010-04-09 10:37 [Show Content]

 Relationships

  Notes
(0020110)
Nicolas Savoire (reporter)
2010-04-09 10:37

 Proposed patch to fix the issue.
(0020111)
Luis Ibanez (manager)
2010-04-09 10:48

 Nicolas,

Could you please indicate with what image file formats you observe this problem ?

and also, it will be great if you could provide a minimal image that will allow us to replicate the problem.

Thanks

     Luis
(0020112)
Nicolas Savoire (reporter)
2010-04-09 10:57

 Luis,

The file itkImageFileReaderBug.cxx attached to this issue writes a vtk file and tries to read it to trigger the bug.
I managed to trigger the bug with the vtk file format and the meta file format if I disable the use of streaming.

Nicolas
(0020113)
Bradley Lowekamp (developer)
2010-04-09 11:04

 Nicolas,

This looks like a nice find!

The comments in the code look like they were helpful to tracking this down:

   loadBuffer = new char[ sizeOfActualIORegion ];
      m_ImageIO->Read( static_cast< void *>(loadBuffer) );
      
      this->DoConvertBuffer(static_cast< void *>(loadBuffer), m_ActualIORegion.GetNumberOfPixels() );
      }
    else if ( m_ActualIORegion.GetNumberOfPixels() != requestedRegion.GetNumberOfPixels() )
      {
      // for the number of pixels read and the number of pixels
      // requested to not match, the dimensions of the two regions may
      // be different, therefore we buffer and copy the pixels

      itkDebugMacro(<< "Buffer required because file dimension is greater then image dimension");
      
      OutputImagePixelType *outputBuffer = output->GetPixelContainer()->GetBufferPointer();
      
      loadBuffer = new char[ sizeOfActualIORegion ];
      m_ImageIO->Read( static_cast< void *>(loadBuffer) );

Essentially the combination of convert buffer with dimension reduction is not implemented correctly ( that is when conversion in needed and ActualIORegion != requestedRegion).

The fix looks correct to me.

I really thought I over tested some of this type of functionality...

Brad
(0021137)
Bradley Lowekamp (developer)
2010-06-22 15:27

 Patch was committed a while ago:
http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/IO/itkImageFileReader.txx?root=Insight&r1=1.89&r2=1.90 [^]
(0021138)
Bradley Lowekamp (developer)
2010-06-22 15:28

 The patch appears to have solved the issue

 Issue History
Date Modified Username Field Change
2010-04-09 10:30 Nicolas Savoire New Issue
2010-04-09 10:30 Nicolas Savoire File Added: itkImageFileReaderBug.cxx
2010-04-09 10:37 Nicolas Savoire File Added: itkImageFileReader.patch
2010-04-09 10:37 Nicolas Savoire Note Added: 0020110
2010-04-09 10:47 Luis Ibanez Status new => assigned
2010-04-09 10:47 Luis Ibanez Assigned To => Bradley Lowekamp
2010-04-09 10:48 Luis Ibanez Note Added: 0020111
2010-04-09 10:57 Nicolas Savoire Note Added: 0020112
2010-04-09 11:04 Bradley Lowekamp Note Added: 0020113
2010-06-22 15:27 Bradley Lowekamp Note Added: 0021137
2010-06-22 15:28 Bradley Lowekamp Note Added: 0021138
2010-06-22 15:28 Bradley Lowekamp Status assigned => resolved
2010-06-22 15:28 Bradley Lowekamp Fixed in Version => ITK-3-18
2010-06-22 15:28 Bradley Lowekamp Resolution open => fixed
2010-10-21 12:31 Gabe Hart Status resolved => closed

Copyright © 2000 - 2018 MantisBT Team