T09 · Insecure Skill Coding Practices
- Location
bin/session-warmer.mjs:124- Finding
Unvalidated configuration permits persistent cron command injection and denial of service
- Content
View full analysis
Vulnerability Details
File Location:
bin/session-warmer.mjs:50-61, 89-94, 124-142
Vulnerability Type: Cron injection through unvalidated configuration and paths
Risk Level: MediumThe application merges arbitrary values from
config.jsoninto its configuration without validating their types, ranges, or characters:js function loadConfig() { const path = join(ROOT, "config.json"); if (!existsSync(path)) return { ...DEFAULTS, _usingDefaults: true }; try { const user = JSON.parse(readFileSync(path, "utf8")); return { ...DEFAULTS, ...user }; } catch (e) { console.error(`! Could not parse config.json: ${e.message}\n Falling back to defaults.`); return { ...DEFAULTS, _usingDefaults: true }; } }Schedule arithmetic also uses these values without checking that they are finite, positive, or within reasonable bounds:
js function computeSchedule(cfg) { const start = hhmmToMinutes(cfg.anchor); const end = hhmmToMinutes(cfg.day_end); const step = cfg.window_minutes + cfg.buffer_minutes; const times = []; for (let t = start; t <= end; t += step) times.push(t); return times; // minutes-of-day, in cfg.timezone }The installation command then places the unvalidated timezone and unquoted project paths directly into shell-interpreted crontab output:
js function cmdInstall(cfg, cronOnly) { const times = computeSchedule(cfg); const self = join(ROOT, "bin", "session-warmer.mjs"); const cronLines = times.map((t) => { const hh = Math.floor(t / 60), mm = t % 60; return `${mm} ${hh} * * * node ${self} warm >> ${join(ROOT, "warmer.log")} 2>&1`; }); if (cronOnly) { // pipeable: just the crontab lines (plus CRON_TZ), nothing else console.log(`CRON_TZ=${cfg.timezone}`); cronLines.forEach((l) => console.log(l)); return; } console.log("# --- Option A: cron on this VPS (recommended) ---") ...[truncated 3584 chars]- Remediation
View remediation
Remediation Suggestions
-
Introduce a strict configuration schema before any command is executed:
- Require
enabledanddry_runto be booleans. - Require
anchorandday_endto match a strictHH:MMformat and verify hour/minute ranges. - Require
window_minutesandbuffer_minutesto be finite integers within conservative positive bounds. - Explicitly require
window_minutes + buffer_minutes > 0. - Require
claude_argsto be an array containing only strings.
- Require
-
Validate
timezoneusingIntl.DateTimeFormatand reject all control characters, especially carriage returns and newlines. Only emit the value after confirming that it is a recognized IANA timezone. -
Do not concatenate filesystem paths directly into cron shell commands. Use a robust POSIX shell-quoting function for the Node executable, script path, and log path, or generate a fixed wrapper script whose location is itself safely quoted.
-
Resolve and emit an explicit trusted Node binary path instead of relying on cron’s restricted or attacker-influenced
PATH. -
Add a hard upper bound to the number of generated schedule entries so malformed arithmetic cannot consume unbounded memory:
js if (!Number.isInteger(step) || step <= 0 || step > 1440) { throw new Error("Invalid scheduling interval"); } -
Avoid recommending direct piping into
crontabuntil all output fields are validated and escaped. Prefer writing the proposed crontab to a temporary preview file and requiring explicit review before installation. -
Add regression tests covering newline-bearing timezone values, shell metacharacters in installation paths, malformed times, non-numeric values, zero/negative intervals, and excessively large schedule configurations.
-