Attachment 17922 Details for Bug 15390 – patch for 4.18

[patch] patch for 4.18

v4-18-fix-safe-tarfile.patch (text/plain), 9.61 KB, created by Andreas Schneider on 2023-06-15 07:47:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andreas Schneider

Created: 2023-06-15 07:47:18 UTC

Size: 9.61 KB

patch

obsolete

>From 10683c0c121e38d8ef60d494b72579b016372592 Mon Sep 17 00:00:00 2001
>From: Andreas Schneider <asn@samba.org>
>Date: Tue, 6 Jun 2023 16:06:57 +0200
>Subject: [PATCH 1/4] python:tests: Adopt safe_tarfile for extraction_filter
> raises
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15390
>
>Signed-off-by: Andreas Schneider <asn@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit ebaa00816259cbae5c45ebf0ba5fb260b09e4695)
>---
> python/samba/tests/safe_tarfile.py | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
>diff --git a/python/samba/tests/safe_tarfile.py b/python/samba/tests/safe_tarfile.py
>index 40aa9e17d4a..cf9b725afcb 100644
>--- a/python/samba/tests/safe_tarfile.py
>+++ b/python/samba/tests/safe_tarfile.py
>@@ -43,9 +43,16 @@ class SafeTarFileTestCase(TestCaseInTempDir):
> 
>         stf = safe_tarfile.open(tarname)
> 
>-        self.assertRaises(tarfile.ExtractError,
>-                          stf.extractall,
>-                          tarname)
>+        # We we have data_filter, we have a patched python to address
>+        # CVE-2007-4559.
>+        if hasattr(tarfile, "data_filter"):
>+            self.assertRaises(tarfile.OutsideDestinationError,
>+                              stf.extractall,
>+                              tarname)
>+        else:
>+            self.assertRaises(tarfile.ExtractError,
>+                              stf.extractall,
>+                              tarname)
>         self.rm_files('x', 'tar.tar')
> 
>     def test_slash(self):
>@@ -60,8 +67,16 @@ class SafeTarFileTestCase(TestCaseInTempDir):
>         tf.close()
> 
>         stf = safe_tarfile.open(tarname)
>-        self.assertRaises(tarfile.ExtractError,
>-                          stf.extractall,
>-                          tarname)
>+
>+        # We we have data_filter, we have a patched python to address
>+        # CVE-2007-4559.
>+        if hasattr(tarfile, "data_filter"):
>+            self.assertRaises(NotADirectoryError,
>+                              stf.extractall,
>+                              tarname)
>+        else:
>+            self.assertRaises(tarfile.ExtractError,
>+                              stf.extractall,
>+                              tarname)
> 
>         self.rm_files('x', 'tar.tar')
>-- 
>2.41.0
>
>
>From 8dd8ea221b040173e55dac57cdb58a5e792bbd16 Mon Sep 17 00:00:00 2001
>From: Andreas Schneider <asn@samba.org>
>Date: Tue, 6 Jun 2023 15:29:06 +0200
>Subject: [PATCH 2/4] python:safe_tarfile: Set extraction_filter for pythons
> providing it
>
>It should be available for Python >= 3.11.4 but also has been
>backported.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15390
>
>Signed-off-by: Andreas Schneider <asn@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit 8c90c66a9a409d807dad56822540509c9813425b)
>---
> python/samba/safe_tarfile.py | 36 ++++++++++++++++++++++--------------
> 1 file changed, 22 insertions(+), 14 deletions(-)
>
>diff --git a/python/samba/safe_tarfile.py b/python/samba/safe_tarfile.py
>index cc19770d73f..164bb0b31fe 100644
>--- a/python/samba/safe_tarfile.py
>+++ b/python/samba/safe_tarfile.py
>@@ -15,6 +15,7 @@
> # along with this program.  If not, see <http://www.gnu.org/licenses/>.
> 
> 
>+import tarfile
> from tarfile import ExtractError, TarInfo, TarFile as UnsafeTarFile
> 
> 
>@@ -24,20 +25,27 @@ class TarFile(UnsafeTarFile):
>     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)
>+    try:
>+        # New in version 3.11.4 (also has been backported)
>+        # https://docs.python.org/3/library/tarfile.html#tarfile.TarFile.extraction_filter
>+        # https://peps.python.org/pep-0706/
>+        extraction_filter = staticmethod(tarfile.data_filter)
>+    except AttributeError:
>+        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
>-- 
>2.41.0
>
>
>From cac6b5ad93e1b97be09faa5cb69f1f5b9072d30e Mon Sep 17 00:00:00 2001
>From: Andreas Schneider <asn@samba.org>
>Date: Tue, 6 Jun 2023 15:30:20 +0200
>Subject: [PATCH 3/4] python:safe_tarfile: Implement safer extractall()
>
>This also checks for symlinks and hardlinks.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15390
>
>Signed-off-by: Andreas Schneider <asn@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit 431f7698e48387413aac586c7a939a1682464681)
>---
> python/samba/safe_tarfile.py | 53 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
>diff --git a/python/samba/safe_tarfile.py b/python/samba/safe_tarfile.py
>index 164bb0b31fe..8535f54f5b3 100644
>--- a/python/samba/safe_tarfile.py
>+++ b/python/samba/safe_tarfile.py
>@@ -15,7 +15,9 @@
> # along with this program.  If not, see <http://www.gnu.org/licenses/>.
> 
> 
>+import os
> import tarfile
>+from pathlib import Path
> from tarfile import ExtractError, TarInfo, TarFile as UnsafeTarFile
> 
> 
>@@ -47,5 +49,56 @@ class TarFile(UnsafeTarFile):
>             super().extract(member, path, set_attrs=set_attrs,
>                             numeric_owner=numeric_owner)
> 
>+        def extractall(self, path, members=None, *, numeric_owner=False):
>+            self._safetarfile_check()
>+            super().extractall(path, members,
>+                               numeric_owner=numeric_owner)
>+
>+        def _safetarfile_check(self):
>+            for tarinfo in self.__iter__():
>+                if self._is_traversal_attempt(tarinfo=tarinfo):
>+                    raise ExtractError(
>+                        "Attempted directory traversal for "
>+                        f"member: {tarinfo.name}")
>+                if self._is_unsafe_symlink(tarinfo=tarinfo):
>+                    raise ExtractError(
>+                        "Attempted directory traversal via symlink for "
>+                        f"member: {tarinfo.linkname}")
>+                if self._is_unsafe_link(tarinfo=tarinfo):
>+                    raise ExtractError(
>+                        "Attempted directory traversal via link for "
>+                        f"member: {tarinfo.linkname}")
>+
>+        def _resolve_path(self, path):
>+            return os.path.realpath(os.path.abspath(path))
>+
>+        def _is_path_in_dir(self, path, basedir):
>+            return self._resolve_path(os.path.join(basedir,
>+                                      path)).startswith(basedir)
>+
>+        def _is_traversal_attempt(self, tarinfo):
>+            if (tarinfo.name.startswith(os.sep)
>+               or ".." + os.sep in tarinfo.name):
>+                return True
>+            return False
>+
>+        def _is_unsafe_symlink(self, tarinfo):
>+            if tarinfo.issym():
>+                symlink_file = Path(
>+                    os.path.normpath(os.path.join(os.getcwd(),
>+                                     tarinfo.linkname)))
>+                if not self._is_path_in_dir(symlink_file, os.getcwd()):
>+                    return True
>+            return False
>+
>+        def _is_unsafe_link(self, tarinfo):
>+            if tarinfo.islnk():
>+                link_file = Path(
>+                    os.path.normpath(os.path.join(os.getcwd(),
>+                                                  tarinfo.linkname)))
>+                if not self._is_path_in_dir(link_file, os.getcwd()):
>+                    return True
>+            return False
>+
> 
> open = TarFile.open
>-- 
>2.41.0
>
>
>From fb45966ea33df8905fc9555ee66ebcb1c3ae278e Mon Sep 17 00:00:00 2001
>From: Andreas Schneider <asn@samba.org>
>Date: Tue, 6 Jun 2023 15:38:12 +0200
>Subject: [PATCH 4/4] python:safe_tarfile: Improve safe extract()
>
>This also checks for symlinks and hardlinks.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15390
>
>Signed-off-by: Andreas Schneider <asn@samba.org>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(cherry picked from commit 1f74f9f366d7f107a89220a4a5951bc4daf18025)
>---
> python/samba/safe_tarfile.py | 12 +-----------
> 1 file changed, 1 insertion(+), 11 deletions(-)
>
>diff --git a/python/samba/safe_tarfile.py b/python/samba/safe_tarfile.py
>index 8535f54f5b3..7a2b0382a79 100644
>--- a/python/samba/safe_tarfile.py
>+++ b/python/samba/safe_tarfile.py
>@@ -35,17 +35,7 @@ class TarFile(UnsafeTarFile):
>     except AttributeError:
>         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 '/'")
>-
>+            self._safetarfile_check()
>             super().extract(member, path, set_attrs=set_attrs,
>                             numeric_owner=numeric_owner)
> 
>-- 
>2.41.0
>

Flags:

dbagnall: review+

Actions: View

Attachments on bug 15390: 17922 | 17992