T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/send_message.py:48
- Finding
- Arbitrary Shell Command Injection Through Recipient and Message Inputs<![CDATA[ ## Vulnerability Details **File Location**: `scripts/send_message.py`, lines 48–52, 140–145, and 202–205 **Vulnerability Type**: OS command injection caused by shell command construction **Risk Level**: High ### Vulnerable Code ```python def run(cmd, timeout=15): log(f"→ {cmd[:120]}") result = subprocess.run(cmd, shell=True, capture_output=True, text=True, timeout=timeout) if result.returncode != 0 and result.stderr: log(f" stderr: {result.stderr[:200]}") return result ``` ```python def ocr(image_path, keyword=None): """Swift Vision OCR""" ocr_script = os.path.join(SCRIPT_DIR, "ocr_screen.swift") cmd = f'swift {ocr_script} "{image_path}"' if keyword: cmd += f' "{keyword}"' result = run(cmd, timeout=30) ``` ```python def paste_text(text): safe_text = text.replace('"', '\\"') run(f'peekaboo paste --text "{safe_text}" --app "{WECOM_BUNDLE_ID}"') time.sleep(1) ``` ### Technical Analysis The central `run()` function invokes commands with `shell=True`. Both the recipient name and message text originate from command-line arguments and are eventually interpolated into shell command strings. Escaping only the double-quote character is not sufficient for values inserted into a double-quoted shell argument. Shell constructs such as command substitution remain active inside double quotes. For example, `$(command)` and backtick-based substitutions can still be evaluated by the shell. The recipient name reaches `ocr()` as the `keyword` argument through `ocr_find()`. The message reaches `paste_text()`. Consequently, both inputs can introduce shell syntax before the intended `swift` or `peekaboo` process is executed. This is an exploitable command injection flaw rather than merely an argument-injection issue because `/bin/sh` interprets the constructed command. ### Attack Path 1. An attacker causes the Skill to be invoked with a crafted recipient name or message. 2. The crafted value contains ...[truncated 985 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Remove `shell=True` and pass every executable and argument as a separate list element: ```python def run(args, timeout=15): result = subprocess.run( args, shell=False, capture_output=True, text=True, timeout=timeout, check=False, ) return result ``` Invoke the affected tools without constructing shell strings: ```python args = ["swift", ocr_script, image_path] if keyword: args.append(keyword) result = run(args, timeout=30) ``` ```python run([ "peekaboo", "paste", "--text", text, "--app", WECOM_BUNDLE_ID, ]) ``` Apply this argument-array approach consistently to `swift`, `peekaboo`, `cliclick`, `screencapture`, `open`, and `osascript` where practical. Validate recipient length and reject control characters as defense in depth, but do not rely on input filtering or shell escaping as the primary fix. ]]>