(security) Fix tar slip in micropackaging (#3559)

* Update safe_extract fn Signed-off-by: Ankita Katiyar <[email protected]> * Update tests Signed-off-by: Ankita Katiyar <[email protected]> * Try fix test Signed-off-by: Ankita Katiyar <[email protected]> * Add comment about bandit Signed-off-by: Ankita Katiyar <[email protected]> * Update release notes Signed-off-by: Ankita Katiyar <[email protected]> --------- Signed-off-by: Ankita Katiyar <[email protected]>
kedro-org · Jan 29, 2024 · 5c61db2 · 5c61db2
1 parent a14b6fa
commit 5c61db2
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 3 deletions.
diff --git a/RELEASE.md b/RELEASE.md
@@ -3,6 +3,7 @@
 ## Major features and improvements
 
 ## Bug fixes and other changes
+* Addressed arbitrary file write via archive extraction security vulnerability in micropackaging.
 
 ## Breaking changes to the API
 

diff --git a/kedro/framework/cli/micropkg.py b/kedro/framework/cli/micropkg.py
@@ -390,12 +390,16 @@ def _is_within_directory(directory: Path, target: Path) -> bool:
 
 
 def safe_extract(tar: tarfile.TarFile, path: Path) -> None:
+    safe_members = []
     for member in tar.getmembers():
         member_path = path / member.name
         if not _is_within_directory(path, member_path):
             # noqa: broad-exception-raised
             raise Exception("Failed to safely extract tar file.")
-    tar.extractall(path)  # nosec B202
+        safe_members.append(member)
+    tar.extractall(path, members=safe_members)  # nosec B202
+    # The nosec is still required because bandit still flags this.
+    # Related issue: https://github.com/PyCQA/bandit/issues/1038
 
 
 def _unpack_sdist(location: str, destination: Path, fs_args: str | None) -> None:

diff --git a/tests/framework/cli/micropkg/test_micropkg_pull.py b/tests/framework/cli/micropkg/test_micropkg_pull.py
@@ -762,7 +762,7 @@ def test_micropkg_pull_invalid_sdist(
         assert sdist_file.is_file()
 
         with tarfile.open(sdist_file, "r:gz") as tar:
-            tar.extractall(tmp_path)
+            safe_extract(tar, tmp_path)
 
         # Create extra project
         extra_project = tmp_path / f"{PIPELINE_NAME}-0.1_extra"
@@ -801,7 +801,7 @@ def test_micropkg_pull_invalid_package_contents(
         assert sdist_file.is_file()
 
         with tarfile.open(sdist_file, "r:gz") as tar:
-            tar.extractall(tmp_path)
+            safe_extract(tar, tmp_path)
 
         # Create extra package
         extra_package = tmp_path / f"{PIPELINE_NAME}-0.1" / f"{PIPELINE_NAME}_extra"