T09 · Insecure Skill Coding Practices
Error
- Location
- lib/store.js:159
- Finding
- Path Traversal Through Unvalidated Object Type Names<![CDATA[ ## Vulnerability Details **File Location**: `cli.js:79-113`, `lib/store.js:159-163`, and `lib/store.js:166-197` **Vulnerability Type**: Path traversal and unrestricted filesystem write **Risk Level**: High ### Vulnerable Code In `cli.js:79-113`, a type name supplied through the command line is stored without validation and can subsequently be used to create an object: ```js case 'type-add': { // Simple: type-add typename field1:text field2:select(a|b|c) const typeName = args[1]; if (!typeName) { console.log('\nUsage: notebook type-add typename field:text ...'); process.exit(1); } const fields = []; const fieldArgs = args.slice(2); for (const arg of fieldArgs) { const [name, typeWithOpts] = arg.split(':'); if (!typeWithOpts) continue; let type = typeWithOpts; let options = []; if (typeWithOpts.includes('(')) { const match = typeWithOpts.match(/(\w+)\(([^)]+)\)/); if (match) { type = match[1]; options = match[2].split('|'); } } fields.push({ name, type: type === 'select' ? 'select' : type, options: options.length ? options : undefined, required: true }); } saveType({ name: typeName, fields }); console.log(`\n✅ Type "${typeName}" created with ${fields.length} fields.`); break; } ``` In `lib/store.js:159-163`, that type name is used directly as a filesystem path component: ```js function getObjectPath(type, id) { const typeDir = path.join(OBJECTS_DIR, type); if (!fs.existsSync(typeDir)) fs.mkdirSync(typeDir, { recursive: true }); return path.join(typeDir, `${id}.yaml`); } ``` The resulting path is then used for file creation in `lib/store.js:166-197`: ```js function createObject(typeName, data) { const type = getType(typeName); if (!type) throw new Error(`Type "${typeName}" not found`); const id = generateId(); const object = { id, type: typeName, created: today(), updated: new Dat ...[truncated 3500 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Strictly validate type names at every trust boundary.** Permit only a conservative identifier format: ```js const TYPE_NAME_PATTERN = /^[A-Za-z0-9_-]+$/; function validateTypeName(typeName) { if ( typeof typeName !== 'string' || !TYPE_NAME_PATTERN.test(typeName) ) { throw new Error( 'Invalid type name: use only letters, numbers, underscores, and hyphens' ); } } ``` Apply this validation before saving, loading, deleting, or using a type name in any filesystem operation. 2. **Enforce canonical path containment.** Do not rely on input validation alone: ```js function getObjectPath(type, id) { validateTypeName(type); if (!/^[A-Za-z0-9_-]+$/.test(id)) { throw new Error('Invalid object ID'); } const objectsRoot = path.resolve(OBJECTS_DIR); const typeDir = path.resolve(objectsRoot, type); const filePath = path.resolve(typeDir, `${id}.yaml`); if ( typeDir !== objectsRoot && !typeDir.startsWith(objectsRoot + path.sep) ) { throw new Error('Object path escapes the storage directory'); } if (!filePath.startsWith(typeDir + path.sep)) { throw new Error('Object file path escapes the type directory'); } fs.mkdirSync(typeDir, { recursive: true }); return filePath; } ``` 3. **Validate persisted data after loading it.** Existing `types.yaml` and `index.json` files must be treated as untrusted because they can contain legacy or externally modified traversal values. 4. **Protect immutable metadata.** Reject updates to fields such as `id`, `type`, and `created` so object identity cannot diverge from its storage location: ```js const IMMUTABLE_FIELDS = new Set(['id', 'type', 'created']); for (const key of Object.keys(updates)) { if (IMMUTABLE_FIELDS.has(key)) { throw new Error(`Field "${key}" cannot be modified`); } } ``` ...[truncated 355 chars]