{
  "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": "The MCPB file upload handler extracts a ZIP file and reads `manifest.json` from it. The `name` field from the manifest is concatenated directly into the file path (line 107) without any sanitization or path traversal character validation. An attacker can craft a malicious MCPB file with `manifest.name` set to `\u0027../../../etc/malicious\u0027` or similar, causing files to be extracted to arbitrary locations on the file system. The `cleanupOldMcpbServer` function (line 110) also uses the unsanitized name, potentially allowing arbitrary directory deletion.\n\n## 1. Summary\n- **Vulnerability Type**: Path Traversal\n- **Flagged Location**: `src/controllers/mcpbController.ts:107`\n- **Vulnerability Description**: The `name` field in the uploaded MCPB manifest is used directly to construct file system paths for directory creation and move operations without sanitization or normalization, potentially leading to a path traversal attack.\n\n## 2. Analysis Logic\n\n### Step 1: Inspect the flagged sink (`src/controllers/mcpbController.ts:106-116`)\nI reviewed the upload handler and located the file system sink where `manifest.name` is used to construct the final extraction path and where files are written 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-controllable and unsanitized, this is a path traversal sink. Next, I traced the source 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 the data flow backward 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 the `manifest.json` inside the uploaded archive. The only check on `manifest.name` is non-emptiness; there is no sanitization, normalization, or whitelist 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-supplied uploads are the source of the manifest content. Next, I confirmed the behavior of the upload middleware.\n\n### Step 4: Confirm the upload middleware (`src/controllers/mcpbController.ts:8-38`)\nI examined how uploaded files are 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 the file extension and size. It does not restrict or validate the contents of the archive or `manifest.name`. Therefore, `manifest.name` is user-controllable input. Next, I checked whether any sanitization or normalization is applied before reaching the sink.\n\n### Step 5: Verify the lack of path validation for `manifest.name` at `src/controllers/mcpbController.ts:92-110`\nI verified that no path sanitization is performed between parsing and use.\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: There is no `path.resolve`/`realpath` check, no use of `basename()`, and no whitelist validation before `manifest.name` is used to construct the file system path. This confirms the path is built from untrusted input with no defenses.\n\n### Step 6: Inspect 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 operation is constrained to directories that already exist within `uploadDir` as returned by `readdirSync`. The primary traversal risk remains in the path construction for `finalExtractDir` and the subsequent file system operations.\n\n### Analysis Process\n- Q1: Does user-controllable input influence the file path? \u2192 **Yes**. `manifest.name` is read from the uploaded archive\u0027s `manifest.json` and used in `path.join(...)` to construct `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 an effective allow-list of valid file 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: **Real Vulnerability** (TP)\n\n## 3. Conclusion\n**Real Vulnerability**\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 the `manifest.json` inside the uploaded archive with only a non-empty check (`src/controllers/mcpbController.ts:89-97`).\n- The `/mcpb/upload` endpoint exposes the upload handler that processes user-supplied archives (`src/routes/index.ts:297-299`).\n\n## 4. Remediation Recommendations\n- Before using `manifest.name` to construct `finalExtractDir`, add normalization and base-directory validation (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 whitelist (letters, digits, `_`, `-`, `.`) before use.\n- Consider rejecting any `manifest.name` containing path separators or traversal sequences, and add unit tests for traversal inputs.\n\n---\n\n*Translated from: Chinese (Simplified) to English using GitHub Copilot*",
  "id": "GHSA-p3h2-2j4p-p83g",
  "modified": "2026-04-22T20:50:19Z",
  "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"
}