From c4f7db0746e1249ca375ee904d1808c8c0a60304 Mon Sep 17 00:00:00 2001
From: Max Isbey <224885523+maxisbey@users.noreply.github.com>
Date: Thu, 26 Mar 2026 18:43:39 +0000
Subject: [PATCH] feat: reject null bytes in safe_join path components

Null bytes pass through Path construction but fail at the syscall
boundary with a cryptic 'embedded null byte' error. Rejecting in
safe_join gives callers a clear PathEscapeError instead, and guards
against null-byte injection when the path is used for anything other
than immediate file I/O (logging, subprocess args, config).
---
 src/mcp/shared/path_security.py    | 18 ++++++++++++------
 tests/shared/test_path_security.py | 10 ++++++++++
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/mcp/shared/path_security.py b/src/mcp/shared/path_security.py
index 23817b94..8d75a419 100644
--- a/src/mcp/shared/path_security.py
+++ b/src/mcp/shared/path_security.py
@@ -124,21 +124,27 @@ def safe_join(base: str | Path, *parts: str) -> Path:
 
     Args:
         base: The sandbox root. May be relative; it will be resolved.
-        parts: Path components to join. Each is checked for absolute
-            form before joining.
+        parts: Path components to join. Each is checked for null bytes
+            and absolute form before joining.
 
     Returns:
         The resolved path, guaranteed to be within ``base``.
 
     Raises:
-        PathEscapeError: If any part is absolute, or if the resolved
-            path is not contained within the resolved base.
+        PathEscapeError: If any part contains a null byte, any part is
+            absolute, or the resolved path is not contained within the
+            resolved base.
     """
     base_resolved = Path(base).resolve()
 
-    # Reject absolute parts up front: Path's / operator would silently
-    # discard everything to the left of an absolute component.
     for part in parts:
+        # Null bytes pass through Path construction but fail at the
+        # syscall boundary with a cryptic error. Reject here so callers
+        # get a clear PathEscapeError instead.
+        if "\0" in part:
+            raise PathEscapeError(f"Path component contains a null byte; refusing to join onto {base_resolved}")
+        # Absolute parts would silently discard everything to the left
+        # in Path's / operator.
         if is_absolute_path(part):
             raise PathEscapeError(f"Path component {part!r} is absolute; refusing to join onto {base_resolved}")
 
diff --git a/tests/shared/test_path_security.py b/tests/shared/test_path_security.py
index fc3bd909..75a1562d 100644
--- a/tests/shared/test_path_security.py
+++ b/tests/shared/test_path_security.py
@@ -120,6 +120,16 @@ def test_safe_join_rejects_windows_drive(tmp_path: Path):
         safe_join(tmp_path, "C:\\Windows\\System32")
 
 
+def test_safe_join_rejects_null_byte(tmp_path: Path):
+    with pytest.raises(PathEscapeError, match="null byte"):
+        safe_join(tmp_path, "file\0.txt")
+
+
+def test_safe_join_rejects_null_byte_in_later_part(tmp_path: Path):
+    with pytest.raises(PathEscapeError, match="null byte"):
+        safe_join(tmp_path, "docs", "file\0.txt")
+
+
 def test_safe_join_rejects_symlink_escape(tmp_path: Path):
     outside = tmp_path / "outside"
     outside.mkdir()