T09 · Insecure Skill Coding Practices
- Location
cli.js:251- Finding
Unrestricted Export Path Allows Arbitrary File Overwrite
- Content
View full analysis
Vulnerability Details
File Location:
cli.js:251-257, with the overwrite sink inledger.js:371-400
Vulnerability Type: Unrestricted file path and arbitrary file overwrite
Risk Level: MediumVulnerable Code
javascript // cli.js:251-257 const filename = options.output || `transactions.${format}`; const result = await this.ledger.exportTransactions(format, filename, { period: options.period }); console.log(`📁 Exported ${result.count} transactions to ${result.path}`);javascript // ledger.js:371-400 async exportTransactions(format, filePath, options = {}) { const transactions = await this.loadTransactions(); let filtered = transactions; if (options.period) { filtered = this.filterTransactionsByPeriod(filtered, options.period); } if (format === 'csv') { const csvHeader = 'Date,Amount,Currency,Vendor,Description,Category,Account,Context,ReceiptURL,ConfirmationID\n'; const csvRows = filtered.map(t => [ t.timestamp.split('T')[0], t.amount, t.currency, `"${t.vendor}"`, `"${t.description}"`, t.category, t.account, `"${t.context}"`, t.receiptUrl, t.confirmationId ].join(',')); const csvContent = csvHeader + csvRows.join('\n'); await fs.writeFile(filePath, csvContent); } else if (format === 'json') { await fs.writeFile(filePath, JSON.stringify(filtered, null, 2)); } return { count: filtered.length, path: filePath }; }Technical Analysis
The
--outputCLI option is accepted as an arbitrary path and passed unchanged tofs.writeFile. The implementation does not:- Restrict exports to a dedicated directory.
- Reject absolute paths.
- canonicalize and validate relative paths.
- Prevent
../path traversal. - Detect symlink-based escapes.
- Require explicit permission before replacing an existing file.
Because
fs.writeFiletruncates an existing file by default, any file writable by the Node.js process can be replace ...[truncated 1621 chars]- Remediation
View remediation
Remediation Suggestions
-
Create a dedicated export directory, such as
workspace/ledger/exports. -
Resolve the requested filename against that directory and verify containment:
javascript const exportRoot = path.resolve(this.ledgerDir, 'exports'); await fs.mkdir(exportRoot, { recursive: true }); const requestedName = path.basename(filePath); const destination = path.resolve(exportRoot, requestedName); if (path.dirname(destination) !== exportRoot) { throw new Error('Invalid export path'); } -
Reject absolute paths and traversal components rather than silently normalizing them.
-
Use
fs.lstatand secure directory handling to prevent symlink escapes. -
Avoid overwriting existing files by default. Use the
wxflag and require an explicit overwrite option when replacement is intended:javascript await fs.writeFile(destination, content, { flag: 'wx', mode: 0o600 }); -
Set restrictive file permissions because exports contain financial records.
-
Validate the format against an allowlist such as
csvandjson. -
Add tests covering absolute paths,
../traversal, existing targets, and symlink destinations.
-