Skip to content

Commit

Permalink
BUG: Fix GDCM crash when reading DICOM image
Browse files Browse the repository at this point in the history
GDCM crashed when a DICOM image that had 32 bits allocated.
The problem was that DoOverlayCleanup returned failure but that return value was ignored and later an empty buffer read was attempted.

This fix avoids the crash by not ignoring the failed overlay cleanup.
Applications can detect the error and switch to DCMTK IO to read such files (DCMTK can correctly read files with 32 bits allocated).
  • Loading branch information
lassoan authored and malaterre committed Mar 21, 2024
1 parent dda17aa commit 9fe1cee
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions Source/MediaStorageAndFileFormat/gdcmImageCodec.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,7 @@ bool ImageCodec::DecodeByStreams(std::istream &is, std::ostream &os)

// Do the overlay cleanup (cleanup the unused bits)
// must be the last operation (duh!)
bool copySuccess = false;
if ( PF.GetBitsAllocated() != PF.GetBitsStored()
&& PF.GetBitsAllocated() != 8 )
{
Expand All @@ -685,21 +686,23 @@ bool ImageCodec::DecodeByStreams(std::istream &is, std::ostream &os)
// Sigh, I finally found someone not declaring that unused bits where not zero:
// gdcmConformanceTests/dcm4chee_unusedbits_not_zero.dcm
if( NeedOverlayCleanup )
DoOverlayCleanup(*cur_is,os);
{
copySuccess = DoOverlayCleanup(*cur_is, os);
}
else
{
// Once the issue with IMAGES/JPLY/RG3_JPLY aka gdcmData/D_CLUNIE_RG3_JPLY.dcm is solved the previous
// code will be replace with a simple call to:
DoSimpleCopy(*cur_is,os);
copySuccess = DoSimpleCopy(*cur_is, os);
}
}
else
{
assert( PF.GetBitsAllocated() == PF.GetBitsStored() );
DoSimpleCopy(*cur_is,os);
copySuccess = DoSimpleCopy(*cur_is, os);
}

return true;
return copySuccess;
}

bool ImageCodec::IsValid(PhotometricInterpretation const &)
Expand Down

0 comments on commit 9fe1cee

Please sign in to comment.