T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/scan.sh:37
- Finding
- Unvalidated executable environment snapshot permits recurring command execution<![CDATA[ ## Vulnerability Details **File Location**: `scripts/scan.sh:37-40`; snapshot creation is specified in `SKILL.md:58-66` **Vulnerability Type**: Unsafe sourcing of a writable configuration file **Risk Level**: Medium ### Complete Code Snippet Snapshot creation: ```bash cat > ~/.okx/earn-hunter/env.snapshot << SNAP # auto-generated by earn-hunter activation — $(date -Iseconds) OKX_BIN=$(command -v okx) NODE_BIN=$(command -v node) JQ_BIN=$(command -v jq) ACTIVATION_PATH=$PATH SNAP ``` Snapshot execution: ```bash _EH_SNAPSHOT="${EH_STATE_DIR:-$HOME/.okx/earn-hunter}/env.snapshot" # shellcheck disable=SC1090 [[ -f "$_EH_SNAPSHOT" ]] && source "$_EH_SNAPSHOT" ``` ### Technical Analysis The snapshot is intended only to preserve executable paths for cron, but it is stored as executable shell syntax and later loaded with `source`. Sourcing a file executes every command in that file rather than merely reading its data. The generated values are not safely shell-escaped. In particular, `ACTIVATION_PATH=$PATH` can produce additional shell statements if the environment value contains a newline or other shell syntax. Separately, any process capable of modifying `~/.okx/earn-hunter/env.snapshot` can insert arbitrary commands. The `EH_STATE_DIR` environment variable also controls the directory from which the snapshot is sourced. This is useful for tests, but it expands the execution surface because an invocation with an attacker-controlled value can cause a different snapshot to be executed. The risk is amplified by the scheduler: after activation, the vulnerable source operation is performed by every cron, LaunchAgent, or interactive scan. ### Attack Path 1. An attacker influences the activation environment, modifies `env.snapshot`, or causes the script to run with a malicious `EH_STATE_DIR`. 2. The selected snapshot contains shell commands in addition to the expected variable assignments. 3. `scan.sh` tests only whether the file exists; it performs no ow ...[truncated 1115 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not represent path data as executable shell code. 2. Store resolved paths in JSON, for example: ```json { "okx_bin": "/usr/local/bin/okx", "node_bin": "/usr/local/bin/node", "jq_bin": "/usr/local/bin/jq" } ``` 3. Read each value as data with `jq`: ```bash _OKX_BIN=$(jq -r '.okx_bin // empty' "$SNAPSHOT_FILE") _NODE_BIN=$(jq -r '.node_bin // empty' "$SNAPSHOT_FILE") _JQ_BIN=$(jq -r '.jq_bin // empty' "$SNAPSHOT_FILE") ``` 4. Validate that every resulting value is an absolute path to an executable regular file. 5. Create the state directory with mode `0700` and snapshot with mode `0600`. 6. Check that the snapshot is owned by the current user and is not group- or world-writable. 7. Remove `ACTIVATION_PATH` because the script reconstructs a restricted `PATH` from resolved executable directories. 8. Restrict `EH_STATE_DIR` to explicit test mode, or reject it when a production scheduler invokes the script. 9. If shell assignments must be retained, generate them with robust shell escaping such as `printf '%q'`; this is less safe than using a non-executable format and should not be the preferred fix. ]]>