Skip to content

fix : ReadFileTool shells out on ranged reads and interpolates untrusted path#5537

Open
hamdi-sakly wants to merge 1 commit intogoogle:mainfrom
hamdi-sakly:bug/ReadFileTool
Open

fix : ReadFileTool shells out on ranged reads and interpolates untrusted path#5537
hamdi-sakly wants to merge 1 commit intogoogle:mainfrom
hamdi-sakly:bug/ReadFileTool

Conversation

@hamdi-sakly
Copy link
Copy Markdown

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

2. Description of change:

Problem:

ReadFileTool shells out for ranged reads (start_line > 1 or end_line is set)
instead of reading the file directly:

# Vulnerable code
cmd = f"cat -n '{path}' | sed -n '{sed_range}p'"
res = await self._environment.execute(cmd)

The user-supplied path is interpolated directly into the shell command without
any sanitization. A path containing shell-sensitive characters (e.g. single quotes,
semicolons) is interpreted as shell syntax, allowing unintended commands to be
executed as a side effect of a file-read operation.

Solution:

Used Python's built-in shlex.quote() to safely escape the path before embedding
it in the shell command. shlex.quote() ensures the shell always treats the entire
path as a single string argument regardless of its content.

# Fixed code
import shlex

safe_path = shlex.quote(path)
cmd = f"cat -n {safe_path} | sed -n '{sed_range}p'"
res = await self._environment.execute(cmd)

shlex is a Python standard library module — no new dependencies are required.


Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.
import asyncio
from pathlib import Path
import tempfile

from google.adk.environment._local_environment import LocalEnvironment
from google.adk.tools.environment._tools import ReadFileTool


async def main():
  with tempfile.TemporaryDirectory() as td:
    env = LocalEnvironment(working_dir=Path(td))
    await env.initialize()

    target = Path(td) / "sample.txt"
    target.write_text("line1\nline2\nline3\n", encoding="utf-8")

    marker = Path(td) / "marker.txt"
    # ReadFileTool is expected to treat `path` as data, not as shell syntax.
    # This payload closes the quoted filename used by the ranged-read path,
    # injects an extra command, and reopens the quote to keep the shell happy.
    injected_path = f"sample.txt'; touch {marker}; echo '"

    tool = ReadFileTool(env)
    result = await tool.run_async(
        # `start_line=2` is important because it triggers the code path that
        # shells out through `cat ... | sed ...` instead of doing pure Python
        # file I/O.
        args={"path": injected_path, "start_line": 2},
        tool_context=None,
    )

    print(result)
    # If this prints True, a file-read API caused an unintended side effect.
    # That demonstrates the bug: the path was interpreted by the shell.
    print("marker exists:", marker.exists())
    await env.close()


asyncio.run(main())

The marker.txt file is no longer created, confirming the injected shell
command is no longer executed.

@adk-bot adk-bot added the tools [Component] This issue is related to tools label Apr 29, 2026
@hamdi-sakly hamdi-sakly changed the title BUG : ReadFileTool shells out on ranged reads and interpolates untrusted path fix : ReadFileTool shells out on ranged reads and interpolates untrusted path Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tools [Component] This issue is related to tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ReadFileTool shells out on ranged reads and interpolates untrusted path

2 participants