T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/auto_screenshot.py:14
- Finding
- Shell Command Injection Through Attacker-Controlled HTML Path<![CDATA[ ## Vulnerability Details **File Location**: `scripts/auto_screenshot.py`, lines 14–23 **Vulnerability Type**: OS command injection through unsafe shell command construction **Risk Level**: High ### Vulnerable Code ```python def take_screenshot(html_path, output_path): """Take screenshot using OpenClaw browser tool""" html_path = os.path.abspath(html_path) output_path = os.path.abspath(output_path) file_url = f"file://{html_path}" print(f"🌐 Opening browser: {file_url}") # Open in browser open_cmd = f'browser open --url "{file_url}"' result = subprocess.run(open_cmd, shell=True, capture_output=True, text=True) ``` The command-line entry point only checks whether the supplied path exists before passing it to the vulnerable function: ```python html_path = sys.argv[1] output_path = sys.argv[2] if not os.path.exists(html_path): print(f"❌ HTML file not found: {html_path}") sys.exit(1) success = take_screenshot(html_path, output_path) ``` ### Technical Analysis The HTML path is obtained directly from a command-line argument and interpolated into a shell command. The command is then executed with `shell=True`. Wrapping the value in double quotes does not make it safe. A filename can contain a double quote and shell metacharacters on supported Unix-like filesystems. An attacker who controls or can create the input filename can terminate the quoted URL argument and append another shell command. The existence check does not mitigate the issue because an attacker can create a file whose literal filename contains the required quote and shell syntax. Converting the path with `os.path.abspath()` also performs no shell escaping or validation. The screenshot command at line 36 uses a constant command string and is not independently injectable. The confirmed injection point is the dynamically constructed `open_cmd`. ### Attack Path 1. An attacker obtains the ability to create an HTML file with a specially c ...[truncated 1409 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Eliminate shell interpretation and pass each command argument directly to `subprocess.run()`: ```python result = subprocess.run( ["browser", "open", "--url", file_url], shell=False, capture_output=True, text=True, check=False, ) ``` Apply additional defense-in-depth controls: 1. Resolve the input with `Path.resolve(strict=True)`. 2. Require the source to be a regular file with an approved extension such as `.html`. 3. Restrict input files to a dedicated trusted directory: ```python allowed_root = (Path(__file__).parent.parent / "output").resolve() html_file = Path(html_path).resolve(strict=True) if not html_file.is_file() or html_file.suffix.lower() != ".html": raise ValueError("Input must be an existing HTML file") if allowed_root not in html_file.parents: raise ValueError("Input must be inside the output directory") ``` 4. Avoid building any future commands through string interpolation. 5. Return failure unless the requested screenshot is actually written and validated. 6. Add regression tests using filenames containing quotes, semicolons, command substitutions, whitespace, and newline characters. ]]>