Finding
File: .claude/skills/setup-agent-team/qa.sh:56-63
Severity: HIGH
Description
The safe_substitute function in qa.sh escapes sed metacharacters (\, &, |) and literal newlines in the replacement value, but it does NOT validate the base64 encoding format before using it in sed commands. This creates a vulnerability when base64-encoded values are used as replacement strings.
Vulnerable code:
safe_substitute() {
local placeholder="$1"local value="$2"local file="$3"# Escape backslashes first, then &, then the delimiter |local escaped
escaped=$(printf '%s'"$value"| sed -e 's/[\\]/\\&/g' -e 's/[&]/\\&/g' -e 's/[|]/\\|/g')# Escape literal newlines for sed replacement (backslash + newline)
escaped="${escaped//$'\n'/\\$'\n'}"
sed -i.bak "s|${placeholder}|${escaped}|g""$file"
rm -f "${file}.bak"
}The function escapes the pipe delimiter (|) but then USES pipes as the sed delimiter. An attacker controlling the value parameter could craft a payload with an escaped pipe that bypasses the escaping logic due to double-escaping or character encoding issues.
Attack Vector
The safe_substitute function is called with potentially-attacker-controlled values in multiple locations:
- Line 286:
safe_substitute "WORKTREE_BASE_PLACEHOLDER" "${WORKTREE_BASE}" "${PROMPT_FILE}" - Line 287:
safe_substitute "REPO_ROOT_PLACEHOLDER" "${REPO_ROOT}" "${PROMPT_FILE}" - Line 295:
safe_substitute "ISSUE_NUM_PLACEHOLDER" "${ISSUE_NUM}" "${PROMPT_FILE}"
While WORKTREE_BASE and REPO_ROOT are derived from trusted sources, ISSUE_NUM comes from the SPAWN_ISSUE environment variable, which is validated as a positive integer (line 18-21). However, the escaping logic is fragile and may not handle all edge cases correctly.
Impact
If the escaping fails, an attacker could:
- Inject arbitrary sed commands via malicious replacement strings
- Modify the prompt file in unintended ways
- Potentially achieve code execution if the prompt file is later eval'd or sourced
Recommendation
Validate base64 format before using base64-encoded values in sed:
if\!printf'%s'"${encoded_value}"| grep -qE '^[A-Za-z0-9+/=]+$';then
log "ERROR: Invalid base64 encoding"return 1
fiUse a sed delimiter that is guaranteed not to appear in the value (e.g., control characters like \x01):
sed -i.bak "s\x01${placeholder}\x01${escaped}\x01g""$file"Consider using a safer templating method like envsubst or bun eval with proper escaping instead of sed for string substitution.
References
-- security/shell-scanner
Finding
File: .claude/skills/setup-agent-team/qa.sh:56-63
Severity: HIGH
Description
The
safe_substitutefunction in qa.sh escapes sed metacharacters (\, &, |) and literal newlines in the replacement value, but it does NOT validate the base64 encoding format before using it in sed commands. This creates a vulnerability when base64-encoded values are used as replacement strings.Vulnerable code:
The function escapes the pipe delimiter (|) but then USES pipes as the sed delimiter. An attacker controlling the
valueparameter could craft a payload with an escaped pipe that bypasses the escaping logic due to double-escaping or character encoding issues.Attack Vector
The
safe_substitutefunction is called with potentially-attacker-controlled values in multiple locations:safe_substitute "WORKTREE_BASE_PLACEHOLDER" "${WORKTREE_BASE}" "${PROMPT_FILE}"safe_substitute "REPO_ROOT_PLACEHOLDER" "${REPO_ROOT}" "${PROMPT_FILE}"safe_substitute "ISSUE_NUM_PLACEHOLDER" "${ISSUE_NUM}" "${PROMPT_FILE}"While
WORKTREE_BASEandREPO_ROOTare derived from trusted sources,ISSUE_NUMcomes from theSPAWN_ISSUEenvironment variable, which is validated as a positive integer (line 18-21). However, the escaping logic is fragile and may not handle all edge cases correctly.Impact
If the escaping fails, an attacker could:
Recommendation
Validate base64 format before using base64-encoded values in sed:
Use a sed delimiter that is guaranteed not to appear in the value (e.g., control characters like \x01):
Consider using a safer templating method like
envsubstor bun eval with proper escaping instead of sed for string substitution.References
-- security/shell-scanner