T09 · Insecure Skill Coding Practices
Warning
- Location
- references/ss-helpers.sh:23
- Finding
- Predictable Shared Token Cache Permits Symlink-Based File Overwrite<![CDATA[ ## Vulnerability Details **File Location**: `references/ss-helpers.sh`, lines 23 and 53–87 **Vulnerability Type**: Predictable unsafe temporary file and symlink following **Risk Level**: Medium ### Vulnerable Code ```bash SS_TOKEN_CACHE="${TMPDIR:-/tmp}/simplisafe-skill-token.json" ``` ```bash ss_access_token() { local now now=$(date +%s) if [ -f "$SS_TOKEN_CACHE" ]; then local cached_exp cached_tok cached_exp=$(jq -r '.expiresAt // 0' "$SS_TOKEN_CACHE" 2>/dev/null || echo 0) cached_tok=$(jq -r '.accessToken // ""' "$SS_TOKEN_CACHE" 2>/dev/null || echo '') # 10# forces base-10 so a value with a leading zero can't be read as octal, # which is a real bash failure mode (zsh has no such rule, so an untested # helper works for its author and breaks for everyone else). if [ -n "$cached_tok" ] && [ "$((10#${cached_exp:-0}))" -gt "$((now + 120))" ]; then printf '%s' "$cached_tok" return 0 fi fi local rt rt=$(ss_refresh_token) || return 1 local body response access expires # Build the JSON with jq so a token containing quotes/backslashes can't break it. body=$(jq -nc --arg cid "$SS_CLIENT_ID" --arg rt "$rt" \ '{grant_type:"refresh_token", client_id:$cid, refresh_token:$rt}') response=$(curl -sS -X POST "$SS_AUTH_URL" -H 'Content-Type: application/json' -d "$body") || return 1 access=$(printf '%s' "$response" | jq -r '.access_token // ""') if [ -z "$access" ]; then echo "ss_access_token: refresh failed." >&2 # Echo only the error fields, never the whole body. printf '%s' "$response" | jq -r '" \(.error // "?"): \(.error_description // "?")"' >&2 echo " If the token was revoked, re-run scripts/bootstrap-auth.mjs." >&2 return 1 fi expires=$(printf '%s' "$response" | jq -r '.expires_in // 3600') ( umask 077; jq -nc --arg t "$access" --argjson e "$((now + expires))" \ '{accessToken:$t, expiresAt:$e}' > "$SS_TOKEN_CACHE" ) printf '%s' "$access" } ``` ...[truncated 2694 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Store the cache in a private per-user runtime directory rather than directly under a shared `/tmp` namespace. Prefer `${XDG_RUNTIME_DIR}` after validating that it is owned by the current user and inaccessible to other users. 2. If a runtime directory is unavailable, create a private directory with `mktemp -d` and mode `0700`. 3. Use a per-user cache name or directory so different users cannot share or interfere with the same path. 4. Reject cache paths that are symbolic links or are not regular files owned by the current user. 5. Write tokens atomically: - Create a temporary file inside the private directory with exclusive creation. - Set mode `0600`. - Write and validate the JSON. - Atomically rename it over the cache file. 6. Validate the ownership and permissions of an existing cache before reading it. 7. Consider avoiding persistent access-token caching entirely if the operational cost of obtaining a fresh token is acceptable. A hardened design should resemble: ```bash cache_root="${XDG_RUNTIME_DIR:-}" if [ -z "$cache_root" ] || [ ! -d "$cache_root" ] || [ ! -O "$cache_root" ]; then cache_root=$(mktemp -d "${TMPDIR:-/tmp}/simplisafe-${UID}.XXXXXX") || return 1 chmod 700 "$cache_root" || return 1 fi SS_TOKEN_CACHE="$cache_root/simplisafe-token.json" tmp_cache=$(mktemp "$cache_root/token.XXXXXX") || return 1 chmod 600 "$tmp_cache" || { rm -f "$tmp_cache" return 1 } jq -nc --arg t "$access" --argjson e "$((now + expires))" \ '{accessToken:$t, expiresAt:$e}' > "$tmp_cache" || { rm -f "$tmp_cache" return 1 } mv -f "$tmp_cache" "$SS_TOKEN_CACHE" ``` Ownership, file-type, and permission checks must still be applied before consuming an existing cache. ]]>