Package: release.debian.org Severity: normal Tags: trixie X-Debbugs-Cc: [email protected] Control: affects -1 + src:pydicom User: [email protected] Usertags: pu
Fix CVE-2026-32711 Path traversal in FileSet/DICOMDIR ReferencedFileID allows file access outside the File-set root [ Tests ] autopkgtest are ok. [ Checklist ] [*] *all* changes are documented in the d/changelog [*] I reviewed all changes and I approve them [*] attach debdiff against the package in (old)stable [*] the issue is verified as fixed in unstable [ Changes ] see attachment
diff --git a/debian/changelog b/debian/changelog index c47a75b..f7cabee 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,10 @@ +pydicom (2.4.3-2+deb13u1) trixie; urgency=medium + + * Team upload. + * Fix CVE-2026-32711 vulnerable to Path Traversal + + -- Karsten Schöke <[email protected]> Thu, 17 Sep 2026 10:11:27 +0200 + pydicom (2.4.3-2) unstable; urgency=medium * Team upload. diff --git a/debian/control b/debian/control index 675f184..e3dc157 100644 --- a/debian/control +++ b/debian/control @@ -9,6 +9,7 @@ Build-Depends: debhelper-compat (= 13), dh-python, python3-all, python3-pytest, + python3-pyfakefs, python3-setuptools, python3-setuptools-scm, python3-numpy, diff --git a/debian/patches/0006-Insert-CVE-2026-32711-patch.patch b/debian/patches/0006-Insert-CVE-2026-32711-patch.patch new file mode 100644 index 0000000..a5ff341 --- /dev/null +++ b/debian/patches/0006-Insert-CVE-2026-32711-patch.patch @@ -0,0 +1,567 @@ +From: =?utf-8?q?Karsten_Sch=C3=B6ke?= <[email protected]> +Date: Wed, 16 Sep 2026 18:51:48 +0200 +Subject: Insert CVE-2026-32711 patch. + +--- + pydicom/cli/codify.py | 2 +- + pydicom/fileset.py | 152 ++++++++++++++++++++++++++++++------------ + pydicom/tests/conftest.py | 8 +++ + pydicom/tests/test_fileset.py | 126 +++++++++++++++++++++++++++++++--- + 4 files changed, 233 insertions(+), 55 deletions(-) + +diff --git a/pydicom/cli/codify.py b/pydicom/cli/codify.py +index 15115c4..2239507 100644 +--- a/pydicom/cli/codify.py ++++ b/pydicom/cli/codify.py +@@ -24,7 +24,7 @@ def add_subparser(subparsers: argparse._SubParsersAction) -> None: + ), + ) + +- # Codify existed before as a stand-alone before, re-use it here ++ # Codify existed before as a stand-alone before, reuse it here + pydicom.util.codify.set_parser_arguments( + codify_parser, default_exclude_size + ) +diff --git a/pydicom/fileset.py b/pydicom/fileset.py +index 5549d6d..f4412a4 100644 +--- a/pydicom/fileset.py ++++ b/pydicom/fileset.py +@@ -344,24 +344,47 @@ class RecordNode(Iterable["RecordNode"]): + + return len(fp.getvalue()) + +- @property +- def _file_id(self) -> Optional[Path]: ++ def file_id_path(self, root_path: Path) -> Path | None: + """Return the *Referenced File ID* as a :class:`~pathlib.Path`. + ++ Params ++ ------ ++ root_path : Path ++ The root path of the parent file set. ++ + Returns + ------- + pathlib.Path or None + The *Referenced File ID* from the directory record as a + :class:`pathlib.Path` or ``None`` if the element value is null. ++ ++ Raises ++ ------ ++ PermissionError ++ If the file ID points to a path outside the fileset root path. ++ ++ AttributeError ++ If the Referenced File ID is missing in the directory record. ++ ++ :meta private: + """ + if "ReferencedFileID" in self._record: + elem = self._record["ReferencedFileID"] ++ if elem.VM < 1: ++ return None + if elem.VM == 1: +- return Path(cast(str, self._record.ReferencedFileID)) +- if elem.VM > 1: +- return Path(*cast(List[str], self._record.ReferencedFileID)) +- +- return None ++ path = Path(cast(str, self._record.ReferencedFileID)) ++ else: ++ path = Path(*cast(list[str], self._record.ReferencedFileID)) ++ ++ if path is not None: ++ if path.anchor or not ( ++ (root_path / path).resolve().is_relative_to(root_path) ++ ): ++ raise PermissionError( ++ f"ReferencedFileID ('{path}') must be inside the DICOMDIR root path" ++ ) ++ return path + + raise AttributeError("No 'Referenced File ID' in the directory record") + +@@ -371,7 +394,7 @@ class RecordNode(Iterable["RecordNode"]): + return self.root.file_set + + def __getitem__(self, key: Union[str, "RecordNode"]) -> "RecordNode": +- """Return the current node's child using it's ++ """Return the current node's child using its + :attr:`~pydicom.fileset.RecordNode.key` + """ + if isinstance(key, RecordNode): +@@ -524,8 +547,8 @@ class RecordNode(Iterable["RecordNode"]): + for node in self: + indent = indent_char * node.depth + if node.children: +- s.append(f"{indent}{str(node)}") +- # Summarise any leaves at the next level ++ s.append(f"{indent}{node}") ++ # Summarize any leaves at the next level + for child in node.children: + if child.has_instance: + s.extend(leaf_summary(child, indent_char)) +@@ -931,9 +954,8 @@ class FileInstance: + return os.fspath(cast(Path, self._stage_path)) + + # If not staged for addition then File Set must exist on file system +- return os.fspath( +- cast(Path, self.file_set.path) / cast(Path, self.node._file_id) +- ) ++ root_path = self.file_set.root_path ++ return os.fspath(root_path / cast(Path, self.node.file_id_path(root_path))) + + @property + def SOPClassUID(self) -> UID: +@@ -966,7 +988,7 @@ class FileSet: + to the DICOMDIR file. + """ + # The nominal path to the root of the File-set +- self._path: Optional[Path] = None ++ self._root_path: Path | None = None + # The root node of the record tree used to fill out the DICOMDIR's + # *Directory Record Sequence*. + # The tree for instances currently in the File-set +@@ -1212,7 +1234,7 @@ class FileSet: + """Clear the File-set.""" + self._tree.children = [] + self._instances = [] +- self._path = None ++ self._root_path = None + self._ds = Dataset() + self._id = None + self._uid = generate_uid() +@@ -1661,7 +1683,7 @@ class FileSet: + ) + + try: +- path = Path(cast(str, ds.filename)).resolve(strict=True) ++ path = Path(ds.filename).resolve(strict=True) + except FileNotFoundError: + raise FileNotFoundError( + "Unable to load the File-set as the 'filename' attribute " +@@ -1692,7 +1714,7 @@ class FileSet: + Optional[str], + ds.get("SpecificCharacterSetOfFileSetDescriptorFile", None) + ) +- self._path = path.parent ++ self._root_path = path.parent + self._ds = ds + + # Create the record tree +@@ -1701,20 +1723,17 @@ class FileSet: + bad_instances = [] + for instance in self: + # Check that the referenced file exists +- file_id = instance.node._file_id +- if file_id is None: +- bad_instances.append(instance) +- continue +- ++ file_id = self._file_id_path(instance.node) ++ assert file_id is not None + try: + # self.path is already set at this point +- (cast(Path, self.path) / file_id).resolve(strict=True) ++ (self.root_path / file_id).resolve(strict=True) + except FileNotFoundError: + bad_instances.append(instance) + warnings.warn( + "The referenced SOP Instance for the directory record at " + f"offset {instance.node._offset} does not exist: " +- f"{cast(Path, self.path) / file_id}" ++ f"{self.root_path / file_id}" + ) + continue + +@@ -1726,6 +1745,31 @@ class FileSet: + for instance in bad_instances: + self._instances.remove(instance) + ++ def _file_id_path(self, node: RecordNode) -> Path | None: ++ """Return the *Referenced File ID* from the given node ++ as a :class:`~pathlib.Path`. ++ ++ Parameters ++ ---------- ++ node: RecordNode ++ The node where the *Referenced File ID* resides. ++ ++ Returns ++ ------- ++ pathlib.Path or None ++ The *Referenced File ID* from the directory record as a ++ :class:`pathlib.Path` or ``None`` if the element value is null. ++ ++ Raises ++ ------ ++ PermissionError ++ If the file ID points to a path outside the fileset root path. ++ ++ AttributeError ++ If the Referenced File ID is missing in the directory record. ++ """ ++ return node.file_id_path(self.root_path) ++ + def _parse_records( + self, + ds: Dataset, +@@ -1782,7 +1826,10 @@ class FileSet: + del node.parent[node] + + # The leaf node references the FileInstance +- if "ReferencedFileID" in node._record: ++ if ( ++ "ReferencedFileID" in node._record ++ and self._file_id_path(node) is not None ++ ): + node.instance = FileInstance(node) + self._instances.append(node.instance) + +@@ -1817,12 +1864,11 @@ class FileSet: + for node in missing: + # Get the path to the orphaned instance + original_value = node._record.ReferencedFileID +- file_id = node._file_id +- if file_id is None: ++ if (file_id := self._file_id_path(node)) is None: + continue + + # self.path is set for an existing File Set +- path = cast(Path, self.path) / file_id ++ path = self.root_path / file_id + if node.record_type == "PRIVATE": + instance = self.add_custom(path, node) + else: +@@ -1832,14 +1878,29 @@ class FileSet: + instance.node._record.ReferencedFileID = original_value + + @property +- def path(self) -> Optional[str]: ++ def root_path(self) -> Path: ++ """Return the absolute path to the File-set root directory as ++ :class:`pathlib.Path`. ++ ++ Raises ++ ------ ++ AttributeError ++ If the root path is not set. ++ """ ++ if self._root_path is None: ++ raise AttributeError("No root path set in the File-set") ++ ++ return self._root_path ++ ++ @property ++ def path(self) -> str | None: + """Return the absolute path to the File-set root directory as + :class:`str` (if set) or ``None`` otherwise. + """ +- if self._path is not None: +- return os.fspath(self._path) ++ if self._root_path is not None: ++ return os.fspath(self._root_path) + +- return self._path ++ return None + + def _recordify(self, ds: Dataset) -> Iterator[Dataset]: + """Yield directory records for a SOP Instance. +@@ -2105,16 +2166,17 @@ class FileSet: + ) + + if path: +- self._path = Path(path) ++ self._root_path = Path(path) + + # Don't write unless changed or new + if not self.is_staged: + return + + # Path to the DICOMDIR file +- p = cast(Path, self._path) / 'DICOMDIR' ++ root = self.root_path ++ p = root / "DICOMDIR" + +- # Re-use the existing directory structure if only moves or removals ++ # Reuse the existing directory structure if only moves or removals + # are required and `use_existing` is True + major_change = bool(self._stage['+']) + if use_existing and major_change: +@@ -2159,26 +2221,30 @@ class FileSet: + # and copy any to the stage + fout = {Path(ii.FileID) for ii in self} + fin = { +- ii.node._file_id for ii in self +- if ii.SOPInstanceUID not in self._stage['+'] ++ self._file_id_path(ii.node) ++ for ii in self ++ if ii.SOPInstanceUID not in self._stage["+"] + } + collisions = fout & fin +- for instance in [ii for ii in self if ii.node._file_id in collisions]: +- self._stage['+'][instance.SOPInstanceUID] = instance +- instance._apply_stage('+') ++ for instance in [ ++ ii for ii in self if self._file_id_path(ii.node) in collisions ++ ]: ++ self._stage["+"][instance.SOPInstanceUID] = instance ++ instance._apply_stage("+") + shutil.copyfile( +- self._path / instance.node._file_id, instance.path ++ root / cast(Path, self._file_id_path(instance.node)), ++ instance.path, + ) + + for instance in self: +- dst = self._path / instance.FileID ++ dst = root / instance.FileID + dst.parent.mkdir(parents=True, exist_ok=True) + fn: Callable + if instance.SOPInstanceUID in self._stage['+']: + src = instance.path + fn = shutil.copyfile + else: +- src = self._path / instance.node._file_id ++ src = root / cast(Path, self._file_id_path(instance.node)) + fn = shutil.move + + fn(os.fspath(src), os.fspath(dst)) +diff --git a/pydicom/tests/conftest.py b/pydicom/tests/conftest.py +index 10d1dc3..301fd00 100644 +--- a/pydicom/tests/conftest.py ++++ b/pydicom/tests/conftest.py +@@ -22,6 +22,14 @@ def allow_reading_invalid_values(): + config.settings.reading_validation_mode = value + + [email protected] ++def ignore_reading_invalid_values(): ++ value = config.settings.reading_validation_mode ++ config.settings.reading_validation_mode = config.IGNORE ++ yield ++ config.settings.reading_validation_mode = value ++ ++ + @pytest.fixture + def enforce_writing_invalid_values(): + value = config.settings.writing_validation_mode +diff --git a/pydicom/tests/test_fileset.py b/pydicom/tests/test_fileset.py +index cff26da..bdb7cee 100644 +--- a/pydicom/tests/test_fileset.py ++++ b/pydicom/tests/test_fileset.py +@@ -1,5 +1,6 @@ + + import os ++import platform + import sys + from pathlib import Path + import shutil +@@ -10,7 +11,7 @@ import pytest + from pydicom import config, dcmread + from pydicom.data import get_testdata_file + from pydicom.dataset import Dataset, FileMetaDataset +-from pydicom.filebase import DicomBytesIO ++from pydicom.filebase import DicomBytesIO, DicomFileLike + from pydicom.fileset import ( + FileSet, FileInstance, RecordNode, is_conformant_file_id, + generate_filename, _define_patient, _define_study, _define_series, +@@ -81,6 +82,56 @@ def tdir(): + return TemporaryDirectory() + + ++FILESET_ROOT = "/path/to/fileset/" ++ABS_FILE_PATH = "/secret.txt" ++SYMLINK_TO_ABS_FILE = "Pat1/St1/Im2" ++SYMLINK_TO_ABS_DIR = "Pat1/St2" ++DOT_DOT_FILE = "../goback.txt" ++ABS_FILE_CONTENTS = "Top Secret file contents" ++COPY_PATH = "/path/to/copied/" ++ ++ [email protected]( ++ params=[ ++ ABS_FILE_PATH, ++ DOT_DOT_FILE, ++ SYMLINK_TO_ABS_FILE, ++ SYMLINK_TO_ABS_DIR + ABS_FILE_PATH, ++ ] ++) ++def fileset_fs(request, fs, ignore_reading_invalid_values): ++ """Create an in-memory file system with pyfakefs and test DICOMDIRs""" ++ # Simplified version of submitted report from JeongAhn Jang, in pyfakefs ++ orig_dicomdir_root = Path(TEST_FILE).parent ++ dicomdir_root = Path(FILESET_ROOT) ++ fs.add_real_file( ++ orig_dicomdir_root / "77654033/CR1/6154", ++ target_path=dicomdir_root / "Pat1/St1/Im1", ++ ) ++ fs.create_file(ABS_FILE_PATH, contents=ABS_FILE_CONTENTS) ++ fs.create_dir(COPY_PATH) ++ fs.create_symlink(dicomdir_root / SYMLINK_TO_ABS_FILE, ABS_FILE_PATH) ++ fs.create_symlink(dicomdir_root / SYMLINK_TO_ABS_DIR, "/") ++ # MAKE DICOMDIR for this simplified file-set ++ fset = FileSet() ++ fset.add(dicomdir_root / "Pat1/St1/Im1") ++ fset.write(dicomdir_root) ++ ++ # Create bad DICOMDIR2 file from the simplified one ++ # Modify first referenced file ++ fset = FileSet(dicomdir_root / "DICOMDIR") ++ record = next( ++ rec for rec in fset._ds.DirectoryRecordSequence if "ReferencedFileID" in rec ++ ) ++ record.ReferencedFileID = request.param ++ ++ # Write modified DICOMDIR file ++ with open(dicomdir_root / "DICOMDIR2", "wb") as fp: ++ fset._write_dicomdir(DicomFileLike(fp)) ++ ++ yield fs ++ ++ + @pytest.fixture + def custom_leaf(): + """Return the leaf node from a custom 4-level record hierarchy""" +@@ -116,7 +167,7 @@ def custom_leaf(): + + + @pytest.fixture +-def private(dicomdir): ++def private(dicomdir, request, ignore_reading_invalid_values): + """Return a DICOMDIR dataset with PRIVATE records.""" + def write_record(ds): + """Return `ds` as explicit little encoded bytes.""" +@@ -143,9 +194,17 @@ def private(dicomdir): + middle = private_record() + bottom = private_record() + bottom.ReferencedSOPClassUIDInFile = "1.2.3.4" +- bottom.ReferencedFileID = [ +- "TINY_ALPHA", "PT000000", "ST000000", "SE000000", "IM000000" +- ] ++ if hasattr(request, "param"): ++ file_ids = request.param ++ else: ++ file_ids = [ ++ "TINY_ALPHA", ++ "PT000000", ++ "ST000000", ++ "SE000000", ++ "IM000000", ++ ] ++ bottom.ReferencedFileID = file_ids + bottom.ReferencedSOPInstanceUIDInFile = ( + "1.2.276.0.7230010.3.1.4.0.31906.1359940846.78187" + ) +@@ -647,6 +706,15 @@ class TestRecordNode: + with pytest.raises(AttributeError, match=msg): + instance.node.key + ++ @pytest.mark.parametrize("private", [["/", "etc", "passwd"]], indirect=True) ++ def test_id_outside_root(self, private): ++ """File ID points to a path outside the root directory.""" ++ with pytest.raises( ++ PermissionError, ++ match=r"ReferencedFileID .* must be inside the DICOMDIR root path", ++ ): ++ FileSet(private) ++ + def test_bad_record(self, private): + """Test a bad directory record raises an exception when loading.""" + del private.DirectoryRecordSequence[0].PatientID +@@ -709,7 +777,33 @@ class TestRecordNode: + item.ReferencedFileID = "01" + ds.save_as(p / "DICOMDIR") + fs = FileSet(ds) +- assert fs._instances[0].node._file_id == Path("01") ++ assert fs._instances[0].node.file_id_path(fs.root_path) == Path("01") ++ ++ def test_absolute_file_id(self, ct, tdir, ignore_reading_invalid_values): ++ """Test a singleton File ID.""" ++ fs = FileSet() ++ p = Path(tdir.name) ++ ct.save_as(p / "01") ++ fs.add(p / "01") ++ fs.write(p) ++ ds = dcmread(p / "DICOMDIR") ++ item = ds.DirectoryRecordSequence[-1] ++ item.ReferencedFileID = "/01" ++ ds.save_as(p / "DICOMDIR") ++ with pytest.raises( ++ PermissionError, ++ match=r"ReferencedFileID .* must be inside the DICOMDIR root path", ++ ): ++ FileSet(ds) ++ ++ def test_root_path_missing(self, ct): ++ """Test RecordNode._file_id if no Referenced File ID.""" ++ fs = FileSet() ++ instance = fs.add(ct) ++ # del instance.node._record.ReferencedFileID ++ msg = r"No root path set in the File-set" ++ with pytest.raises(AttributeError, match=msg): ++ fs.root_path + + def test_file_id_missing(self, ct): + """Test RecordNode._file_id if no Referenced File ID.""" +@@ -718,7 +812,7 @@ class TestRecordNode: + del instance.node._record.ReferencedFileID + msg = r"No 'Referenced File ID' in the directory record" + with pytest.raises(AttributeError, match=msg): +- instance.node._file_id ++ instance.node.file_id_path(Path("/dicom_data")) + + + @pytest.mark.filterwarnings("ignore:The 'DicomDir'") +@@ -1651,7 +1745,7 @@ class TestFileSet: + assert "ISO 1" == fs.descriptor_character_set + assert [] != fs._instances + assert fs._id is not None +- assert fs._path is not None ++ assert fs.root_path is not None + uid = fs._uid + assert fs._uid is not None + assert fs._ds is not None +@@ -1662,7 +1756,7 @@ class TestFileSet: + fs.clear() + assert [] == fs._instances + assert fs._id is None +- assert fs._path is None ++ assert fs._root_path is None + assert uid != fs._uid + assert fs._uid.is_valid + assert fs._ds == Dataset() +@@ -2323,14 +2417,14 @@ class TestFileSet_Modify: + tdir, ds = dicomdir_copy + assert 52 == len(ds.DirectoryRecordSequence) + fs = FileSet(ds) +- orig_paths = [p for p in fs._path.glob('**/*') if p.is_file()] ++ orig_paths = [p for p in fs.root_path.glob("**/*") if p.is_file()] + instance = fs._instances[0] + assert Path(instance.path) in orig_paths + fs.remove(instance) + orig_file_ids = [ii.ReferencedFileID for ii in fs] + fs.write(use_existing=True) + assert 50 == len(fs._ds.DirectoryRecordSequence) +- paths = [p for p in fs._path.glob('**/*') if p.is_file()] ++ paths = [p for p in fs.root_path.glob("**/*") if p.is_file()] + assert orig_file_ids == [ii.ReferencedFileID for ii in fs] + assert Path(instance.path) not in paths + assert sorted(orig_paths)[1:] == sorted(paths) +@@ -2480,6 +2574,16 @@ class TestFileSet_Copy: + def teardown_method(self): + FileSet.__len__ = self.orig + ++ @pytest.mark.skipif(platform.python_implementation() == "PyPy", ++ reason="pyfakefs does not work with generate_uid() in PyPy") ++ def test_constrained_to_fileset_root(self, fileset_fs): ++ """Ensure files cannot be copied outside the FileSet root""" ++ with pytest.raises( ++ PermissionError, ++ match=r"ReferencedFileID .* must be inside the DICOMDIR root path", ++ ): ++ FileSet(Path(FILESET_ROOT) / "DICOMDIR2") ++ + def test_copy(self, dicomdir, tdir): + """Test FileSet.copy()""" + orig_root = Path(dicomdir.filename).parent diff --git a/debian/patches/series b/debian/patches/series index b82a55e..d48f0cc 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -3,3 +3,4 @@ ignore_tests_downloading_data.patch ignore_tests_with_wrong_gdcm_usage.patch sphinx_no_git.patch skip-i386-incapable-tests.patch +0006-Insert-CVE-2026-32711-patch.patch
