T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/setup-daily-reporting.sh:18
- Finding
- Persistent Cron Command Injection Through an Unvalidated Business Name<![CDATA[ ## Vulnerability Details **File Location**: `scripts/setup-daily-reporting.sh:18-24, 92-110` **Vulnerability Type**: Persistent command injection through generated crontab content **Risk Level**: High ### Vulnerable Code ```bash # Prompt for business name if [ -z "$1" ]; then read -p "Business name (e.g., my-company): " BUSINESS_NAME else BUSINESS_NAME="$1" fi BUSINESS_DIR="$HOME/business/$BUSINESS_NAME" ``` The unvalidated value is subsequently embedded into paths written directly to a cron configuration: ```bash # Setup cron jobs CRON_FILE="$HOME/.business-cron-$BUSINESS_NAME" cat > "$CRON_FILE" << EOF # AI Business Hierarchies - Automated Reporting for $BUSINESS_NAME # Daily Supervisor Reports (8 AM UTC) 0 8 * * * $REPORT_SCRIPT # Weekly CEO Report (Monday 9 AM UTC) 0 9 * * 1 $BUSINESS_DIR/scripts/generate-weekly-report.sh # Monthly Strategy Review (1st of month, 10 AM UTC) 0 10 1 * * $BUSINESS_DIR/scripts/generate-monthly-report.sh EOF echo -e "${YELLOW}Installing cron jobs...${NC}" crontab -l > /tmp/current-cron 2>/dev/null || touch /tmp/current-cron cat "$CRON_FILE" >> /tmp/current-cron crontab /tmp/current-cron ``` ### Technical Analysis `BUSINESS_NAME` is accepted from either a command-line argument or interactive input without enforcing an identifier format. It is used to construct `BUSINESS_DIR`, `REPORT_SCRIPT`, and `CRON_FILE`. The resulting paths are inserted into executable cron lines without shell quoting or escaping. Cron executes the command portion of each entry through a shell. Consequently, shell metacharacters in a business name—such as semicolons, command substitutions, comment markers, or line breaks—can alter the command interpreted by cron. The directory existence check does not sanitize the value; an attacker can create a correspondingly named directory or target an existing specially named directory. Because the generated text is installed with `crontab`, successful injection persists beyond the setup ...[truncated 2024 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Enforce a strict allowlist before constructing any path: ```bash if [[ ! "$BUSINESS_NAME" =~ ^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$ ]]; then printf 'Error: business name may contain only letters, digits, underscores, and hyphens.\n' >&2 exit 1 fi ``` 2. Explicitly reject control characters, whitespace, slashes, shell metacharacters, and leading hyphens. 3. Do not place variable paths directly into cron command fields. Generate a fixed wrapper command and pass data through a validated configuration file, or apply robust shell quoting before writing an entry. 4. Use managed start/end markers for this Skill’s entries rather than appending unrestricted content: ```text # BEGIN ai-business-hierarchies: validated-name ... # END ai-business-hierarchies: validated-name ``` 5. Show the exact proposed entries and require explicit user confirmation before changing the crontab. 6. Apply the same business-name validation in `scripts/setup-business.sh` so malformed or ambiguous directory names cannot be created through the normal workflow. 7. Add automated tests covering semicolons, command substitutions, spaces, tabs, line breaks, percent signs, comment markers, slashes, and leading hyphens. ]]>