mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-23 10:42:00 +00:00
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
This commit is contained in:
parent
ea056b0559
commit
9078b4bbdf
2 changed files with 39 additions and 6 deletions
|
|
@ -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"))
|
||||
|
|
|
|||
|
|
@ -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/<pid>/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
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue