]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-146121: Clarify security model of pkgutil.getdata; revert checks (GH-148197)
authorPetr Viktorin <encukou@gmail.com>
Tue, 7 Apr 2026 09:39:50 +0000 (11:39 +0200)
committerGitHub <noreply@github.com>
Tue, 7 Apr 2026 09:39:50 +0000 (11:39 +0200)
This reverts commit bcdf231946b1da8bdfbab4c05539bb0cc964a1c7,
and clarifies get_data's security model.

Co-authored-by: Stan Ulbrych <stan@python.org>
Doc/library/pkgutil.rst
Lib/pkgutil.py
Lib/test/test_pkgutil.py
Misc/NEWS.d/next/Security/2026-03-16-18-07-00.gh-issue-146121.vRbdro.rst [deleted file]

index 47d24b6f7d06bbe29c45e4a5aa916f8093d854d0..aa7dd71c1329dfd7466ec0da38e7f6d6d3bd4024 100644 (file)
@@ -151,24 +151,48 @@ support.
    :meth:`get_data <importlib.abc.ResourceLoader.get_data>` API.  The
    *package* argument should be the name of a package, in standard module format
    (``foo.bar``).  The *resource* argument should be in the form of a relative
-   filename, using ``/`` as the path separator.  The parent directory name
-   ``..`` is not allowed, and nor is a rooted name (starting with a ``/``).
+   filename, using ``/`` as the path separator.
 
    The function returns a binary string that is the contents of the specified
    resource.
 
+   This function uses the :term:`loader` method
+   :func:`~importlib.abc.FileLoader.get_data`
+   to support modules installed in the filesystem, but also in zip files,
+   databases, or elsewhere.
+
    For packages located in the filesystem, which have already been imported,
    this is the rough equivalent of::
 
       d = os.path.dirname(sys.modules[package].__file__)
       data = open(os.path.join(d, resource), 'rb').read()
 
+   Like the :func:`open` function, :func:`!get_data` can follow parent
+   directories (``../``) and absolute paths (starting with ``/`` or ``C:/``,
+   for example).
+   It can open compilation/installation artifacts like ``.py`` and ``.pyc``
+   files or files with :func:`reserved filenames <os.path.isreserved>`.
+   To be compatible with non-filesystem loaders, avoid using these features.
+
+   .. warning::
+
+      This function is intended for trusted input.
+      It does not verify that *resource* "belongs" to *package*.
+
+   If you use a user-provided *resource* path, consider verifying it.
+   For example, require an alphanumeric filename with a known extension, or
+   install and check a list of known resources.
+
    If the package cannot be located or loaded, or it uses a :term:`loader`
    which does not support :meth:`get_data <importlib.abc.ResourceLoader.get_data>`,
    then ``None`` is returned.  In particular, the :term:`loader` for
    :term:`namespace packages <namespace package>` does not support
    :meth:`get_data <importlib.abc.ResourceLoader.get_data>`.
 
+   .. seealso::
+
+      The :mod:`importlib.resources` module provides structured access to
+      module resources.
 
 .. function:: resolve_name(name)
 
index c3109a3a4cd414d5a7d03a27a19a7d1161152d0a..8772a66791a3c9aced4fa96e23ea79c1eda17f5d 100644 (file)
@@ -393,9 +393,6 @@ def get_data(package, resource):
     # signature - an os.path format "filename" starting with the dirname of
     # the package's __file__
     parts = resource.split('/')
-    if os.path.isabs(resource) or '..' in parts:
-        raise ValueError("resource must be a relative path with no "
-                         "parent directory components")
     parts.insert(0, os.path.dirname(mod.__file__))
     resource_name = os.path.join(*parts)
     return loader.get_data(resource_name)
index 948afb8c18cf67e671eea16d43bac77e825b881d..d4faaaeca0045726ce2c49e61497a6a7640a8115 100644 (file)
@@ -61,25 +61,6 @@ class PkgutilTests(unittest.TestCase):
 
         del sys.modules[pkg]
 
-    def test_getdata_path_traversal(self):
-        pkg = 'test_getdata_traversal'
-
-        # Make a package with some resources
-        package_dir = os.path.join(self.dirname, pkg)
-        os.mkdir(package_dir)
-        # Empty init.py
-        f = open(os.path.join(package_dir, '__init__.py'), "wb")
-        f.close()
-
-        with self.assertRaises(ValueError):
-            pkgutil.get_data(pkg, '../../../etc/passwd')
-        with self.assertRaises(ValueError):
-            pkgutil.get_data(pkg, 'sub/../../../etc/passwd')
-        with self.assertRaises(ValueError):
-            pkgutil.get_data(pkg, os.path.abspath('/etc/passwd'))
-
-        del sys.modules[pkg]
-
     def test_getdata_zipfile(self):
         zip = 'test_getdata_zipfile.zip'
         pkg = 'test_getdata_zipfile'
diff --git a/Misc/NEWS.d/next/Security/2026-03-16-18-07-00.gh-issue-146121.vRbdro.rst b/Misc/NEWS.d/next/Security/2026-03-16-18-07-00.gh-issue-146121.vRbdro.rst
deleted file mode 100644 (file)
index c0ee07d..0000000
+++ /dev/null
@@ -1,3 +0,0 @@
-:func:`pkgutil.get_data` now raises rejects *resource* arguments containing the
-parent directory components or that is an absolute path.
-This addresses :cve:`2026-3479`.