MCPJam / MCPJam/inspector

[BUG] isBlockedEgressHost allows empty hostnames

Open
#4,058 2 comments 0 reactions 1 assignee Claimed by @code-withkrishna View on GitHub
bug
Dominant language
TypeScript
Stars
2.2k
Forks
283
Avg merge
11h 47m
Merged PRs (30d)
737

Description

### Description

````markdown
## Issue

While reviewing the hosted egress/SSRF guard, I noticed that `isBlockedEgressHost()` treats an empty hostname as allowed.

In `server/utils/hosted-egress-guard.ts`, after trimming the hostname and removing trailing dots, the function contains:

```ts
host = host.replace(/\.+$/, "");
if (!host) return false;
````

This means:

```ts
isBlockedEgressHost("", true)
```

returns `false`.

I reproduced this behavior locally against the current `main` branch.

I don't believe this is currently a direct SSRF bypass because `assertAllowedHostedTargetUrl()` validates the URL with `new URL()` before calling the egress guard. However, the behavior appears inconsistent with the guard's fail-closed security model.

## How to reproduce

1. Open `server/utils/hosted-egress-guard.ts`.
2. Locate the `isBlockedEgressHost()` function.
3. Pass an empty hostname:

```ts
isBlockedEgressHost("", true)
```

4. Observe that it returns:

```text
false
```

The same behavior occurs with a whitespace-only hostname because the value is trimmed first:

```ts
isBlockedEgressHost(" ", true)
```

## Expected Behavior

An empty or whitespace-only hostname should fail closed and be treated as blocked:

```ts
isBlockedEgressHost("", true) === true
isBlockedEgressHost(" ", true) === true
```

This would be consistent with the existing fail-closed behavior for invalid or unresolvable egress targets.

## Proposed Fix

Change:

```ts
if (!host) return false;
```

to:

```ts
if (!host) return true;
```

and add regression tests covering empty and whitespace-only hostnames.

## Impact

This appears to be a defense-in-depth/correctness issue rather than a currently exploitable SSRF vulnerability in the normal URL-validation flow.

I have not found an existing issue or PR covering this exact case.

Would this be a change the maintainers would be open to? If so, I'd be happy to work on it.

```

### Don't add anything else

You **don't need a screenshot**, because this is a code-level issue.

And importantly, don't describe it as a **"Critical SSRF vulnerability"**. The wording above is deliberately accurate and technically defensible.

Once you click **Create**, send me the issue number. Then we'll wait for/handle the maintainer response before making the change.
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.