Skip to content

Commit ce848e5

Browse files
committed
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 <rafaelagd@gmail.com>
1 parent 7fe412b commit ce848e5

2 files changed

Lines changed: 53 additions & 0 deletions

File tree

monai/data/image_reader.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import glob
1515
import gzip
1616
import io
17+
import math
1718
import os
1819
import re
1920
import tempfile
@@ -752,13 +753,29 @@ 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(value: Any, tag: str) -> None:
758+
if not math.isfinite(value):
759+
raise ValueError(
760+
f"PydicomReader: cannot derive affine matrix because DICOM tag {tag} "
761+
f"has a non-finite value: {value}."
762+
)
763+
755764
# "00200037" is the tag of `ImageOrientationPatient`
756765
rx, ry, rz, cx, cy, cz = metadata["00200037"]["Value"]
766+
for value in (rx, ry, rz, cx, cy, cz):
767+
_raise_if_not_finite(value, "ImageOrientationPatient (0020,0037)")
757768
# "00200032" is the tag of `ImagePositionPatient`
758769
sx, sy, sz = metadata["00200032"]["Value"]
770+
for value in (sx, sy, sz):
771+
_raise_if_not_finite(value, "ImagePositionPatient (0020,0032)")
759772
# "00280030" is the tag of `PixelSpacing`
760773
spacing = metadata["00280030"]["Value"] if "00280030" in metadata else (1.0, 1.0)
774+
for value in spacing:
775+
_raise_if_not_finite(value, "PixelSpacing (0028,0030)")
761776
dr, dc = metadata.get("spacing", spacing)[:2]
777+
_raise_if_not_finite(dr, "spacing")
778+
_raise_if_not_finite(dc, "spacing")
762779
affine[0, 0] = cx * dr
763780
affine[0, 1] = rx * dc
764781
affine[0, 3] = sx
@@ -773,6 +790,8 @@ def _get_affine(self, metadata: dict, lps_to_ras: bool = True):
773790
# 3d
774791
if "lastImagePositionPatient" in metadata:
775792
t1n, t2n, t3n = metadata["lastImagePositionPatient"]
793+
for value in (t1n, t2n, t3n):
794+
_raise_if_not_finite(value, "lastImagePositionPatient")
776795
n = metadata[MetaKeys.SPATIAL_SHAPE][-1]
777796
if n > 1:
778797
affine[0, 2] = (t1n - sx) / (n - 1)

tests/data/test_pydicom_reader.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,40 @@ def test_partial_orientation_tags_warns(self):
3939
affine = reader._get_affine(metadata)
4040
np.testing.assert_array_equal(affine, np.eye(4))
4141

42+
def test_non_finite_pixel_spacing_raises(self):
43+
reader = PydicomReader()
44+
metadata = {
45+
"00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]},
46+
"00200032": {"Value": [0.0, 0.0, 0.0]},
47+
"00280030": {"Value": [np.nan, 1.0]},
48+
}
49+
with self.assertRaisesRegex(ValueError, "PixelSpacing"):
50+
reader._get_affine(metadata, lps_to_ras=False)
51+
52+
def test_non_finite_image_position_raises(self):
53+
reader = PydicomReader()
54+
metadata = {
55+
"00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]},
56+
"00200032": {"Value": [np.inf, 0.0, 0.0]},
57+
"00280030": {"Value": [1.0, 1.0]},
58+
}
59+
with self.assertRaisesRegex(ValueError, "ImagePositionPatient"):
60+
reader._get_affine(metadata, lps_to_ras=False)
61+
62+
def test_finite_values_return_affine(self):
63+
reader = PydicomReader()
64+
metadata = {
65+
"00200037": {"Value": [1.0, 0.0, 0.0, 0.0, 1.0, 0.0]},
66+
"00200032": {"Value": [10.0, 20.0, 30.0]},
67+
"00280030": {"Value": [0.5, 0.25]},
68+
}
69+
affine = reader._get_affine(metadata, lps_to_ras=False)
70+
self.assertEqual(affine.shape, (4, 4))
71+
self.assertTrue(np.all(np.isfinite(affine)))
72+
np.testing.assert_allclose(affine[0, 3], 10.0)
73+
np.testing.assert_allclose(affine[1, 3], 20.0)
74+
np.testing.assert_allclose(affine[2, 3], 30.0)
75+
4276

4377
if __name__ == "__main__":
4478
unittest.main()

0 commit comments

Comments
 (0)