From ce848e5f9d5deba5281c27ea44a98aa21afb44fe Mon Sep 17 00:00:00 2001 From: "R. Garcia-Dias" Date: Thu, 3 Sep 2026 13:05:09 +0100 Subject: [PATCH 1/4] fix(pydicom): reject non-finite affine metadata in PydicomReader Validate ImageOrientationPatient, ImagePositionPatient, PixelSpacing and lastImagePositionPatient values with math.isfinite before building the affine matrix, raising ValueError on NaN/inf instead of propagating corrupted geometry (GHSA-6hp3-vr39-rqw8). Signed-off-by: R. Garcia-Dias --- monai/data/image_reader.py | 19 +++++++++++++++++ tests/data/test_pydicom_reader.py | 34 +++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/monai/data/image_reader.py b/monai/data/image_reader.py index 27ac4d8287..5404fcaad8 100644 --- a/monai/data/image_reader.py +++ b/monai/data/image_reader.py @@ -14,6 +14,7 @@ import glob import gzip import io +import math import os import re import tempfile @@ -752,13 +753,29 @@ def _get_affine(self, metadata: dict, lps_to_ras: bool = True): stacklevel=2, ) return affine + + def _raise_if_not_finite(value: Any, tag: str) -> None: + if not math.isfinite(value): + raise ValueError( + f"PydicomReader: cannot derive affine matrix because DICOM tag {tag} " + f"has a non-finite value: {value}." + ) + # "00200037" is the tag of `ImageOrientationPatient` rx, ry, rz, cx, cy, cz = metadata["00200037"]["Value"] + for value in (rx, ry, rz, cx, cy, cz): + _raise_if_not_finite(value, "ImageOrientationPatient (0020,0037)") # "00200032" is the tag of `ImagePositionPatient` sx, sy, sz = metadata["00200032"]["Value"] + for value in (sx, sy, sz): + _raise_if_not_finite(value, "ImagePositionPatient (0020,0032)") # "00280030" is the tag of `PixelSpacing` spacing = metadata["00280030"]["Value"] if "00280030" in metadata else (1.0, 1.0) + for value in spacing: + _raise_if_not_finite(value, "PixelSpacing (0028,0030)") dr, dc = metadata.get("spacing", spacing)[:2] + _raise_if_not_finite(dr, "spacing") + _raise_if_not_finite(dc, "spacing") affine[0, 0] = cx * dr affine[0, 1] = rx * dc affine[0, 3] = sx @@ -773,6 +790,8 @@ def _get_affine(self, metadata: dict, lps_to_ras: bool = True): # 3d if "lastImagePositionPatient" in metadata: t1n, t2n, t3n = metadata["lastImagePositionPatient"] + for value in (t1n, t2n, t3n): + _raise_if_not_finite(value, "lastImagePositionPatient") n = metadata[MetaKeys.SPATIAL_SHAPE][-1] if n > 1: affine[0, 2] = (t1n - sx) / (n - 1) diff --git a/tests/data/test_pydicom_reader.py b/tests/data/test_pydicom_reader.py index 1e55ee7a4e..ede8a93b94 100644 --- a/tests/data/test_pydicom_reader.py +++ b/tests/data/test_pydicom_reader.py @@ -39,6 +39,40 @@ def test_partial_orientation_tags_warns(self): affine = reader._get_affine(metadata) np.testing.assert_array_equal(affine, np.eye(4)) + def test_non_finite_pixel_spacing_raises(self): + reader = PydicomReader() + metadata = { + "00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]}, + "00200032": {"Value": [0.0, 0.0, 0.0]}, + "00280030": {"Value": [np.nan, 1.0]}, + } + with self.assertRaisesRegex(ValueError, "PixelSpacing"): + reader._get_affine(metadata, lps_to_ras=False) + + def test_non_finite_image_position_raises(self): + reader = PydicomReader() + metadata = { + "00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]}, + "00200032": {"Value": [np.inf, 0.0, 0.0]}, + "00280030": {"Value": [1.0, 1.0]}, + } + with self.assertRaisesRegex(ValueError, "ImagePositionPatient"): + reader._get_affine(metadata, lps_to_ras=False) + + def test_finite_values_return_affine(self): + reader = PydicomReader() + metadata = { + "00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]}, + "00200032": {"Value": [10.0, 20.0, 30.0]}, + "00280030": {"Value": [0.5, 0.25]}, + } + affine = reader._get_affine(metadata, lps_to_ras=False) + self.assertEqual(affine.shape, (4, 4)) + self.assertTrue(np.all(np.isfinite(affine))) + np.testing.assert_allclose(affine[0, 3], 10.0) + np.testing.assert_allclose(affine[1, 3], 20.0) + np.testing.assert_allclose(affine[2, 3], 30.0) + if __name__ == "__main__": unittest.main() From 5bf4e3fb19b3705f54a893f71f2f4db23483d22e Mon Sep 17 00:00:00 2001 From: "R. Garcia-Dias" Date: Thu, 3 Sep 2026 14:56:36 +0100 Subject: [PATCH 2/4] fix: address PR #9087 review feedback - vectorize finite validation with np.isfinite over each tag's value tuple - validate the composed affine is finite before the LPS-to-RAS flip - add regression tests for non-finite orientation, last image position, and finite inputs that overflow during affine composition Signed-off-by: R. Garcia-Dias --- monai/data/image_reader.py | 26 +++++++++++------------ tests/data/test_pydicom_reader.py | 35 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/monai/data/image_reader.py b/monai/data/image_reader.py index 5404fcaad8..53fcaa9545 100644 --- a/monai/data/image_reader.py +++ b/monai/data/image_reader.py @@ -14,7 +14,6 @@ import glob import gzip import io -import math import os import re import tempfile @@ -348,6 +347,7 @@ def _get_affine(self, img, lps_to_ras: bool = True): affine: np.ndarray = np.eye(sr + 1) affine[:sr, :sr] = direction[:sr, :sr] @ np.diag(spacing[:sr]) affine[:sr, -1] = origin[:sr] + if lps_to_ras: affine = orientation_ras_lps(affine) return affine @@ -754,28 +754,24 @@ def _get_affine(self, metadata: dict, lps_to_ras: bool = True): ) return affine - def _raise_if_not_finite(value: Any, tag: str) -> None: - if not math.isfinite(value): + def _raise_if_not_finite(values: Sequence[Any], tag: str) -> None: + if not np.isfinite(tuple(values)).all(): raise ValueError( f"PydicomReader: cannot derive affine matrix because DICOM tag {tag} " - f"has a non-finite value: {value}." + f"has a non-finite value: {values}." ) # "00200037" is the tag of `ImageOrientationPatient` rx, ry, rz, cx, cy, cz = metadata["00200037"]["Value"] - for value in (rx, ry, rz, cx, cy, cz): - _raise_if_not_finite(value, "ImageOrientationPatient (0020,0037)") + _raise_if_not_finite((rx, ry, rz, cx, cy, cz), "ImageOrientationPatient (0020,0037)") # "00200032" is the tag of `ImagePositionPatient` sx, sy, sz = metadata["00200032"]["Value"] - for value in (sx, sy, sz): - _raise_if_not_finite(value, "ImagePositionPatient (0020,0032)") + _raise_if_not_finite((sx, sy, sz), "ImagePositionPatient (0020,0032)") # "00280030" is the tag of `PixelSpacing` spacing = metadata["00280030"]["Value"] if "00280030" in metadata else (1.0, 1.0) - for value in spacing: - _raise_if_not_finite(value, "PixelSpacing (0028,0030)") + _raise_if_not_finite(tuple(spacing), "PixelSpacing (0028,0030)") dr, dc = metadata.get("spacing", spacing)[:2] - _raise_if_not_finite(dr, "spacing") - _raise_if_not_finite(dc, "spacing") + _raise_if_not_finite((dr, dc), "spacing") affine[0, 0] = cx * dr affine[0, 1] = rx * dc affine[0, 3] = sx @@ -790,14 +786,16 @@ def _raise_if_not_finite(value: Any, tag: str) -> None: # 3d if "lastImagePositionPatient" in metadata: t1n, t2n, t3n = metadata["lastImagePositionPatient"] - for value in (t1n, t2n, t3n): - _raise_if_not_finite(value, "lastImagePositionPatient") + _raise_if_not_finite((t1n, t2n, t3n), "lastImagePositionPatient") n = metadata[MetaKeys.SPATIAL_SHAPE][-1] if n > 1: affine[0, 2] = (t1n - sx) / (n - 1) affine[1, 2] = (t2n - sy) / (n - 1) affine[2, 2] = (t3n - sz) / (n - 1) + if not np.isfinite(affine).all(): + raise ValueError("PydicomReader: affine matrix not finite after composition.") + if lps_to_ras: affine = orientation_ras_lps(affine) return affine diff --git a/tests/data/test_pydicom_reader.py b/tests/data/test_pydicom_reader.py index ede8a93b94..eadb1d8b33 100644 --- a/tests/data/test_pydicom_reader.py +++ b/tests/data/test_pydicom_reader.py @@ -16,6 +16,7 @@ import numpy as np from monai.data import PydicomReader +from monai.utils import MetaKeys from tests.test_utils import SkipIfNoModule @@ -73,6 +74,40 @@ def test_finite_values_return_affine(self): np.testing.assert_allclose(affine[1, 3], 20.0) np.testing.assert_allclose(affine[2, 3], 30.0) + def test_non_finite_orientation_raises(self): + reader = PydicomReader() + metadata = { + "00200037": {"Value": [np.nan, 0.0, 0.0, 0.0, 1.0, 0.0]}, + "00200032": {"Value": [0.0, 0.0, 0.0]}, + "00280030": {"Value": [1.0, 1.0]}, + } + with self.assertRaisesRegex(ValueError, "ImageOrientationPatient"): + reader._get_affine(metadata, lps_to_ras=False) + + def test_non_finite_last_image_position_raises(self): + reader = PydicomReader() + metadata = { + "00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]}, + "00200032": {"Value": [0.0, 0.0, 0.0]}, + "00280030": {"Value": [1.0, 1.0]}, + "lastImagePositionPatient": [0.0, 0.0, np.inf], + MetaKeys.SPATIAL_SHAPE: [1, 1, 2], + } + with self.assertRaisesRegex(ValueError, "lastImagePositionPatient"): + reader._get_affine(metadata, lps_to_ras=False) + + def test_overflow_from_finite_inputs_raises(self): + # Finite inputs whose product overflows produce a non-finite affine. + reader = PydicomReader() + metadata = { + "00200037": {"Value": [1e308, 0.0, 0.0, 1e308, 0.0, 0.0]}, + "00200032": {"Value": [0.0, 0.0, 0.0]}, + "00280030": {"Value": [1e308, 1e308]}, + } + with self.assertRaisesRegex(ValueError, "not finite"): + reader._get_affine(metadata, lps_to_ras=False) + + if __name__ == "__main__": unittest.main() From 04cbfd5ffa66dcf614f04801c6322888be458292 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 3 Sep 2026 13:57:40 +0000 Subject: [PATCH 3/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/data/test_pydicom_reader.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/data/test_pydicom_reader.py b/tests/data/test_pydicom_reader.py index eadb1d8b33..42fcd89185 100644 --- a/tests/data/test_pydicom_reader.py +++ b/tests/data/test_pydicom_reader.py @@ -108,6 +108,5 @@ def test_overflow_from_finite_inputs_raises(self): reader._get_affine(metadata, lps_to_ras=False) - if __name__ == "__main__": unittest.main() From 22ddf44b20a67d45c955321cc2414b6a2593fe16 Mon Sep 17 00:00:00 2001 From: "R. Garcia-Dias" Date: Thu, 3 Sep 2026 15:16:43 +0100 Subject: [PATCH 4/4] chore: re-trigger CI