{
  "affected": [
    {
      "package": {
        "ecosystem": "npm",
        "name": "@samanhappy/mcphub"
      },
      "ranges": [
        {
          "events": [
            {
              "introduced": "0"
            },
            {
              "fixed": "0.12.13"
            }
          ],
          "type": "ECOSYSTEM"
        }
      ]
    }
  ],
  "aliases": [],
  "database_specific": {
    "cwe_ids": [
      "CWE-22"
    ],
    "github_reviewed": true,
    "github_reviewed_at": "2026-04-22T20:50:19Z",
    "nvd_published_at": null,
    "severity": "HIGH"
  },
  "details": "**MCPB File Upload Handler** extracts a ZIP file and reads `manifest.json` from it. The `name` field in the manifest is directly concatenated into a file path (line 107) without any sanitization or path traversal character validation. An attacker can craft a malicious MCPB file where `manifest.name` is set to something like `../../../etc/malicious`, causing the file to be extracted to an arbitrary location on the file system. The `cleanupOldMcpbServer` function (line 110) also uses the unsanitized name, potentially allowing deletion of arbitrary directories.\n\n## 1. Summary\n- **Vulnerability Type**: Path Traversal (CWE-22)\n- **Sink Location**: src/controllers/mcpbController.ts:107\n- **Vulnerability Description**: The `name` field from an uploaded MCPB manifest is used directly, without sanitization or normalization, to construct a file system path for directory creation and move operations, which may lead to path traversal attacks.\n\n## 2. Analysis Logic\n\n### Step 1: Inspect the identified sink (src/controllers/mcpbController.ts:106-116)\nI examined the upload handler and located the file system sink where `manifest.name` is used to build the final extraction path and write files to that path.\n\n```ts\n// src/controllers/mcpbController.ts:106-116\n// Use server name as the final extract directory for automatic version management\nconst finalExtractDir = path.join(path.dirname(mcpbFilePath), `server-${manifest.name}`);\n\n// Clean up any existing version of this server\ncleanupOldMcpbServer(manifest.name);\nif (!fs.existsSync(finalExtractDir)) {\n  fs.mkdirSync(finalExtractDir, { recursive: true });\n}\n\n// Move the temporary directory to the final location\nfs.renameSync(tempExtractDir, finalExtractDir);\n```\n\nAnalysis: `manifest.name` is used to build `finalExtractDir`, which is then operated on by `fs.mkdirSync` and `fs.renameSync`. These are file system write/move operations, so if `name` is user-controlled and unsanitized, this is a path traversal sink. Next, I traced the origin of `manifest.name`.\n\n### Step 2: Trace the source of `manifest.name` in the upload handler (src/controllers/mcpbController.ts:83-104)\nI traced back the data flow to see how the manifest is read and validated.\n\n```ts\n// src/controllers/mcpbController.ts:83-104\nconst manifestPath = path.join(tempExtractDir, \u0027manifest.json\u0027);\nif (!fs.existsSync(manifestPath)) {\n  throw new Error(\u0027manifest.json not found in MCPB file\u0027);\n}\n\nconst manifestContent = fs.readFileSync(manifestPath, \u0027utf-8\u0027);\nconst manifest = JSON.parse(manifestContent);\n\n// Validate required fields in manifest\nif (!manifest.manifest_version) {\n  throw new Error(\u0027Invalid manifest: missing manifest_version\u0027);\n}\nif (!manifest.name) {\n  throw new Error(\u0027Invalid manifest: missing name\u0027);\n}\n```\n\nAnalysis: `manifest` is parsed directly from `manifest.json` inside the uploaded archive. The only check on `manifest.name` is that it is non\u2011empty; there is no sanitization, normalization, or allow\u2011list validation. Next, I confirmed the entry point for uploading MCPB files to verify user control.\n\n### Step 3: Trace the HTTP entry point in src/routes/index.ts:297-299\nI located the route that exposes the upload handler.\n\n```ts\n// src/routes/index.ts:297-299\n// MCPB upload routes\nrouter.post(\u0027/mcpb/upload\u0027, uploadMiddleware, uploadMcpbFile);\n```\n\nAnalysis: The `/mcpb/upload` endpoint invokes `uploadMiddleware` and `uploadMcpbFile`, so user\u2011supplied uploads are the source of the manifest content. Next, I verified the upload middleware behavior.\n\n### Step 4: Confirm the upload middleware (src/controllers/mcpbController.ts:8-38)\nI inspected how the uploaded file is received and stored.\n\n```ts\n// src/controllers/mcpbController.ts:8-38\nconst storage = multer.diskStorage({\n  destination: (_req, _file, cb) =\u003e {\n    const uploadDir = path.join(process.cwd(), \u0027data/uploads/mcpb\u0027);\n    if (!fs.existsSync(uploadDir)) {\n      fs.mkdirSync(uploadDir, { recursive: true });\n    }\n    cb(null, uploadDir);\n  },\n  filename: (_req, file, cb) =\u003e {\n    const timestamp = Date.now();\n    const originalName = path.parse(file.originalname).name;\n    cb(null, `${originalName}-${timestamp}.mcpb`);\n  },\n});\n\nconst upload = multer({\n  storage,\n  fileFilter: (_req, file, cb) =\u003e {\n    if (file.originalname.endsWith(\u0027.mcpb\u0027)) {\n      cb(null, true);\n    } else {\n      cb(new Error(\u0027Only .mcpb files are allowed\u0027));\n    }\n  },\n  limits: {\n    fileSize: 500 * 1024 * 1024, // 500MB limit\n  },\n});\n\nexport const uploadMiddleware = upload.single(\u0027mcpbFile\u0027);\n```\n\nAnalysis: The upload middleware only checks file extension and size. It does not restrict or validate the contents of the archive or `manifest.name`. Therefore, `manifest.name` is user\u2011controlled input. Next, I checked whether any sanitization or normalization is applied before reaching the sink.\n\n### Step 5: Verify lack of path validation on `manifest.name` in src/controllers/mcpbController.ts:92-110\nI verified that no path sanitization occurs between parsing and usage.\n\n```ts\n// src/controllers/mcpbController.ts:92-110\nif (!manifest.name) {\n  throw new Error(\u0027Invalid manifest: missing name\u0027);\n}\n// ...\nconst finalExtractDir = path.join(path.dirname(mcpbFilePath), `server-${manifest.name}`);\ncleanupOldMcpbServer(manifest.name);\n```\n\nAnalysis: Before using `manifest.name` to construct a file system path, there is no `path.resolve`/`realpath` check, no use of `basename()`, and no allow\u2011list validation. This confirms that the path is built from untrusted input without defenses.\n\n### Step 6: Examine cleanup behavior using the unsanitized name (src/controllers/mcpbController.ts:41-52)\nI verified how `cleanupOldMcpbServer` uses the same input.\n\n```ts\n// src/controllers/mcpbController.ts:41-52\nconst uploadDir = path.join(process.cwd(), \u0027data/uploads/mcpb\u0027);\nconst serverPattern = `server-${serverName}`;\n\nif (fs.existsSync(uploadDir)) {\n  const files = fs.readdirSync(uploadDir);\n  files.forEach((file) =\u003e {\n    if (file.startsWith(serverPattern)) {\n      const filePath = path.join(uploadDir, file);\n      if (fs.statSync(filePath).isDirectory()) {\n        fs.rmSync(filePath, { recursive: true, force: true });\n      }\n    }\n  });\n}\n```\n\nAnalysis: `serverName` is used without validation, but the deletion is limited to directories already present in `uploadDir` as returned by `readdirSync`. The main traversal risk remains in constructing the path for `finalExtractDir` and the subsequent file system operations.\n\n### Analysis Walkthrough\n- Q1: Does user\u2011controllable input affect the file path? \u2192 **Yes**. `manifest.name` is read from the uploaded archive\u2019s `manifest.json` and used in `path.join(...)` to build `finalExtractDir` (src/controllers/mcpbController.ts:89-110).\n- Q2: Is the path normalized and validated against a base directory? \u2192 **No**. There is no `resolve`/`realpath` + `startsWith` check before `fs.mkdirSync`/`fs.renameSync` (src/controllers/mcpbController.ts:106-116).\n- Q3: Is `basename()`/`getName()` used to strip directory components? \u2192 **No**. `manifest.name` is used directly in a template string (src/controllers/mcpbController.ts:106-107).\n- Q4: Is there a valid allow\u2011list for allowed names? \u2192 **No**. Only an existence check is performed on `manifest.name` (src/controllers/mcpbController.ts:92-97).\n- Q5: Is the code in a test/demo/deprecated/generated context? \u2192 **No**. This is a production controller and route (src/controllers/mcpbController.ts:64-130, src/routes/index.ts:297-299).\n- \u2192 Reached leaf node: **True Positive**\n\n## 3. Conclusion\n**True Positive**\n\n**Key evidence:**\n- `manifest.name` flows directly into `finalExtractDir` and is used by `fs.mkdirSync` and `fs.renameSync` without sanitization (src/controllers/mcpbController.ts:106-116).\n- `manifest.name` is parsed from `manifest.json` inside an uploaded archive, with only a non\u2011empty check (src/controllers/mcpbController.ts:89-97).\n- The `/mcpb/upload` endpoint exposes the upload handler that processes user\u2011supplied archives (src/routes/index.ts:297-299).\n\n## 4. Remediation Recommendations\n- Add normalization and base directory validation before using `manifest.name` to construct `finalExtractDir` (e.g., `const resolved = path.resolve(baseDir, `server-${safeName}`); if (!resolved.startsWith(baseDir)) reject;`).\n- Use `path.basename()` to strip directory components from `manifest.name` and enforce a strict character allow\u2011list (alphanumeric, `_`, `-`, `.`) before use.\n- Consider rejecting any `manifest.name` that contains path separators or traversal sequences, and add unit tests for malicious traversal inputs.",
  "id": "GHSA-p3h2-2j4p-p83g",
  "modified": "2026-05-05T21:43:32Z",
  "published": "2026-04-22T20:50:19Z",
  "references": [
    {
      "type": "WEB",
      "url": "https://github.com/samanhappy/mcphub/security/advisories/GHSA-p3h2-2j4p-p83g"
    },
    {
      "type": "WEB",
      "url": "https://github.com/samanhappy/mcphub/commit/af5b013c09bb0add6b7ad9aaa5b875cf150d2a7c"
    },
    {
      "type": "PACKAGE",
      "url": "https://github.com/samanhappy/mcphub"
    }
  ],
  "schema_version": "1.4.0",
  "severity": [
    {
      "score": "CVSS:4.0/AV:N/AC:L/AT:N/PR:L/UI:N/VC:N/VI:H/VA:H/SC:N/SI:N/SA:N",
      "type": "CVSS_V4"
    }
  ],
  "summary": "MCPHub has Path Traversal via Malicious MCPB Manifest Name"
}