T09 · Insecure Skill Coding Practices
- Location
scripts/start-session.sh:112- Finding
Incomplete JSON Escaping Permits Persistent Session Record Corruption
- Content
View full analysis
"$SESSION_FILE" << EOF { "session_id": "$SESSION_ID", "version": "2.0", "timestamp": "$TIMESTAMP", "time_of_day": "$TIME_LABEL", "duration_planned": $DURATION, "duration_actual": null, "task_category": "$TASK_CATEGORY", "task_description": "$TASK_ESC", "first_microstep": "$MICROSTEP_ESC", "smallest_step": "$SMALLEST_ESC", "energy_start": ${ENERGY_START:-5}, "energy_end": null, "completion_rate": null, "what_worked": [], "what_didnt": [], "dopamine_menu_used": null, "check_ins_completed": 0, "aborted": false, "revenue_impact": null, "check_ins": [] } EOF ``` ### Technical Analysis The custom `json_escape` function escapes backslashes, double quotes, and tab characters, but does not correctly encode every character that JSON prohibits inside an unescaped string. Relevant omissions include carriage returns and other control characters in the U+0000–U+001F range. Because the escaped values are inserted directly into a manually constructed JSON document, accepted control characters can make the resulting session file syntactically invalid. Escaping double quotes prevents a straightforward property-boundary escape, so arbitrary JSON property injection or code execution is not demonstrated by the reviewed co ...[truncated 1563 chars]- Remediation
View remediation
"$SESSION_FILE" ``` 2. If adding `jq` is undesirable, implement complete JSON string encoding for every U+0000–U+001F control character rather than maintaining a partial substitution list. 3. Validate the completed document before retaining it: ```bash jq empty "$SESSION_FILE" ``` 4. Write to a temporary file in the destination directory, validate it, and atomically rename it to `SESSION_FILE`. This prevents incomplete records from remaining after serialization or validation failures. 5. Protect potentially sensitive task descriptions by creating history files with restrictive permissions: ```bash umask 077 mkdir -p "$SESSION_DIR" ``` 6. Add regression tests covering quotes, backslashes, tabs, carriage returns, and other control characters. ]]>
