T09 · Insecure Skill Coding Practices
Error
- Location
- memory-guard.sh:127
- Finding
- Arbitrary Python Code Execution Through Untrusted Integrity Registry Keys<![CDATA[ ## Vulnerability Details **File Location**: `memory-guard.sh:127-143` and `memory-guard.sh:167-171` **Vulnerability Type**: Python source-code injection caused by unsafe shell-variable interpolation **Risk Level**: High ### Vulnerable Code ```bash while IFS= read -r fname; do local stored_hash stored_hash=$(python3 -c " import json d = json.load(open('$HASH_FILE')) print(d.get('$fname', {}).get('hash', 'UNTRACKED')) " 2>/dev/null) [ "$stored_hash" = "UNTRACKED" ] && continue checked=$((checked + 1)) local current_hash current_hash=$(hash_file "$fname") local is_critical is_critical=$(python3 -c " import json d = json.load(open('$HASH_FILE')) print(d.get('$fname', {}).get('critical', False)) " 2>/dev/null) ``` The value assigned to `fname` comes directly from keys in the JSON registry: ```bash done < <(python3 -c " import json for k in json.load(open('$HASH_FILE')): print(k) " 2>/dev/null) ``` ### Technical Analysis The `verify` command reads property names from `.memory-guard/hashes.json` and assigns each property name to the shell variable `fname`. It then embeds that value directly inside Python source passed to `python3 -c`: ```python print(d.get('$fname', {}).get('hash', 'UNTRACKED')) ``` No quoting, escaping, or structural validation is performed before interpolation. A malicious JSON property name containing quote characters, line breaks, and Python statements can terminate the intended Python string literal and introduce attacker-controlled Python code. This is source-code injection rather than ordinary argument injection: the untrusted value is inserted into the program text itself. Shell quoting around the multiline `python3 -c` argument does not protect the Python interpreter from malicious Python syntax embedded through variable expansion. The configured registry path, `HASH_FILE`, is also interpolated into Python source in the same code blocks. Because it is derived from the externally config ...[truncated 2042 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Never interpolate shell variables into Python source.** Pass the registry path and file name as positional arguments: ```bash stored_hash=$(python3 - "$HASH_FILE" "$fname" <<'PY' import json import sys registry_path = sys.argv[1] file_name = sys.argv[2] with open(registry_path, encoding="utf-8") as registry: data = json.load(registry) print(data.get(file_name, {}).get("hash", "UNTRACKED")) PY ) ``` Apply the same pattern to every Python invocation using `HASH_FILE`, `fname`, target file names, timestamps, or other shell-derived values. 2. **Parse the registry once where practical.** A dedicated Python helper can load and validate the JSON, perform all comparisons, and return structured results without repeatedly generating source code. 3. **Validate registry keys against an explicit allowlist.** Reject any key not present in the predefined critical or monitored file lists. If arbitrary tracked paths are later supported, validate them as data rather than executable syntax and reject control characters. 4. **Protect the integrity registry.** Store it outside directories writable by untrusted workspace processes, enforce restrictive ownership and permissions, and authenticate its contents with an HMAC or signature whose key is unavailable to workspace writers. 5. **Validate `MEMORY_GUARD_DIR`.** Treat the environment variable strictly as a filesystem path and pass it only as an argument. Consider requiring an absolute path owned by the invoking user and rejecting unsafe ownership or permissions. 6. **Add regression tests.** Test JSON keys and paths containing single quotes, double quotes, newlines, backslashes, Unicode characters, and Python-like text. Verification must handle these values as inert data and must never execute them. ]]>