T09 · Insecure Skill Coding Practices
- Location
skill.md:236- Finding
Shell Command Injection Through Unsafe subprocess Usage
- Content
View full analysis
Vulnerability Details
File Location:
skill.md:236-237,skill.md:277,reference/best-practices.md:48,reference/best-practices.md:110-111, andreference/best-practices.md:158
Vulnerability Type: OS command injection
Risk Level: HighVulnerable Code
python cmd = f"hcloud obs upload-object --bucket={bucket} --key={file} --file={local_dir}/{file}" subprocess.run(cmd, shell=True)The generic command helper repeats the same unsafe pattern:
python def safe_command(cmd): try: result = subprocess.run(cmd, shell=True, capture_output=True, text=True) if result.returncode != 0: print(f"Error: {result.stderr}") return None return json.loads(result.stdout) except Exception as e: print(f"Exception: {e}") return NoneTechnical Analysis
The examples construct command strings using values such as
bucket,file, andlocal_dir, and then execute those strings through a command shell. No quoting, validation, or separation between executable arguments and shell syntax is applied.A filename or argument containing shell metacharacters can therefore alter the intended command. The generic
safe_commandhelper is also unsafe because it accepts an arbitrary command string and forwards it directly to the shell. Error handling does not make this execution pattern safe.The duplicated examples in
skill.mdandreference/best-practices.mdincrease the likelihood that users will copy this vulnerable pattern into automation with access to production cloud credentials.Attack Path
- An attacker gains the ability to create or influence a filename in a directory processed by
batch_upload, or to control another value passed into a command-building helper. - The attacker supplies a value containing shell syntax, such as a command separator followed by an attacker-selected command.
- The Python code ...[truncated 1096 chars]
- An attacker gains the ability to create or influence a filename in a directory processed by
- Remediation
View remediation
Remediation Suggestions
- Remove
shell=Trueand pass each command argument as a separate list element. - Validate bucket names, server identifiers, object keys, and other externally influenced values against restrictive allowlists.
- Use
os.path.joinorpathlib.Pathfor local paths rather than string concatenation. - Do not expose a generic helper that accepts arbitrary command strings.
- Enable
check=Trueso command failures are handled consistently. - Run automation under a least-privileged local account and a narrowly scoped IAM identity.
- Add tests using filenames and parameters containing spaces, quotes, semicolons, command substitutions, and newline characters.
A safer implementation is:
python import os import subprocess def upload_object(bucket, local_dir, filename): local_path = os.path.join(local_dir, filename) return subprocess.run( [ "hcloud", "obs", "upload-object", f"--bucket={bucket}", f"--key={filename}", f"--file={local_path}", ], shell=False, check=True, capture_output=True, text=True, )- Remove
