File CVE-2026-32711.patch of Package python-pydicom
From 6414f01a053dff925578799f5a7208d2ae585e82 Mon Sep 17 00:00:00 2001
From: Darcy Mason <darcymason@gmail.com>
Date: Thu, 19 Mar 2026 14:33:11 -0400
Subject: [PATCH] Merge from fork - security fix in `FileSet`s * Raise
PermissionError for bad file id * Add fixture to Ignore unneded warnings in
tests ---------
Co-authored-by: mrbean-bremen
---
doc/release_notes/index.rst | 1 +
doc/release_notes/v3.0.2.rst | 8 ++
pyproject.toml | 27 ++++++-
src/pydicom/fileset.py | 141 ++++++++++++++++++++++++++---------
tests/conftest.py | 8 ++
tests/test_fileset.py | 127 +++++++++++++++++++++++++++----
6 files changed, 260 insertions(+), 52 deletions(-)
create mode 100644 doc/release_notes/v3.0.2.rst
Index: pydicom-2.3.1/pydicom/fileset.py
===================================================================
--- pydicom-2.3.1.orig/pydicom/fileset.py
+++ pydicom-2.3.1/pydicom/fileset.py
@@ -344,8 +344,7 @@ class RecordNode(Iterable["RecordNode"])
return len(fp.getvalue())
- @property
- def _file_id(self) -> Optional[Path]:
+ def file_id_path(self, root_path: Path) -> Optional[Path]:
"""Return the *Referenced File ID* as a :class:`~pathlib.Path`.
Returns
@@ -356,12 +355,22 @@ class RecordNode(Iterable["RecordNode"])
"""
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))
+ path = Path(cast(str, self._record.ReferencedFileID))
+ else:
+ path = Path(*cast(List[str], self._record.ReferencedFileID))
- return None
+ if path is not None:
+ pp = (root_path / path).resolve()
+ if path.anchor or not (
+ pp == root_path or root_path in pp.parents
+ ):
+ 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 +380,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):
@@ -931,9 +940,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 +974,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: Optional[Path] = 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 +1220,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 +1669,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 +1700,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 +1709,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 +1731,31 @@ class FileSet:
for instance in bad_instances:
self._instances.remove(instance)
+ def _file_id_path(self, node: RecordNode) -> Optional[Path]:
+ """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 +1812,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 +1850,12 @@ class FileSet:
for node in missing:
# Get the path to the orphaned instance
original_value = node._record.ReferencedFileID
- file_id = node._file_id
+ file_id = self._file_id_path(node)
if file_id 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:
@@ -1830,16 +1863,31 @@ class FileSet:
# Because the record is new the Referenced File ID isn't set
instance.node._record.ReferencedFileID = original_value
+
+ @property
+ 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) -> Optional[str]:
"""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 self._root_path
def _recordify(self, ds: Dataset) -> Iterator[Dataset]:
"""Yield directory records for a SOP Instance.
@@ -2105,14 +2153,15 @@ 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
# are required and `use_existing` is True
@@ -2159,26 +2208,27 @@ 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
+ 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]:
+ 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 = self._root_path / 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))
Index: pydicom-2.3.1/pydicom/tests/test_fileset.py
===================================================================
--- pydicom-2.3.1.orig/pydicom/tests/test_fileset.py
+++ pydicom-2.3.1/pydicom/tests/test_fileset.py
@@ -1651,7 +1651,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 +1662,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()
@@ -2321,14 +2321,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)
