fix: Enhancements Needed for Secure Tar Extraction (5560)

sagemaker-core/src/sagemaker/core/common_utils.py

@@ -1688,7 +1688,11 @@ def _is_bad_path(path, base):
         bool: True if the path is not rooted under the base directory, False otherwise.
     """
     # joinpath will ignore base if path is absolute
-    return not _get_resolved_path(joinpath(base, path)).startswith(base)
+    resolved = _get_resolved_path(joinpath(base, path))
+    try:
+        return os.path.commonpath([resolved, base]) != base
+    except ValueError:
+        return True  # If we can't determine safety, treat as bad path
 
 
 def _is_bad_link(info, base):
@@ -1704,23 +1708,26 @@ def _is_bad_link(info, base):
         bool: True if the link is not rooted under the base directory, False otherwise.
     """
     # Links are interpreted relative to the directory containing the link
+    # Wrap with _get_resolved_path to ensure consistent normalization for commonpath comparison
     tip = _get_resolved_path(joinpath(base, dirname(info.name)))
     return _is_bad_path(info.linkname, base=tip)
 
 
-def _get_safe_members(members):
+def _get_safe_members(members, base=None):
     """A generator that yields members that are safe to extract.
 
     It filters out bad paths and bad links.
 
     Args:
-        members (list): A list of members to check.
+        members (list): A list of TarInfo members to check.
+        base (str): The base directory for extraction. If None, defaults to the
+            current working directory (for backward compatibility).
 
     Yields:
         tarfile.TarInfo: The tar file info.
     """
-    base = _get_resolved_path("")
-
+    if base is None:
+        base = _get_resolved_path("")
     for file_info in members:
         if _is_bad_path(file_info.name, base):
             logger.error("%s is blocked (illegal path)", file_info.name)
@@ -1783,7 +1790,8 @@ def custom_extractall_tarfile(tar, extract_path):
     if hasattr(tarfile, "data_filter"):
         tar.extractall(path=extract_path, filter="data")
     else:
-        tar.extractall(path=extract_path, members=_get_safe_members(tar))
+        base = _get_resolved_path(extract_path)
+        tar.extractall(path=extract_path, members=_get_safe_members(tar.getmembers(), base))
         # Re-validate extracted paths to catch symlink race conditions
         _validate_extracted_paths(extract_path)
 

sagemaker-core/src/sagemaker/core/utils/__init__.py

@@ -16,9 +16,19 @@
 for backward compatibility and convenience.
 
 Note: Uses lazy imports via __getattr__ to avoid circular import issues.
+
+Private tar extraction safety helpers (_get_resolved_path, _is_bad_path,
+_is_bad_link, _get_safe_members, _validate_extracted_paths) are internal
+implementation details and are NOT re-exported from this package. They can
+be imported directly from sagemaker.core.common_utils if needed.
+
+custom_extractall_tarfile is the public entry point for safe tar extraction.
 """
 from __future__ import absolute_import
 
+# Public API surface.
+# Note: _save_model is underscore-prefixed but was already in __all__ (pre-existing).
+# custom_extractall_tarfile is the main public entry point for safe tar extraction.
 __all__ = [
     "_save_model",
     "download_file_from_url",