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 '../../../etc/malicious' 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.
1. Summary
- Vulnerability Type: Path Traversal
- Flagged Location:
src/controllers/mcpbController.ts:107
- 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.
2. Analysis Logic
Step 1: Inspect the flagged sink (src/controllers/mcpbController.ts:106-116)
I 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.
// src/controllers/mcpbController.ts:106-116
// Use server name as the final extract directory for automatic version management
const finalExtractDir = path.join(path.dirname(mcpbFilePath), `server-${manifest.name}`);
// Clean up any existing version of this server
cleanupOldMcpbServer(manifest.name);
if (!fs.existsSync(finalExtractDir)) {
fs.mkdirSync(finalExtractDir, { recursive: true });
}
// Move the temporary directory to the final location
fs.renameSync(tempExtractDir, finalExtractDir);
Analysis: 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.
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.
// src/routes/index.ts:297-299
// MCPB upload routes
router.post('/mcpb/upload', uploadMiddleware, uploadMcpbFile);
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.
// src/controllers/mcpbController.ts:8-38
const storage = multer.diskStorage({
destination: (_req, _file, cb) => {
const uploadDir = path.join(process.cwd(), 'data/uploads/mcpb');
if (!fs.existsSync(uploadDir)) {
fs.mkdirSync(uploadDir, { recursive: true });
}
cb(null, uploadDir);
},
filename: (_req, file, cb) => {
const timestamp = Date.now();
const originalName = path.parse(file.originalname).name;
cb(null, `${originalName}-${timestamp}.mcpb`);
},
});
const upload = multer({
storage,
fileFilter: (_req, file, cb) => {
if (file.originalname.endsWith('.mcpb')) {
cb(null, true);
} else {
cb(new Error('Only .mcpb files are allowed'));
}
},
limits: {
fileSize: 500 * 1024 * 1024, // 500MB limit
},
});
export const uploadMiddleware = upload.single('mcpbFile');
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.
// src/controllers/mcpbController.ts:41-52
const uploadDir = path.join(process.cwd(), 'data/uploads/mcpb');
const serverPattern = `server-${serverName}`;
if (fs.existsSync(uploadDir)) {
const files = fs.readdirSync(uploadDir);
files.forEach((file) => {
if (file.startsWith(serverPattern)) {
const filePath = path.join(uploadDir, file);
if (fs.statSync(filePath).isDirectory()) {
fs.rmSync(filePath, { recursive: true, force: true });
}
}
});
}
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