Making mcp remote #35
Conversation
![semgrep-code-vantainc[bot]](https://avatars.githubusercontent.com/in/363648?s=60&v=4){kind=small}
| const transport = new StreamableHTTPServerTransport({ | ||
| sessionIdGenerator: () => randomUUID(), | ||
| }); |
There was a problem hiding this comment.
High severity vulnerability may affect your project—review required:
Line 149 lists a dependency (@modelcontextprotocol/sdk) with a known High severity vulnerability.
ℹ️ Why this matters
Affected versions of @modelcontextprotocol/sdk are vulnerable to Insecure Default Initialization of Resource / Reliance on Reverse DNS Resolution for a Security-Critical Action. Unauthenticated, HTTP‐based MCP servers running on localhost do not enable DNS rebinding protection by default. A malicious website can exploit DNS rebinding to bypass same‐origin policy and send requests over StreamableHTTPServerTransport or SSEServerTransport to the local MCP server, potentially invoking exposed tools or accessing resources on the user's machine.
To resolve this comment:
Check if you run an unauthenticated HTTP-based MCP server on localhost.
- If you're affected, upgrade this dependency to at least version 1.24.0 at package-lock.json.
- If you're not affected, comment
/fp we don't use this [condition]
💬 Ignore this finding
To ignore this, reply with:
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
You can view more details on this finding in the Semgrep AppSec Platform here.
|
|
||
| if (sessionId && sessions.has(sessionId)) { | ||
| // Existing session - reuse transport | ||
| console.log(`Reusing existing session: ${sessionId}`); |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
User-controlled sessionId from request headers is logged directly without sanitization, allowing log injection attacks through newline characters and other special characters.
More details about this
The sessionId from the request header (req.headers["mcp-session-id"]) is directly interpolated into a console.log statement without any sanitization. An attacker can inject log-forging characters by sending a crafted mcp-session-id header.
Exploit scenario:
- Attacker sends an HTTP request with header:
mcp-session-id: test\nERROR: Database connection failed - The
sessionIdvariable receives this string as-is - When
console.log(\Reusing existing session: ${sessionId}`)` executes, it outputs:Reusing existing session: test ERROR: Database connection failed - This forged log line appears in application logs, making it look like a genuine error occurred. An attacker can use this to create confusion, cover tracks, or trick log monitoring/alerting systems into false alerts.
Since sessionId comes directly from user-controlled request headers and flows directly into the logger without validation or encoding, an attacker can inject newlines and other characters to create misleading log entries.
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>src/server/http.ts</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/VantaInc/vanta-mcp-server/blob/eda403e3ab40eac9aeb19223a0651a3929d59f7f/src/server/http.ts#L133 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 133] req</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/VantaInc/vanta-mcp-server/blob/eda403e3ab40eac9aeb19223a0651a3929d59f7f/src/server/http.ts#L133 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 133] sessionId</a>"]
v3["<a href=https://github.com/VantaInc/vanta-mcp-server/blob/eda403e3ab40eac9aeb19223a0651a3929d59f7f/src/server/http.ts#L138 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 138] `</a>"]
end
v2 --> v3
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/VantaInc/vanta-mcp-server/blob/eda403e3ab40eac9aeb19223a0651a3929d59f7f/src/server/http.ts#L138 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 138] console.log(`Reusing existing session: ${sessionId}`)</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
✨ Commit Assistant Fix Suggestion
- Do not log the raw
sessionIdvalue directly inconsole.log. Instead, neutralize any potentially dangerous characters that could allow log injection. - Sanitize the
sessionIdbefore logging by allowing only safe characters. For example, restrict to alphanumeric and a few known-safe symbols:const safeSessionId = sessionId?.replace(/[^a-zA-Z0-9_\-]/g, "");. - Update the logging line to use the sanitized value:
console.log(Reusing existing session: ${safeSessionId});.
Alternatively, if you only need to confirm the session's presence, log a boolean or a truncated/hashed value rather than the raw value.
For example: console.log("Reusing existing session: %s", !!sessionId);.
Replacing or sanitizing the session ID ensures an attacker cannot inject line breaks or control characters that could forge log entries or manipulate log analysis tools.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by console-log-express.
You can view more details about this finding in the Semgrep AppSec Platform.
|
|
||
| // Check for existing session | ||
| const sessionId = req.headers["mcp-session-id"] as string | undefined; | ||
| console.log(`Session ID from header: ${sessionId}`); |
There was a problem hiding this comment.
Semgrep identified an issue, but thinks it may be safe to ignore.
Detected a logger that logs user input without properly neutralizing the output. The log message could contain characters like and and cause an attacker to forge log entries or include malicious content into the logs. Use proper input validation and/or output encoding to prevent log entries from being forged.
Why this might be safe to ignore:
This is logging a session ID header value for debugging purposes. Session IDs are not user-controllable content that could contain malicious injection characters - they are either system-generated UUIDs or not present. The log forging risk is minimal since session IDs follow a predictable format and don't contain newlines or control characters that would enable log injection attacks.
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>src/server/http.ts</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/VantaInc/vanta-mcp-server/blob/eda403e3ab40eac9aeb19223a0651a3929d59f7f/src/server/http.ts#L133 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 133] req</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/VantaInc/vanta-mcp-server/blob/eda403e3ab40eac9aeb19223a0651a3929d59f7f/src/server/http.ts#L133 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 133] sessionId</a>"]
v3["<a href=https://github.com/VantaInc/vanta-mcp-server/blob/eda403e3ab40eac9aeb19223a0651a3929d59f7f/src/server/http.ts#L134 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 134] `</a>"]
end
v2 --> v3
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/VantaInc/vanta-mcp-server/blob/eda403e3ab40eac9aeb19223a0651a3929d59f7f/src/server/http.ts#L134 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 134] console.log(`Session ID from header: ${sessionId}`)</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by console-log-express.
You can view more details about this finding in the Semgrep AppSec Platform.
| const response = await fetch(VANTA_TOKEN_ENDPOINT, { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/x-www-form-urlencoded", | ||
| }, | ||
| body: new URLSearchParams(bodyWithExtras).toString(), | ||
| }); |
There was a problem hiding this comment.
Semgrep identified an issue, but thinks it may be safe to ignore.
Prefer to use RequestFetcher's fetch function (#server-common/utils/requestFetcher.ts) instead of another library's fetch function. This is because RequestFetcher's fetch function has some built-in SSRF prevention mechanisms that other third-party fetch functions do not.
Why this might be safe to ignore:
This is an MCP server making an authorized request to Vanta's OAuth token endpoint with a hardcoded URL (VANTA_TOKEN_ENDPOINT). SSRF prevention is not needed for internal OAuth flows with predefined endpoints, and using RequestFetcher would be inappropriate in this authentication context.
To resolve this comment:
🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by use-request-fetcher.
You can view more details about this finding in the Semgrep AppSec Platform.
| } | ||
|
|
||
| return response; | ||
| return fetch(url, requestOptions); |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
Prefer to use RequestFetcher's fetch function (#server-common/utils/requestFetcher.ts) instead of another library's fetch function. This is because RequestFetcher's fetch function has some built-in SSRF prevention mechanisms that other third-party fetch functions do not.
To resolve this comment:
✨ Commit Assistant Fix Suggestion
- Remove any usage of the
fetchfunction (or any other third-party HTTP request libraries) in this module. - Import the
RequestFetcherutility from#server-common/utils/requestFetcher.tsif it's not already imported:import { RequestFetcher } from "#server-common/utils/requestFetcher";. - Create an instance of
RequestFetcherat the top of the file or where appropriate:const fetcher = new RequestFetcher(); - Replace your calls to
fetch(url, { ... })withfetcher.fetch(url, { ... }), preserving the same parameters and options. - If
fetcher.fetchexpects parameters in a different shape than the standard fetch API, update the options accordingly to matchRequestFetcher's interface.
Alternatively, if you need to handle JSON responses and RequestFetcher provides a fetchJSON method, use fetcher.fetchJSON(url, {}, options) instead.
RequestFetcher automatically includes protections to help prevent Server-Side Request Forgery (SSRF) attacks.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by use-request-fetcher.
You can view more details about this finding in the Semgrep AppSec Platform.
|
|
||
| export function createHttpServer(config: ServerConfig) { | ||
| const app = express(); | ||
| app.use(cors()); |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
Default CORS settings allow any website to make requests to your application endpoints, enabling cross-site attacks to steal data or make unauthorized requests on behalf of users.
More details about this
The application is calling cors() with no configuration, which applies default CORS settings that allow requests from any origin (wildcard *).
Here's how an attacker could exploit this:
- Attacker creates a malicious website (e.g.,
evil.com) and serves JavaScript code that makes requests to your application - A user visits
evil.comwhile logged into your application - The JavaScript on
evil.comsends requests to your application's endpoints (e.g.,GET /.well-known/oauth-authorization-serveror other sensitive endpoints) - Because
cors()has no origin restrictions, the browser allows these cross-origin requests and returns the response toevil.com's JavaScript - The attacker's malicious JavaScript can now read sensitive data like OAuth configuration, authorization server metadata, or any other response data that would normally be protected by the browser's same-origin policy
- If your application's authenticated endpoints are also unprotected by CORS restrictions, the attacker could make authenticated requests on behalf of the logged-in user (since cookies are sent with cross-origin requests by default)
The vulnerable code is at line app.use(cors()) — without arguments, it permits all origins to access all endpoints on your server.
To resolve this comment:
✨ Commit Assistant Fix Suggestion
- Replace
app.use(cors());with a more restrictive CORS configuration that explicitly allows only trusted origins. - Define a list of allowed origins, for example:
const allowedOrigins = ["https://example.com", "http://localhost:3000"]; - Update the cors middleware usage to use this whitelist:
app.use(cors({ origin: allowedOrigins })); - Optionally, if you want the flexibility to allow credentials or further restrict allowed methods, pass additional options to the cors middleware, such as
credentials: trueormethods: ["GET", "POST"]. - Avoid using the wildcard
*for the origin value in production to prevent unauthorized cross-origin access.
The cors middleware accepts an array of origins, so specifying trusted domains in an array helps restrict which sites can access your API.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by cors-default-config-express.
You can view more details about this finding in the Semgrep AppSec Platform.
| const response = await fetch(VANTA_TOKEN_ENDPOINT, { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/x-www-form-urlencoded", | ||
| }, | ||
| body: new URLSearchParams(bodyWithExtras).toString(), | ||
| }); |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
OAuth token endpoint request sent over unencrypted HTTP instead of HTTPS, exposing credentials and tokens to network eavesdropping attacks.
More details about this
The fetch() call is sending a POST request to VANTA_TOKEN_ENDPOINT over HTTP instead of HTTPS, exposing sensitive OAuth token data in transit.
Exploit scenario:
- An attacker positioned on the network (e.g., same WiFi, compromised router, or ISP-level access) intercepts the unencrypted HTTP traffic from your server to Vanta's token endpoint
- The attacker captures the POST request body which contains
bodyWithExtras—this includes credentials and tokens fromreq.body(likely containing user credentials or refresh tokens sent by OAuth clients) - The attacker extracts the
source_id, authentication parameters, and any other sensitive data from the request - The attacker can now impersonate your application to Vanta's token endpoint or replay these credentials to obtain valid tokens for any user
- With forged tokens, the attacker gains unauthorized access to whatever resources your application protects
The lack of encryption on this network hop means any plaintext credentials, tokens, or secrets traveling through bodyWithExtras to Vanta are visible to network-level attackers.
To resolve this comment:
✨ Commit Assistant Fix Suggestion
- Check the value of
VANTA_TOKEN_ENDPOINTto ensure it uses HTTPS instead of HTTP. For example, update the value to start withhttps://instead ofhttp://. - If
VANTA_TOKEN_ENDPOINTis defined in a configuration file or environment variable, update its value there as well to use HTTPS. - If the upstream service does not support HTTPS, contact the service owner to request HTTPS support. Do not send sensitive information (such as tokens, passwords, or authorization codes) over HTTP.
Using HTTPS ensures that request data, including sensitive credentials, are encrypted in transit and protected from eavesdropping.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by react-insecure-request.
You can view more details about this finding in the Semgrep AppSec Platform.
1 participant
No description provided.