The vulnerability is a path traversal issue within the MCPB file upload functionality. The root cause is the failure to sanitize the name field from the manifest.json file inside the uploaded archive. The commit af5b013c09bb0add6b7ad9aaa5b875cf150d2a7c clearly shows the remediation by adding validation and sanitization logic.
The primary vulnerable function is uploadMcpbFile, which orchestrates the process. It reads the untrusted manifest.name and uses it directly to construct file system paths for creating directories and moving files. This allows an attacker to write files outside the intended destination.
Additionally, the cleanupOldMcpbServer function is also vulnerable as it's called by uploadMcpbFile and uses the same unsanitized manifest.name to identify and delete old server directories. This could lead to arbitrary directory deletion.
The patch introduces validateMcpbServerName to sanitize the input and resolveMcpbServerExtractDir to ensure the final path is safe, confirming that these were the points of failure. During an exploit, both uploadMcpbFile and cleanupOldMcpbServer would appear in a runtime profile, operating on the malicious path.
Vulnerable functions
uploadMcpbFile
src/controllers/mcpbController.ts
This function is responsible for handling the MCPB file upload. It reads the `manifest.json` file from the uploaded archive and uses the `name` field to construct the final extraction directory path. The vulnerability lies in the fact that `manifest.name` is used without any sanitization, allowing an attacker to use path traversal characters (e.g., `../`) to cause files to be written to arbitrary locations on the filesystem.
cleanupOldMcpbServer
src/controllers/mcpbController.ts
This function is called by `uploadMcpbFile` to delete old server files before installing a new version. It uses the `serverName` (derived from the unsanitized `manifest.name`) to construct a pattern for finding and deleting directories. A malicious `serverName` with path traversal characters could trick this function into deleting directories outside of the intended `uploads` directory.
GHSA-p3h2-2j4p-p83g: MCPHub has Path Traversal via Malicious MCPB Manifest Name
name
manifest.name
Step 2: Trace the source of manifest.name in the upload handler (src/controllers/mcpbController.ts:83-104)
I traced the data flow backward to see how the manifest is read and validated.
// src/controllers/mcpbController.ts:83-104
const manifestPath = path.join(tempExtractDir, 'manifest.json');
if (!fs.existsSync(manifestPath)) {
throw new Error('manifest.json not found in MCPB file');
}
const manifestContent = fs.readFileSync(manifestPath, 'utf-8');
const manifest = JSON.parse(manifestContent);
// Validate required fields in manifest
if (!manifest.manifest_version) {
throw new Error('Invalid manifest: missing manifest_version');
}
if (!manifest.name) {
throw new Error('Invalid manifest: missing name');
}
Analysis: 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.
Step 3: Trace the HTTP entry point in src/routes/index.ts:297-299
I located the route that exposes the upload handler.
Analysis: 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.
Step 4: Confirm the upload middleware (src/controllers/mcpbController.ts:8-38)
I examined how uploaded files are received and stored.
Analysis: 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.
Step 5: Verify the lack of path validation for manifest.name at src/controllers/mcpbController.ts:92-110
I verified that no path sanitization is performed between parsing and use.
// src/controllers/mcpbController.ts:92-110
if (!manifest.name) {
throw new Error('Invalid manifest: missing name');
}
// ...
const finalExtractDir = path.join(path.dirname(mcpbFilePath), `server-${manifest.name}`);
cleanupOldMcpbServer(manifest.name);
Analysis: 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.
Step 6: Inspect cleanup behavior using the unsanitized name (src/controllers/mcpbController.ts:41-52)
I verified how cleanupOldMcpbServer uses the same input.
Analysis: 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.
Analysis Process
Q1: Does user-controllable input influence the file path? → Yes. manifest.name is read from the uploaded archive's manifest.json and used in path.join(...) to construct finalExtractDir (src/controllers/mcpbController.ts:89-110).
Q2: Is the path normalized and validated against a base directory? → No. There is no resolve/realpath + startsWith check before fs.mkdirSync/fs.renameSync (src/controllers/mcpbController.ts:106-116).
Q3: Is basename()/getName() used to strip directory components? → No. manifest.name is used directly in a template string (src/controllers/mcpbController.ts:106-107).
Q4: Is there an effective allow-list of valid file names? → No. Only an existence check is performed on manifest.name (src/controllers/mcpbController.ts:92-97).
Q5: Is the code in a test/demo/deprecated/generated context? → No. This is a production controller and route (src/controllers/mcpbController.ts:64-130, src/routes/index.ts:297-299).
→ Reached leaf node: Real Vulnerability (TP)
3. Conclusion
Real Vulnerability
Key Evidence:
manifest.name flows directly into finalExtractDir and is used by fs.mkdirSync and fs.renameSync without sanitization (src/controllers/mcpbController.ts:106-116).
manifest.name is parsed from the manifest.json inside the uploaded archive with only a non-empty check (src/controllers/mcpbController.ts:89-97).
The /mcpb/upload endpoint exposes the upload handler that processes user-supplied archives (src/routes/index.ts:297-299).
4. Remediation Recommendations
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;).
Use path.basename() to strip directory components from manifest.name, and enforce a strict character whitelist (letters, digits, _, -, .) before use.
Consider rejecting any manifest.name containing path separators or traversal sequences, and add unit tests for traversal inputs.
Translated from: Chinese (Simplified) to English using GitHub Copilot
Miggo AI