T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/voice_translate_app/notifier.py:47
- Finding
- Shell Command Injection Through Notification Hooks<![CDATA[ ## Vulnerability Details **File Location**: `scripts/voice_translate_app/notifier.py:47-59` **Additional Locations**: `scripts/voice_translate_app/cli.py:42-44, 67-69`; `scripts/send_text.py:25-26, 54`; `scripts/send_audio.py:15-16, 38-44` **Vulnerability Type**: OS command injection through unsafe shell invocation **Risk Level**: High ### Vulnerable Code ```python def _run_text_command(self, command: str | None, text: str) -> None: if not command: return subprocess.run(command, input=text.encode("utf-8"), shell=True, check=True) def _run_audio_command(self, command: str | None, audio_file: Path) -> None: if not command: return if "{audio_file}" in command: resolved = command.format(audio_file=str(audio_file)) else: resolved = f'{command} "{audio_file}"' subprocess.run(resolved, shell=True, check=True) ``` Related standalone wrapper: ```python command = args.command_template.format(audio_file=shlex.quote(str(audio_path))) if args.dry_run: print(command) return subprocess.run(command, shell=True, check=True) ``` ### Technical Analysis Notification commands are accepted from command-line arguments or environment-backed command templates and passed to `subprocess.run` with `shell=True`. This causes the system shell to interpret command separators, substitutions, redirections, pipelines, and other metacharacters. The audio notifier introduces an additional injection boundary by interpolating `audio_file` directly into the shell command. In the placeholder branch, the path is inserted without any quoting. In the fallback branch, it is enclosed in double quotes but embedded quote characters are not escaped. Because the generated audio path includes the operator-provided output directory, an attacker who can influence that directory can potentially insert shell syntax into the resulting command. The standalone sender uses `shlex.quote` for the audio path, which protects that specif ...[truncated 1729 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove `shell=True` and execute commands using argument arrays: ```python import shlex import subprocess argv = shlex.split(command) subprocess.run(argv, input=text.encode("utf-8"), check=True) ``` 2. For audio notifications, append the path as a separate argument rather than interpolating it into a command string: ```python argv = shlex.split(command) argv.append(str(audio_file)) subprocess.run(argv, check=True) ``` 3. Replace free-form shell templates with a structured configuration containing an executable and argument list. 4. If placeholder support is required, substitute placeholders at the argument level after parsing, not in a complete shell string. 5. Validate configured executables against an allowlist or require absolute executable paths in security-sensitive deployments. 6. Treat notifier configuration and related environment variables as privileged configuration. Do not populate them from chat content, transcript content, attachment metadata, or other untrusted input. 7. Add regression tests using paths containing spaces, quotes, semicolons, command substitutions, and newline characters to verify that they remain literal arguments. 8. Run the pipeline under a dedicated, minimally privileged operating-system account and restrict its filesystem and network access. ]]>