From 9078b4bbdfa79f4e71f9478208211c078e68ce92 Mon Sep 17 00:00:00 2001 From: Eugeniusz Gilewski Date: Fri, 29 May 2026 09:23:03 +0200 Subject: [PATCH] fix(file): harden read_file device alias blocking Security-hardening fix for the read_file device guard, not a new sandbox boundary. The guard already rejects direct device paths and upstream now has a resolved-path pass for workspace symlinks to blocked devices, but its concrete-path helper still compared the expanded path before normalization. That leaves residual alias cases where the dangerous path is visible before final terminal-specific resolution, for example: 1. /dev/../dev/zero and /dev/./urandom should match the blocked-device list as concrete paths, not only after final realpath; 2. /dev/stdin-style aliases can disappear once realpath follows them to /proc/self/fd/0 and then to a tty path; 3. a user symlink to /dev/../dev/stdin exposes the dangerous intermediate target before final resolution, but not necessarily after it. Normalize expanded paths before matching and inspect each symlink hop before falling back to realpath. This preserves the existing /proc fd and /proc pseudo-file guards while enforcing the intended security invariant: model-supplied read paths must not reach blocking or infinite device streams through spelling, normalization, or symlink-hop tricks. Classification: security hardening / residual bypass fix for the read_file device blocklist. This is defensive code at the file-tool boundary, but it fixes a concrete denial-of-service class tracked as security in #10141 and #29158. Tests: - normalized /dev/../dev/zero and /dev/./urandom aliases - symlink to /dev/../dev/stdin blocked before realpath - existing symlink-to-device and regular-symlink guards still pass Fixes #10141 Fixes #29158 --- tests/tools/test_file_read_guards.py | 15 ++++++++++++++ tools/file_tools.py | 30 ++++++++++++++++++++++------ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/tests/tools/test_file_read_guards.py b/tests/tools/test_file_read_guards.py index fbe09f360bc..ee4e43a8774 100644 --- a/tests/tools/test_file_read_guards.py +++ b/tests/tools/test_file_read_guards.py @@ -109,6 +109,10 @@ class TestDevicePathBlocking(unittest.TestCase): for path in ("/proc/cpuinfo", "/proc/meminfo", "/proc/uptime", "/proc/version"): self.assertFalse(_is_blocked_device(path), f"{path} should not be blocked") + def test_normpath_alias_to_blocked_device_is_blocked(self): + self.assertTrue(_is_blocked_device("/dev/../dev/zero")) + self.assertTrue(_is_blocked_device("/dev/./urandom")) + def test_normal_files_not_blocked(self): self.assertFalse(_is_blocked_device("/tmp/test.py")) self.assertFalse(_is_blocked_device("/home/user/.bashrc")) @@ -134,6 +138,17 @@ class TestDevicePathBlocking(unittest.TestCase): self.skipTest(f"symlink unavailable: {exc}") self.assertFalse(_is_blocked_device(link_path)) + def test_symlink_to_blocked_alias_is_blocked_before_realpath(self): + if not os.path.exists("/dev/stdin"): + self.skipTest("/dev/stdin is not available on this platform") + with tempfile.TemporaryDirectory() as tmpdir: + link_path = os.path.join(tmpdir, "stdin-link") + try: + os.symlink("/dev/../dev/stdin", link_path) + except OSError as exc: + self.skipTest(f"symlink unavailable: {exc}") + self.assertTrue(_is_blocked_device(link_path)) + def test_read_file_tool_rejects_device(self): """read_file_tool returns an error without any file I/O.""" result = json.loads(read_file_tool("/dev/zero", task_id="dev_test")) diff --git a/tools/file_tools.py b/tools/file_tools.py index e819b6b6029..3f9a9f2ad13 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -285,7 +285,7 @@ def _path_resolution_warning(filepath: str, resolved: Path, task_id: str = "defa def _is_blocked_device_path(path: str) -> bool: """Return True for concrete device/fd paths that can hang reads.""" - normalized = os.path.expanduser(path) + normalized = os.path.normpath(os.path.expanduser(path)) if normalized in _BLOCKED_DEVICE_PATHS: return True # /proc/self/fd/0-2 and /proc//fd/0-2 are Linux aliases for stdio @@ -306,17 +306,35 @@ def _is_blocked_device(filepath: str) -> bool: """Return True if the path would hang the process (infinite output or blocking input). Check the literal path first so aliases like /dev/stdin are caught before - they resolve to terminal-specific paths. Then check the resolved path so a - workspace symlink to /dev/zero cannot bypass the guard. + they resolve to terminal-specific paths. Then check each symlink hop before + the final resolved path so aliases to devices cannot bypass the guard. """ - normalized = os.path.expanduser(filepath) + normalized = os.path.normpath(os.path.expanduser(filepath)) if _is_blocked_device_path(normalized): return True + + seen: set[str] = set() + current = normalized + for _ in range(20): + try: + target = os.readlink(current) + except OSError: + break + if not os.path.isabs(target): + target = os.path.join(os.path.dirname(current), target) + target = os.path.normpath(target) + if _is_blocked_device_path(target): + return True + if target in seen: + break + seen.add(target) + current = target + try: - resolved = os.path.realpath(normalized) + resolved = os.path.normpath(os.path.realpath(normalized)) except (OSError, ValueError): return False - if resolved != normalized and _is_blocked_device_path(resolved): + if _is_blocked_device_path(resolved): return True return False