Skip to content

Commit 434c094

Browse files
garciadiaspre-commit-ci[bot]ericspod
authored
Reject non-finite DICOM affine metadata in PydicomReader (#9087)
### Description `PydicomReader._get_affine` builds the affine matrix from DICOM `PixelSpacing`, `ImagePositionPatient`, and `ImageOrientationPatient` values with no finite check. A crafted DICOM carrying `NaN`/`inf` in those DS tags produces a corrupted affine that propagates through spatial transforms and crashes MONAILabel inference or silently corrupts results. Validate all affine inputs with `math.isfinite()` and raise `ValueError` naming the offending tag before building the matrix (GHSA-6hp3-vr39-rqw8). ### Types of changes - [ ] Non-breaking change - [x] Breaking change (non-finite DICOM geometry now raises instead of silently proceeding) - [x] New tests added to cover the changes. --------- Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com>
1 parent 97843f8 commit 434c094

2 files changed

Lines changed: 85 additions & 0 deletions

File tree

monai/data/image_reader.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ def _get_affine(self, img, lps_to_ras: bool = True):
347347
affine: np.ndarray = np.eye(sr + 1)
348348
affine[:sr, :sr] = direction[:sr, :sr] @ np.diag(spacing[:sr])
349349
affine[:sr, -1] = origin[:sr]
350+
350351
if lps_to_ras:
351352
affine = orientation_ras_lps(affine)
352353
return affine
@@ -752,13 +753,25 @@ def _get_affine(self, metadata: dict, lps_to_ras: bool = True):
752753
stacklevel=2,
753754
)
754755
return affine
756+
757+
def _raise_if_not_finite(values: Sequence[Any], tag: str) -> None:
758+
if not np.isfinite(tuple(values)).all():
759+
raise ValueError(
760+
f"PydicomReader: cannot derive affine matrix because DICOM tag {tag} "
761+
f"has a non-finite value: {values}."
762+
)
763+
755764
# "00200037" is the tag of `ImageOrientationPatient`
756765
rx, ry, rz, cx, cy, cz = metadata["00200037"]["Value"]
766+
_raise_if_not_finite((rx, ry, rz, cx, cy, cz), "ImageOrientationPatient (0020,0037)")
757767
# "00200032" is the tag of `ImagePositionPatient`
758768
sx, sy, sz = metadata["00200032"]["Value"]
769+
_raise_if_not_finite((sx, sy, sz), "ImagePositionPatient (0020,0032)")
759770
# "00280030" is the tag of `PixelSpacing`
760771
spacing = metadata["00280030"]["Value"] if "00280030" in metadata else (1.0, 1.0)
772+
_raise_if_not_finite(tuple(spacing), "PixelSpacing (0028,0030)")
761773
dr, dc = metadata.get("spacing", spacing)[:2]
774+
_raise_if_not_finite((dr, dc), "spacing")
762775
affine[0, 0] = cx * dr
763776
affine[0, 1] = rx * dc
764777
affine[0, 3] = sx
@@ -773,12 +786,16 @@ def _get_affine(self, metadata: dict, lps_to_ras: bool = True):
773786
# 3d
774787
if "lastImagePositionPatient" in metadata:
775788
t1n, t2n, t3n = metadata["lastImagePositionPatient"]
789+
_raise_if_not_finite((t1n, t2n, t3n), "lastImagePositionPatient")
776790
n = metadata[MetaKeys.SPATIAL_SHAPE][-1]
777791
if n > 1:
778792
affine[0, 2] = (t1n - sx) / (n - 1)
779793
affine[1, 2] = (t2n - sy) / (n - 1)
780794
affine[2, 2] = (t3n - sz) / (n - 1)
781795

796+
if not np.isfinite(affine).all():
797+
raise ValueError("PydicomReader: affine matrix not finite after composition.")
798+
782799
if lps_to_ras:
783800
affine = orientation_ras_lps(affine)
784801
return affine

tests/data/test_pydicom_reader.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import numpy as np
1717

1818
from monai.data import PydicomReader
19+
from monai.utils import MetaKeys
1920
from tests.test_utils import SkipIfNoModule
2021

2122

@@ -39,6 +40,73 @@ def test_partial_orientation_tags_warns(self):
3940
affine = reader._get_affine(metadata)
4041
np.testing.assert_array_equal(affine, np.eye(4))
4142

43+
def test_non_finite_pixel_spacing_raises(self):
44+
reader = PydicomReader()
45+
metadata = {
46+
"00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]},
47+
"00200032": {"Value": [0.0, 0.0, 0.0]},
48+
"00280030": {"Value": [np.nan, 1.0]},
49+
}
50+
with self.assertRaisesRegex(ValueError, "PixelSpacing"):
51+
reader._get_affine(metadata, lps_to_ras=False)
52+
53+
def test_non_finite_image_position_raises(self):
54+
reader = PydicomReader()
55+
metadata = {
56+
"00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]},
57+
"00200032": {"Value": [np.inf, 0.0, 0.0]},
58+
"00280030": {"Value": [1.0, 1.0]},
59+
}
60+
with self.assertRaisesRegex(ValueError, "ImagePositionPatient"):
61+
reader._get_affine(metadata, lps_to_ras=False)
62+
63+
def test_finite_values_return_affine(self):
64+
reader = PydicomReader()
65+
metadata = {
66+
"00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]},
67+
"00200032": {"Value": [10.0, 20.0, 30.0]},
68+
"00280030": {"Value": [0.5, 0.25]},
69+
}
70+
affine = reader._get_affine(metadata, lps_to_ras=False)
71+
self.assertEqual(affine.shape, (4, 4))
72+
self.assertTrue(np.all(np.isfinite(affine)))
73+
np.testing.assert_allclose(affine[0, 3], 10.0)
74+
np.testing.assert_allclose(affine[1, 3], 20.0)
75+
np.testing.assert_allclose(affine[2, 3], 30.0)
76+
77+
def test_non_finite_orientation_raises(self):
78+
reader = PydicomReader()
79+
metadata = {
80+
"00200037": {"Value": [np.nan, 0.0, 0.0, 0.0, 1.0, 0.0]},
81+
"00200032": {"Value": [0.0, 0.0, 0.0]},
82+
"00280030": {"Value": [1.0, 1.0]},
83+
}
84+
with self.assertRaisesRegex(ValueError, "ImageOrientationPatient"):
85+
reader._get_affine(metadata, lps_to_ras=False)
86+
87+
def test_non_finite_last_image_position_raises(self):
88+
reader = PydicomReader()
89+
metadata = {
90+
"00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]},
91+
"00200032": {"Value": [0.0, 0.0, 0.0]},
92+
"00280030": {"Value": [1.0, 1.0]},
93+
"lastImagePositionPatient": [0.0, 0.0, np.inf],
94+
MetaKeys.SPATIAL_SHAPE: [1, 1, 2],
95+
}
96+
with self.assertRaisesRegex(ValueError, "lastImagePositionPatient"):
97+
reader._get_affine(metadata, lps_to_ras=False)
98+
99+
def test_overflow_from_finite_inputs_raises(self):
100+
# Finite inputs whose product overflows produce a non-finite affine.
101+
reader = PydicomReader()
102+
metadata = {
103+
"00200037": {"Value": [1e308, 0.0, 0.0, 1e308, 0.0, 0.0]},
104+
"00200032": {"Value": [0.0, 0.0, 0.0]},
105+
"00280030": {"Value": [1e308, 1e308]},
106+
}
107+
with self.assertRaisesRegex(ValueError, "not finite"):
108+
reader._get_affine(metadata, lps_to_ras=False)
109+
42110

43111
if __name__ == "__main__":
44112
unittest.main()

0 commit comments

Comments
 (0)