CVE-2007-4559 python: ensure sanity in our tarfiles

Python's tarfile module is not very careful about paths that step out
of the target directory. We can be a bit better at little cost.

This was reported in 2007[1], and has recently been publicised [2, for
example].

We were informed of this bug in December 2021 by Luis Alberto López
Alvar, but decided then that there were no circumstances under which
this was a security concern. That is, if you can alter the backup
files, you can already do worse things. But there is a case to guard
against an administrator being tricked into trying to restore a file
that isn't based on a real backup.

[1] https://nvd.nist.gov/vuln/detail/CVE-2007-4559
[2] https://www.theregister.com/2022/09/22/python_vulnerability_tarfile/

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15185

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Tue Oct  4 03:48:43 UTC 2022 on sn-devel-184

python/samba/netcmd/domain_backup.py

@@ -18,7 +18,6 @@
 import datetime
 import os
 import sys
-import tarfile
 import logging
 import shutil
 import tempfile
@@ -56,6 +55,7 @@ from samba import sites
 from samba.dsdb import _dsdb_load_udv_v2
 from samba.ndr import ndr_pack
 from samba.credentials import SMB_SIGNING_REQUIRED
+from samba import safe_tarfile as tarfile
 
 
 # work out a SID (based on a free RID) to use when the domain gets restored.

python/samba/ntacls.py

@@ -19,7 +19,6 @@
 
 
 import os
-import tarfile
 import tempfile
 import shutil
 
@@ -34,6 +33,8 @@ from samba.samba3 import libsmb_samba_internal as libsmb
 from samba.logger import get_samba_logger
 from samba import NTSTATUSError
 from samba.auth_util import system_session_unix
+from samba import safe_tarfile as tarfile
+
 
 # don't include volumes
 SMB_FILE_ATTRIBUTE_FLAGS = libsmb.FILE_ATTRIBUTE_SYSTEM | \

python/samba/safe_tarfile.py

@@ -0,0 +1,43 @@
+# Unix SMB/CIFS implementation.
+# Copyright (C) Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+from tarfile import ExtractError, TarInfo, TarFile as UnsafeTarFile
+
+
+class TarFile(UnsafeTarFile):
+    """This TarFile implementation is trying to ameliorate CVE-2007-4559,
+    where tarfile.TarFiles can step outside of the target directory
+    using '../../'.
+    """
+
+    def extract(self, member, path="", set_attrs=True, *, numeric_owner=False):
+        if isinstance(member, TarInfo):
+            name = member.name
+        else:
+            name = member
+
+        if '../' in name:
+            raise ExtractError(f"'../' is not allowed in path '{name}'")
+
+        if name.startswith('/'):
+            raise ExtractError(f"path '{name}' should not start with '/'")
+
+        super().extract(member, path, set_attrs=set_attrs,
+                        numeric_owner=numeric_owner)
+
+
+open = TarFile.open

python/samba/tests/domain_backup.py

@@ -15,7 +15,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 from samba import provision, param
-import tarfile
 import os
 import shutil
 from samba.tests import (env_loadparm, create_test_ou, BlackboxProcessError,
@@ -28,6 +27,7 @@ from samba.netcmd.fsmo import get_fsmo_roleowner
 import re
 from samba import sites
 from samba.dsdb import _dsdb_load_udv_v2
+from samba import safe_tarfile as tarfile
 
 
 def get_prim_dom(secrets_path, lp):

python/samba/tests/domain_backup_offline.py

@@ -15,7 +15,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-import tarfile
 import os
 import shutil
 import tempfile
@@ -25,6 +24,8 @@ from samba.param import LoadParm
 from samba.join import join_DC
 from samba.credentials import Credentials
 from samba.logger import get_samba_logger
+from samba import safe_tarfile as tarfile
+
 
 # The backup tests require that a completely clean LoadParm object gets used
 # for the restore. Otherwise the same global LP gets re-used, and the LP

python/samba/tests/safe_tarfile.py

@@ -0,0 +1,67 @@
+# Unix SMB/CIFS implementation.
+# Copyright (C) Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+import tarfile
+from samba import safe_tarfile
+import tempfile
+
+import os
+from samba.tests import TestCaseInTempDir
+
+
+def filterer(prefix):
+    def f(info):
+        info.name = prefix + info.name
+        return info
+    return f
+
+
+class SafeTarFileTestCase(TestCaseInTempDir):
+
+    def test_dots(self):
+        filename = os.path.join(self.tempdir, 'x')
+        tarname = os.path.join(self.tempdir, 'tar.tar')
+        f = open(filename, 'w')
+        f.write('x')
+        f.close()
+
+        tf = tarfile.open(tarname, 'w')
+        tf.add(filename, filter=filterer('../../'))
+        tf.close()
+
+        stf = safe_tarfile.open(tarname)
+
+        self.assertRaises(tarfile.ExtractError,
+                          stf.extractall,
+                          tarname)
+        self.rm_files('x', 'tar.tar')
+
+    def test_slash(self):
+        filename = os.path.join(self.tempdir, 'x')
+        tarname = os.path.join(self.tempdir, 'tar.tar')
+        f = open(filename, 'w')
+        f.write('x')
+        f.close()
+
+        tf = tarfile.open(tarname, 'w')
+        tf.add(filename, filter=filterer('/'))
+        tf.close()
+
+        stf = safe_tarfile.open(tarname)
+        self.assertRaises(tarfile.ExtractError,
+                          stf.extractall,
+                          tarname)
+
+        self.rm_files('x', 'tar.tar')

selftest/tests.py

@@ -105,6 +105,7 @@ planpythontestsuite('ad_dc_fips:local',
                     environ={'GNUTLS_FORCE_FIPS_MODE': '1',
                              'OPENSSL_FORCE_FIPS_MODE': '1'})
 
+planpythontestsuite("none", "samba.tests.safe_tarfile")
 
 def cmdline(script, *args):
     """