+ "details": "## Affected Package\n\n| Field | Value |\n|-------|-------|\n| **Package** | `@tinacms/cli` |\n| **Version** | `2.0.5` (latest at time of discovery) |\n| **Vulnerable File** | `packages/@tinacms/cli/src/next/commands/dev-command/server/media.ts` |\n| **Vulnerable Lines** | 42-43 |\n\n---\n\n## Summary\n\nA **path traversal vulnerability (CWE-22)** exists in the TinaCMS development server's media upload handler. The code at `media.ts:42-43` joins user-controlled path segments using `path.join()` without validating that the resulting path stays within the intended media directory. This allows writing files to arbitrary locations on the filesystem.\n\n**Attack Vector**: Network (HTTP POST request) \n**Impact**: Arbitrary file write, potential Remote Code Execution\n\n---\n\n## Details\n\n### Vulnerable Code Location\n\n**File**: `packages/@tinacms/cli/src/next/commands/dev-command/server/media.ts` \n**Lines**: 42-43\n\n```typescript\nbb.on('file', async (_name, file, _info) => {\n const fullPath = decodeURI(req.url?.slice('/media/upload/'.length)); // Line 42\n const saveTo = path.join(mediaFolder, ...fullPath.split('/')); // Line 43\n // make sure the directory exists before writing the file\n await fs.ensureDir(path.dirname(saveTo));\n file.pipe(fs.createWriteStream(saveTo));\n});\n```\n\n### Root Cause\n\nThe `path.join()` function resolves `..` (parent directory) segments in the path. When the user-supplied path contains traversal sequences like `../../../etc/passwd`, these are resolved relative to the media folder, allowing escape to arbitrary filesystem locations.\n\n**Example**:\n```javascript\nconst mediaFolder = '/app/public/uploads';\nconst maliciousInput = '../../../tmp/evil.txt';\nconst saveTo = path.join(mediaFolder, ...maliciousInput.split('/'));\n// Result: '/tmp/evil.txt' - OUTSIDE the media folder!\n```\n\n### Additional Affected Endpoints\n\nThe same vulnerability pattern exists in:\n\n1. **Delete Handler** (`handleDelete`, lines 29-33) - Arbitrary file deletion\n2. **List Handler** (`handleList`, lines 16-27) + `MediaModel.listMedia` - Directory enumeration\n3. **MediaModel.deleteMedia** (lines 201-217) - Arbitrary file deletion\n\nSimilar code also exists in the Express version at:\n- `packages/@tinacms/cli/src/server/routes/index.ts`\n- `packages/@tinacms/cli/src/server/models/media.ts`\n\n---\n\n## PoC\n\n### Quick Verification (No Server Required)\n\nThis Node.js script directly tests the vulnerable code logic:\n\n```javascript\n#!/usr/bin/env node\n/**\n * TinaCMS Path Traversal Vulnerability - Direct Code Test\n * Run: node test-vulnerability.js\n */\n\nconst path = require('path');\nconst fs = require('fs');\n\n// Simulated configuration (matches typical TinaCMS setup)\nconst rootPath = '/tmp/tinacms-test';\nconst publicFolder = 'public';\nconst mediaRoot = 'uploads';\nconst mediaFolder = path.join(rootPath, publicFolder, mediaRoot);\n\n// Setup test directories\nfs.mkdirSync(path.join(rootPath, publicFolder, mediaRoot), { recursive: true });\nfs.mkdirSync('/tmp/target-dir', { recursive: true });\n\nconsole.log(`Media folder: ${mediaFolder}`);\n\n// Simulate vulnerable code from media.ts:42-43\nfunction vulnerableUpload(reqUrl) {\n const fullPath = decodeURI(reqUrl.slice('/media/upload/'.length));\n const saveTo = path.join(mediaFolder, ...fullPath.split('/'));\n return saveTo;\n}\n\n// Test cases\nconst tests = [\n { url: '/media/upload/image.png', desc: 'Normal upload' },\n { url: '/media/upload/../../../tmp/target-dir/evil.txt', desc: 'Path traversal' },\n];\n\ntests.forEach(test => {\n const result = vulnerableUpload(test.url);\n const isVuln = !path.resolve(result).startsWith(path.resolve(mediaFolder));\n \n console.log(`\\n${test.desc}:`);\n console.log(` Input: ${test.url}`);\n console.log(` Result: ${result}`);\n console.log(` Vulnerable: ${isVuln ? 'YES ⚠️' : 'No ✓'}`);\n \n if (isVuln) {\n // Actually write the file to prove it works\n fs.mkdirSync(path.dirname(result), { recursive: true });\n fs.writeFileSync(result, `PWNED at ${new Date().toISOString()}`);\n console.log(` File written: ${fs.existsSync(result)}`);\n }\n});\n\n// Cleanup\nfs.rmSync(rootPath, { recursive: true, force: true });\n```\n\n### Output\n\n```\nMedia folder: /tmp/tinacms-test/public/uploads\n\nNormal upload:\n Input: /media/upload/image.png\n Result: /tmp/tinacms-test/public/uploads/image.png\n Vulnerable: No ✓\n\nPath traversal:\n Input: /media/upload/../../../tmp/target-dir/evil.txt\n Result: /tmp/tmp/target-dir/evil.txt\n Vulnerable: YES ⚠️\n File written: true\n```\n\nThe file was successfully written to `/tmp/tmp/target-dir/evil.txt`, which is **completely outside** the intended media folder at `/tmp/tinacms-test/public/uploads`.\n\n### Important Note: HTTP Layer vs Code Vulnerability\n\nI want to be transparent about my findings:\n\n**What I observed:**\n- When testing via HTTP requests against the Vite dev server, path traversal sequences (`../`) are normalized by Node.js/Vite's HTTP layer *before* reaching the vulnerable code\n- This means direct HTTP exploitation like `curl POST /media/upload/../../../tmp/evil.txt` is mitigated in the default configuration\n\n**Why this is still a valid vulnerability that should be fixed:**\n\n1. **The code itself has no validation** - If the path reaches the handler (via any vector), it will be exploited\n2. **Defense-in-depth principle** - Security should not rely solely on HTTP normalization\n3. **Inconsistent protection** - Your GraphQL layer (`addPendingDocument`) explicitly validates paths and rejects `../` (see test at `packages/@tinacms/graphql/tests/pending-document-validation/index.test.ts:59`), but the media endpoints don't have equivalent protection\n4. **Different deployment contexts**:\n - Reverse proxies (nginx, Apache) with `proxy_pass` may preserve raw paths\n - Custom server configurations\n - Future refactoring that uses this code differently\n5. **The `parseMediaFolder` helper** (line 66-74) shows intent to restrict paths - the upload handler should have similar restrictions\n6. **Express version also affected** - `packages/@tinacms/cli/src/server/routes/index.ts` has the same pattern\n\n---\n\n### Evidence That Path Traversal Should Be Blocked\n\nYour codebase already shows that path traversal is considered a security issue:\n\n```typescript\n// From: packages/@tinacms/graphql/tests/pending-document-validation/index.test.ts:52-70\nit('handles validation error for invalid path format', async () => {\n const { query } = await setupMutation(__dirname, config);\n\n const invalidPathMutation = `\n mutation {\n addPendingDocument(\n collection: \"post\"\n relativePath: \"../invalid-path.md\" // <-- Path traversal is rejected!\n ) {\n __typename\n }\n }\n `;\n\n const result = await query({ query: invalidPathMutation, variables: {} });\n\n expect(result.errors).toBeDefined();\n expect(result.errors?.length).toBeGreaterThan(0);\n});\n```\n\nThis test explicitly verifies that `../invalid-path.md` is rejected in the GraphQL layer. The media upload endpoints should have the same protection.\n\n---\n\n## Impact\n\n### Who is Affected\n\n- Developers running TinaCMS in development mode\n- Any deployment exposing the TinaCMS dev server API\n- Particularly concerning if dev servers are exposed to networks (common for mobile testing)\n\n### Potential Attack Scenarios\n\n1. **Remote Code Execution**: Write malicious files to executable locations\n - Overwrite `~/.ssh/authorized_keys` for SSH access\n - Modify application source code\n - Create cron jobs or systemd services\n\n2. **Denial of Service**: Delete critical application or system files\n\n3. **Information Disclosure**: List directory contents outside the media folder\n\n### CVSS Score Estimate\n\n**CVSS 3.1 Base Score: 8.1 (High)**\n- Attack Vector: Network (AV:N)\n- Attack Complexity: Low (AC:L) \n- Privileges Required: None (PR:N)\n- User Interaction: None (UI:N)\n- Scope: Unchanged (S:U)\n- Confidentiality: None (C:N)\n- Integrity: High (I:H)\n- Availability: High (A:H)\n\n---\n\n## Recommended Fix\n\nAdd path validation to ensure the resolved path stays within the media directory:\n\n```typescript\nimport path from 'path';\n\nconst handlePost = async function (req, res) {\n const bb = busboy({ headers: req.headers });\n\n bb.on('file', async (_name, file, _info) => {\n const fullPath = decodeURI(req.url?.slice('/media/upload/'.length));\n const saveTo = path.join(mediaFolder, ...fullPath.split('/'));\n\n // ✅ SECURITY FIX: Validate path stays within media folder\n const resolvedPath = path.resolve(saveTo);\n const resolvedMediaFolder = path.resolve(mediaFolder);\n\n if (!resolvedPath.startsWith(resolvedMediaFolder + path.sep)) {\n res.statusCode = 403;\n res.end(JSON.stringify({ error: 'Invalid file path' }));\n return;\n }\n\n await fs.ensureDir(path.dirname(saveTo));\n file.pipe(fs.createWriteStream(saveTo));\n });\n \n // ... rest of handler\n};\n```\n\nThe same fix should be applied to:\n- `handleDelete` function\n- `handleList` function \n- `MediaModel.listMedia` method\n- `MediaModel.deleteMedia` method\n- Express router in `packages/@tinacms/cli/src/server/`\n\n### Alternative: Create a Validation Helper\n\n```typescript\nfunction validateMediaPath(userPath: string, mediaFolder: string): string {\n const resolved = path.resolve(path.join(mediaFolder, ...userPath.split('/')));\n const resolvedBase = path.resolve(mediaFolder);\n \n if (!resolved.startsWith(resolvedBase + path.sep) && resolved !== resolvedBase) {\n throw new Error('Path traversal detected');\n }\n \n return resolved;\n}\n```\n\n---\n\n## References\n\n- [CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')](https://cwe.mitre.org/data/definitions/22.html)\n- [OWASP Path Traversal](https://owasp.org/www-community/attacks/Path_Traversal)\n- [Node.js path.join() Documentation](https://nodejs.org/api/path.html#pathjoinpaths)\n- [OWASP Testing Guide - Path Traversal](https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/05-Authorization_Testing/01-Testing_Directory_Traversal_File_Include)",
0 commit comments