T09 · Insecure Skill Coding Practices
Warning
- Location
- hooks/guard.js:87
- Finding
- Non-Atomic Session Counter Allows Concurrent Spawn-Limit Bypass<![CDATA[ ## Vulnerability Details **File Location**: `hooks/guard.js`, lines 87–91 and 122–140 **Vulnerability Type**: Race condition caused by a non-atomic read-check-write operation **Risk Level**: Medium ### Vulnerable Code ```javascript // 2. Per-session spawn budget. Denied calls are not counted. const state = loadState(ev.session_id); const max = parseInt(env('FRUGAL_SUBAGENTS_MAX_SPAWNS', '12'), 10); if (state && Number.isFinite(max) && max > 0 && state.count + 1 > max) { deny(`frugal-subagents: this session has already spawned ${max} subagents (FRUGAL_SUBAGENTS_MAX_SPAWNS). ` + 'Continue with the results you have, or send a follow-up to an agent that is still running ' + 'instead of starting a new one; if more spawns are genuinely needed, ask the user to raise the limit.'); } // Allowed: count it, then either pass through untouched or inject the default. if (state) { state.count += 1; } if (!defaulted) { saveState(state); process.exit(0); } const result = { hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'allow', updatedInput: { ...input, model }, additionalContext: `frugal-subagents: this subagent runs on "${model}" because no model was named. ` + 'Name the model explicitly when a different tier is justified.', }, }; if (state && !state.notified) { state.notified = true; result.systemMessage = `frugal-subagents: subagents spawned without an explicit model run on "${model}" ` + '(FRUGAL_SUBAGENTS_DEFAULT_MODEL). Nested spawns are blocked; ' + `budget ${Number.isFinite(max) && max > 0 ? max : 'unlimited'} spawns per session.`; } saveState(state); out(result); ``` ### Technical Analysis The per-session spawn limit is implemented as a filesystem-backed counter. Each hook process independently: 1. Reads the current counter. 2. Checks whether the next spawn would exceed the limit. 3. Increments its in-m ...[truncated 1831 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Make the load, limit check, increment, and save operation atomic for each session. Recommended hardening steps: 1. Acquire a per-session exclusive lock before reading the counter. 2. Re-read and validate the state only after obtaining the lock. 3. Check the limit and increment the counter while holding the lock. 4. Persist the new state using a temporary file followed by an atomic rename. 5. Release the lock only after the durable update is complete. 6. Define lock timeout and stale-lock recovery behavior so a crashed hook cannot permanently deny future calls. 7. Fail closed for spawn-budget enforcement if valid state cannot be loaded or safely updated, where operational requirements permit. 8. Add an automated concurrency test that launches substantially more simultaneous hook processes than the configured maximum and verifies that no more than the maximum receive approval. A lock library is not strictly required; exclusive file creation with appropriate stale-lock handling can provide serialization. If platform support allows it, a transactional local data store or a dedicated coordinator process would provide stronger semantics. ]]>