Skip to content

fix(dicom): null-check DCMTK pixel-data accessors before use - #5364

Merged
lgritz merged 2 commits into
AcademySoftwareFoundation:mainfrom
lgritz:lg-dicom
Aug 9, 2026
Merged

fix(dicom): null-check DCMTK pixel-data accessors before use#5364
lgritz merged 2 commits into
AcademySoftwareFoundation:mainfrom
lgritz:lg-dicom

Conversation

@lgritz

@lgritz lgritz commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

DicomImage::getInterData() and getOutputData() can return null when a corrupt DICOM's frame fails to decode, but their results were used without checking.

Assisted-by: Claude Code / Claude Opus 4.8

DicomImage::getInterData() and getOutputData() can return null when a
corrupt DICOM's frame fails to decode, but their results were used
without checking: getInterData() is immediately dereferenced via
getData()/getRepresentation(), and getOutputData()'s pointer is stored
and later memcpy'd in read_native_scanline(). The only guard was an
OIIO_DASSERT, which compiles out in release builds, leaving a
null-pointer dereference on malformed input. Check both and error
cleanly.

Assisted-by: Claude Code / Claude Opus 4.8

Signed-off-by: Larry Gritz <lg@larrygritz.com>

@jinhgkim jinhgkim left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few nits I encountered while reviewing...

Comment thread src/dicom.imageio/dicominput.cpp Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: redundant, already set in close()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quite right

Comment thread src/dicom.imageio/dicominput.cpp Outdated
m_img.reset();
return false;
}
m_internal_data = (const char*)m_dipixel->getData();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this assignment can be removed since it's being overwritten by line 204.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for those patches, which were not lines changed by this PR, but have been there for many years!

Signed-off-by: Larry Gritz <lg@larrygritz.com>
@lgritz
lgritz merged commit a94e645 into AcademySoftwareFoundation:main Aug 9, 2026
29 checks passed
@lgritz
lgritz deleted the lg-dicom branch August 9, 2026 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants