Bug 2218237

+++ This bug was initially created as a clone of Bug #2207692 +++

Hello,
In RHEL 9.3 and 8.9, we're planning to fix the long-standing CVE-2007-4559: Python's `tarfile` module makes it too easy to extract tarballs in an unsafe way.
Unfortunately, for the CVE to be considered fixed, this needs a behavior change. (If you don't think this is the case, let's bring it up with the security team.)
Upstream, Python will emit deprecation warnings for 2 releases, but in RHEL we change the behavior now, emit warnings, and provide ways for customers to restore earlier behavior.
To avoid the warning, software shipped by Red Hat will need a change.

For more details see upstream PEP 706: https://peps.python.org/pep-0706
and the Red Hat knowledge base draft: https://access.redhat.com/articles/7004769

---

In ntacls.py and in netcmd/domain_backup.py in /usr/lib*/python*/site-packages/samba/, python3-samba calls `tf.extractall(targetdir)` and `f.extractall(path=tempdir)`.
The calls will emit a warning by default, and tar features deemed too dangerous for general use will even be blocked by default.

I am not sure what archive is being extracted. If this is a trusted backup, please add the following before the extract call:

    # The archive is fully trusted, override warnings and "safe" defaults
    # see https://docs.python.org/3.12/library/tarfile.html#supporting-older-python-versions
    tf.extraction_filter = (lambda member, path: member)

This will restore previous behaviour, and it's a no-op on earlier Python versions.

In case this tarball is not always fully trusted, please let me know privately.

---

Let me know if you have any questions!

--- Additional comment from Andreas Schneider on 2023-06-03 15:23:27 CEST ---

Samba has a python/samba/safe_tarfile.py [1] which is used by netcmd/domain_backup.py and others. However it doesn't overwrite extractall() only extract(). But looking at the test at [2] extratall seems to be covered as well.

So if we pass filter='data'


I think if we just pass filter='data' always to tarfile extract() we are fine? Or do we need to also add extractall() as it will print the warning otherwise, then we need to also implement extractall() in python/samba/safe_tarfile.py



 python/samba/safe_tarfile.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/python/samba/safe_tarfile.py b/python/samba/safe_tarfile.py
index cc19770d73f..489ccb1a65b 100644
--- a/python/samba/safe_tarfile.py
+++ b/python/samba/safe_tarfile.py
@@ -24,7 +24,8 @@ class TarFile(UnsafeTarFile):
     using '../../'.
     """
 
-    def extract(self, member, path="", set_attrs=True, *, numeric_owner=False):
+    def extract(self, member, path="", set_attrs=True, *, numeric_owner=False,
+                filter="data"):
         if isinstance(member, TarInfo):
             name = member.name
         else:
@@ -37,7 +38,7 @@ class TarFile(UnsafeTarFile):
             raise ExtractError(f"path '{name}' should not start with '/'")
 
         super().extract(member, path, set_attrs=set_attrs,
-                        numeric_owner=numeric_owner)
+                        numeric_owner=numeric_owner, filter=filter)
 
 
 open = TarFile.open


[1] https://gitlab.com/samba-team/samba/-/blob/master/python/samba/safe_tarfile.py
[2] https://gitlab.com/samba-team/samba/-/blob/master/python/samba/tests/safe_tarfile.py

--- Additional comment from Petr Viktorin on 2023-06-06 10:07:37 CEST ---

Note that SafeTarFile will not prevent symlink attacks, e.g. one tar member setting up `./etc -> /etc`, and the next one extracting to `./etc/passwd`.

--- Additional comment from Petr Viktorin on 2023-06-06 10:24:57 CEST ---

For passing the filter, you'd also need to provide `extractall`. Beware that `extractall` does *not* call `extract`, they're speparate "frontends" for the internals.
Also, the filter argument isn't available on older/unpatched versions of Python, so you probably can't add it unconditionally.

But since you have a subclass, things are easier: you can just add an attribute to it:

    import tarfile

    class TarFile(UnsafeTarFile):
        try:
            extraction_filter = staticmethod(tarfile.data_filter)
        except AttributeError:
            ... # override extract/extractall for earlier/unpatched versions

See the docs: https://docs.python.org/3/library/tarfile.html#tarfile.TarFile.extraction_filter
(Please let me know if anything is unclear/missing, Samba would be the first project I know doing it this way.)


Note that the errors raised are subclasses of TarError, but *not* ExtractError like your existing safe TarFile.

--- Additional comment from Andreas Schneider on 2023-06-07 11:05:25 CEST ---

Upstream Merge Request: https://gitlab.com/samba-team/samba/-/merge_requests/3114