T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/okki-integration.js:369
- Finding
- Draft-Controlled Arbitrary File Overwrite During Synchronization<![CDATA[ ## Vulnerability Details **File Location**: `scripts/okki-integration.js:369-400` **Vulnerability Type**: Untrusted property overwrite leading to arbitrary file write **Risk Level**: High ### Vulnerable Code ```js const filepath = path.join(CONFIG.draftsDir, file); try { const draft = JSON.parse(fs.readFileSync(filepath, 'utf8')); // Only process drafts whose status is draft if (draft.status === 'draft') { drafts.push({ filepath, ...draft }); } } catch (e) { log(`Failed to read draft file ${file}: ${e.message}`, 'WARN'); } ``` ```js function updateDraftStatus(draft, newStatus, okkiTrailId = null) { const updatedDraft = { ...draft, status: newStatus, updated_at: new Date().toISOString() }; if (okkiTrailId) { updatedDraft.okki_trail_id = okkiTrailId; } fs.writeFileSync(draft.filepath, JSON.stringify(updatedDraft, null, 2), 'utf8'); log(`Draft status updated: ${draft.draft_id} → ${newStatus}`); } ``` ### Technical Analysis The application calculates a trusted `filepath` from a filename found inside the drafts directory. It then merges that path with attacker-controlled JSON using: ```js { filepath, ...draft } ``` JavaScript object spread applies later properties last. Consequently, a `filepath` property inside the parsed JSON document overrides the trusted path. After a CRM trail is created successfully, `updateDraftStatus()` passes the resulting attacker-controlled `draft.filepath` directly to `fs.writeFileSync()`. There is no canonicalization, directory-containment check, schema validation, or rejection of unexpected properties. This creates a write-what-where condition in which the content is constrained to the serialized draft object, but the destination can be any path writable by the Node.js process. ### Attack Path 1. Obtain the ability to create or modify a file under the project's `drafts` directory. 2. Create a file whose name begins with `draft-` and ends with `.json`. 3. Set its `status` ...[truncated 1138 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Ensure the trusted path is applied after all untrusted properties: ```js drafts.push({ ...draft, filepath }); ``` 2. Prefer not to store an internal filesystem path on the draft object. Pass the trusted path as a separate function argument: ```js updateDraftStatus(filepath, draft, 'synced', trailId); ``` 3. Reject input documents containing internal fields such as `filepath`. 4. Validate drafts against a strict schema with: - Required properties and expected types. - Enumerated status and stage values. - Length limits. - `additionalProperties: false`. 5. Canonicalize and verify every write destination: ```js const base = path.resolve(CONFIG.draftsDir); const target = path.resolve(filepath); if (!target.startsWith(base + path.sep)) { throw new Error('Draft path escapes the drafts directory'); } ``` 6. Use atomic writes through a securely created temporary file followed by `rename()`. 7. Run the integration under a dedicated, least-privileged account that cannot modify executable code, credentials, startup files, or unrelated application data. 8. Add a regression test containing a malicious JSON `filepath` property and verify that no file outside `draftsDir` is modified. ]]>