cornerstonejs / dicomParser

JavaScript parser for DICOM Part 10 data
MIT License
712 stars 228 forks source link

Buffer overrun when encapsulated pixel data SequenceDelimitationItem (FFFE,E0DD) has undefined length #266

Open jmhmd opened 5 months ago

jmhmd commented 5 months ago

I have some test files which I think are non-compliant, but wondering if dicomParser's handling is optimal. These files have encapsulated pixel data with one defined length item containing the pixel data stream. However, it appears that the pixel data sequence delimitation item (FFFE,E0DD) has length = 4294967295 (undefined), rather than 0. I believe a length of 0 is required per the standard: https://dicom.nema.org/medical/dicom/current/output/chtml/part05/sect_7.5.2.html

dicomParser, however, takes whatever length is given and tries to seek to that position, which results in a buffer overrun error being thrown. Why doesn't dicomParser just assume a length of 0 for the FFFE,E0DD element as required by the standard? Relevant code here: https://github.com/cornerstonejs/dicomParser/blob/7d2084349bf2bdaffe74021e27b286a6c295ca66/src/findEndOfEncapsulatedPixelData.js#L50

Example file: https://drive.google.com/file/d/1tqXGAdfSq3H4oJA_w_dbhWJG2JrPapEx/view?usp=sharing

jmhmd commented 5 months ago

Running the file through dcmtk dcmconv without any options appears to correctly re-encode the file, and dicomParser parses without error. Here is the fixed file: https://drive.google.com/file/d/1-i64W77GhN7oLGK2CzO_MoD0xnCRmMFZ/view?usp=sharing