+ "details": "### Summary\nThe `log_file_name` parameter in the `stata_do` API and CLI is directly interpolated into a Stata command string without sanitization. The security guard (`GuardValidator`) only scans the do-file content but does not validate this parameter. An attacker can inject arbitrary Stata commands (including `shell`, `python`, `erase`, etc.) by crafting a malicious `log_file_name` containing quotes, newlines, or Stata command separators.\n\n### Details\n\nIn `src/stata_mcp/stata/stata_do/do.py`, both `_execute_unix_like` and `_execute_windows` construct a Stata command string using Python f-strings:\n\n```python\ncommands = f\"\"\"\ncapture log close\n{self.generate_log_command(log_file, is_replace)}\n...\ndo \"{dofile_path}\"\n...\n\"\"\"\n```\n\nThe `generate_log_command` method returns:\n\n```python\nlog_cmd = f'log using \"{log_file.as_posix()}\", {replace_clause} {log_type} name({log_type}_log)'\n```\n\nWhere `log_file` is constructed from user-supplied `log_name`:\n\n```python\ndef generate_log_file(self, log_name: str, extension='log'):\n return self.log_file_path / f\"{log_name}.{extension}\"\n```\n\nThe `log_name` parameter comes directly from user input (via MCP tool `stata_do` or CLI `stata-mcp tool do`) without any validation. Since the path is embedded inside double quotes in a Stata command string, an attacker can break out of the string context and inject arbitrary commands.\n\nAdditionally, `generate_log_file` does not prevent path traversal via `log_name`, allowing arbitrary file write outside the intended log directory.\n\n### Proof of Concept\n\nWhen calling `stata_do` via MCP tool with:\n\n```json\n{\n \"dofile_path\": \"test.do\",\n \"log_file_name\": \"'; shell echo pwned > /tmp/pwned.txt; '\"\n}\n```\n\nThe generated Stata commands become:\n\n```stata\nlog using \"<log_dir>/'; shell echo pwned > /tmp/pwned.txt; '.log\", replace text name(text_log)\n```\n\nStata interprets this as multiple commands, with `shell echo pwned > /tmp/pwned.txt;` executed as an arbitrary shell command.\n\n### Impact\n\n- **Remote Code Execution** via `shell` command injection\n- **Arbitrary file write/overwrite** via path traversal in `log_name`\n- Complete bypass of the security guard, as the guard only validates do-file content, not wrapper parameters\n\n### Remediation / Fix\n\n1. Apply strict allowlist validation to `log_name` (only alphanumeric, underscore, dot, hyphen; max 128 chars)\n2. Resolve and verify the constructed log path remains within the intended log directory\n3. Consider generating safe internal filenames (e.g., UUIDs) instead of accepting user-defined log names for command construction\n4. Apply similar sanitization to `dofile_path` before embedding it into Stata command strings\n\n### References\n\n- Issue: #74\n- Fix commit: https://github.com/SepineTam/stata-mcp/commit/e6f945941ae0c7cf5e74a428e0b3dc82b396382f",
0 commit comments