T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/update-task-stats.js:62
- Finding
- Incorrect delimiter validation can corrupt the target task file## Vulnerability Details **File Location**: `scripts/update-task-stats.js`, lines 62–68 **Vulnerability Type**: Improper validation of a section delimiter before an in-place file write **Risk Level**: Medium ### Vulnerable Code ```js const statsStart = content.indexOf('## 任务统计'); const statsEnd = content.indexOf('---', statsStart) + 3; if (statsStart === -1 || statsEnd === -1) { console.error('Could not find stats section in file'); process.exit(1); } const newContent = content.substring(0, statsStart) + newStats + content.substring(statsEnd); ``` The resulting content is subsequently written directly over the source file: ```js try { fs.writeFileSync(filePath, newContent, 'utf8'); console.log('Task statistics updated successfully'); console.log(`Total: ${totalTasks}, Running: ${running}, Completed: ${completed}`); console.log(`Persistent: ${persistent}, One-time: ${oneTime}`); } catch (err) { console.error(`Error writing file: ${err.message}`); process.exit(1); } ``` ### Technical Analysis `String.prototype.indexOf()` returns `-1` when the requested delimiter is absent. The script adds `3` to that result before validating it: ```js const statsEnd = content.indexOf('---', statsStart) + 3; ``` If the delimiter is missing, `statsEnd` becomes `2`, not `-1`. Therefore, the condition `statsEnd === -1` cannot detect this failure. A file containing the expected statistics heading but no subsequent `---` delimiter passes validation. The script then constructs replacement content using offset `2` as the end of the old statistics section. Because it writes the result directly over the original file without a backup, temporary file, or atomic rename, malformed input can cause destructive rearrangement or duplication of file content. The caller controls the target path through `process.argv[2]`. Exploitation therefore requires the ability to supply or influence both the processed path and its content, or to modify the normal task file before the ...[truncated 1438 chars]
- Remediation
- ## Remediation Suggestions Validate the raw delimiter-search result before performing arithmetic: ```js const statsStart = content.indexOf('## 任务统计'); const delimiterStart = statsStart === -1 ? -1 : content.indexOf('---', statsStart); if (statsStart === -1 || delimiterStart === -1) { console.error('Could not find a valid stats section in file'); process.exit(1); } const statsEnd = delimiterStart + '---'.length; const newContent = content.substring(0, statsStart) + newStats + content.substring(statsEnd); ``` Apply the following additional hardening measures: 1. Verify that the heading and delimiter occur in the expected order and represent a complete statistics section. 2. Parse the Markdown structure more strictly instead of relying on unrestricted substring searches. 3. Write the updated content to a temporary file in the same directory and atomically rename it over the original only after all validation and writes succeed. 4. Preserve a backup when processing user-important task records. 5. Restrict accepted targets where appropriate, such as requiring a `.md` file within an approved workspace. 6. Add tests for a missing heading, missing delimiter, delimiter before the heading, empty content, truncated content, multiple statistics sections, and write failures.