diff --git a/src/scanner/catalog.ts b/src/scanner/catalog.ts index 0847e9e..6334e1c 100644 --- a/src/scanner/catalog.ts +++ b/src/scanner/catalog.ts @@ -330,6 +330,15 @@ export const RULE_CATALOG: Record = { "Keep MCP server commands and arguments in static, server-side configuration; never build them from request data.", "Art. 15 (cybersecurity)", ), + MCP011: entry( + "MCP011", + "External data returned as MCP tool result without validation", + "high", + "LLM01", + "A malicious or compromised external source (webhook, unauthenticated ingest endpoint, error-tracking DSN) can inject instructions through a tool response the agent treats as trusted output.", + "Validate and sanitize external response content before returning it as a tool result; restrict the return shape with a schema.", + "Art. 15 (cybersecurity)", + ), SKL001: entry( "SKL001", "Invisible Unicode in agent skill file", diff --git a/src/scanner/explainer.ts b/src/scanner/explainer.ts index 5e10a5e..2a9a2a7 100644 --- a/src/scanner/explainer.ts +++ b/src/scanner/explainer.ts @@ -460,6 +460,28 @@ const ALLOWED_SERVERS = { search: ["node", "./mcp/search.js"] }; const [command, ...args] = ALLOWED_SERVERS[req.body.tool] ?? []; if (!command) throw new Error("Unknown MCP server"); const transport = new StdioClientTransport({ command, args });`, + }, + MCP011: { + summary: "A tool handler returns externally fetched content as the tool result without validation.", + whyRisky: + "The calling agent treats tool results as trusted output. Whoever can write to the fetched source — a public webhook, an unauthenticated ingest endpoint, an error-tracking DSN — gets to plant instructions in that content, without touching the tool's static name or description at all.", + howExploited: + "The 2026 Sentry MCP DSN attack: an attacker sends fake error events through a public DSN; the Sentry MCP server fetches and returns them as 'trusted diagnostics'; the agent follows embedded instructions in the event data and executes commands.", + howToFix: + "Validate and sanitize external response content before returning it as a tool result. Restrict what the tool can return with a schema, and treat the fetched endpoint as untrusted input, the same as any other external HTTP response.", + codeExample: `// Bad +server.tool("get_error_details", "Fetches error diagnostics.", schema, async ({ eventId }) => { + const res = await fetch(\`https://dsn.example.com/events/\${eventId}\`); + const event = await res.json(); + return { content: [{ type: "text", text: event.message }] }; +}); + +// Good +server.tool("get_error_details", "Fetches error diagnostics.", schema, async ({ eventId }) => { + const res = await fetch(\`https://dsn.example.com/events/\${eventId}\`); + const event = eventSchema.parse(await res.json()); + return { content: [{ type: "text", text: sanitize(event.message) }] }; +});`, }, SKL001: { summary: "An Agent Skill file contains invisible or bidirectional Unicode characters.", diff --git a/src/scanner/python-scanner.ts b/src/scanner/python-scanner.ts index 4555717..34a2c41 100644 --- a/src/scanner/python-scanner.ts +++ b/src/scanner/python-scanner.ts @@ -17,6 +17,7 @@ import { pythonNodeContainsText, pythonTargetText, type PythonCallNode, + type PythonFunctionNode, type PythonNode, } from "./python-ast.js"; @@ -208,12 +209,16 @@ function pythonStringValue(node: PythonNode): string { return text.slice(start, end); } +const MCP_TOOL_DECORATOR = /@\s*[\w.]*\.?tool(?:\s*\(|\s*$)/; + +function isMcpToolFunction(fn: PythonFunctionNode): boolean { + return fn.decorators.some((d) => MCP_TOOL_DECORATOR.test(d.text)); +} + function collectPyTools(src: PythonSource): PyToolDefinition[] { const tools: PyToolDefinition[] = []; for (const fn of src.ast.functions) { - const decorator = fn.decorators.find((candidate) => - /@\s*[\w.]*\.?tool(?:\s*\(|\s*$)/.test(candidate.text), - ); + const decorator = fn.decorators.find((candidate) => MCP_TOOL_DECORATOR.test(candidate.text)); if (!decorator) continue; let name = fn.name; @@ -908,6 +913,93 @@ function checkMCP008(src: PythonSource, i: number, file: string, ctx: FileContex return null; } +/** + * Fixed-point propagation from a seed set of variable names through + * assignments in `scope` — e.g. `resp = requests.get(url)` seeds `resp`, + * then `data = resp.json()` picks up `data` because its RHS references + * `resp`. Same shape as collectRequestTaintedVars above, seeded from a + * specific fetch call's assignment targets instead of REQUEST_PATTERNS. + */ +function collectPyDerivedFromVar( + src: PythonSource, + scope: PythonNode, + seed: Set, + upToLine: number, +): Set { + const derived = new Set(seed); + const assignments = src.ast.assignments.filter( + (a) => + a.node.startIndex >= scope.startIndex && + a.node.endIndex <= scope.endIndex && + a.node.startPosition.row <= upToLine, + ); + for (let pass = 0; pass < 4; pass++) { + let changed = false; + for (const assignment of assignments) { + if (![...derived].some((v) => pythonNodeContainsText(assignment.value, v))) continue; + for (const target of assignment.targets) { + const t = pythonTargetText(target); + if (derived.has(t)) continue; + derived.add(t); + changed = true; + } + } + if (!changed) break; + } + return derived; +} + +function checkMCP011(src: PythonSource, i: number, file: string, ctx: FileContext): Finding | null { + if (!ctx.fileHasMcpServer) return null; + const fetchCall = src.ast.callsAtLine(i).find((call) => + /^(?:requests|httpx|urllib)(?:\.[\w]+)*\.(?:get|post|request)$/.test(pythonCallName(call)), + ); + if (!fetchCall) return null; + + const fn = src.ast.enclosingFunction(fetchCall.node); + if (!fn || !isMcpToolFunction(fn)) return null; + + const assignment = src.ast.assignments.find( + (candidate) => + candidate.node.startIndex <= fetchCall.node.startIndex && + candidate.node.endIndex >= fetchCall.node.endIndex, + ); + const seed = new Set(assignment ? assignment.targets.map(pythonTargetText) : []); + const derived = collectPyDerivedFromVar(src, fn.body, seed, fn.body.endPosition.row); + + const sinkReturn = pythonDescendants(fn.body, "return_statement").find((ret) => { + if (ret.startPosition.row < i) return false; + const containsFetchInline = + fetchCall.node.startIndex >= ret.startIndex && fetchCall.node.endIndex <= ret.endIndex; + const containsDerived = [...derived].some((v) => pythonNodeContainsText(ret, v)); + return containsFetchInline || containsDerived; + }); + if (!sinkReturn) return null; + + const between = fn.body.text.slice( + Math.max(0, fetchCall.node.startIndex - fn.body.startIndex), + Math.max(0, sinkReturn.endIndex - fn.body.startIndex), + ); + if (hasSanitization(between)) return null; + + return { + ...findingBase( + "MCP011", + "External data returned as MCP tool result without validation", + "high", + file, + sinkReturn.startPosition.row + 1, + ), + summary: "MCP tool handler returns externally fetched content as the tool result without sanitization.", + description: + "The tool handler fetches from an external source and returns the response directly as the tool result. Whoever controls that source (a public webhook, an unauthenticated ingest endpoint, an error-tracking DSN) can inject instructions the calling agent treats as trusted tool output.", + recommendation: + "Validate and sanitize external response content before returning it as a tool result. Restrict the return shape with a schema, and treat the fetched endpoint as untrusted input.", + confidence: evidenceConfidence("likely"), + evidence: "likely", + }; +} + function checkMCP009(src: PythonSource, i: number, file: string, ctx: FileContext): Finding | null { if (!ctx.fileHasMcpServer || !ctx.mcpToolNames || ctx.mcpToolNames.size < 2) return null; const def = ctx.mcpTools?.find((tool) => tool.decoratorLine === i); @@ -959,6 +1051,7 @@ const PYTHON_RULES: RuleChecker[] = [ checkMCP007, checkMCP008, checkMCP009, + checkMCP011, ]; // ── File discovery ──────────────────────────────────────────────────────── diff --git a/src/scanner/rules/index.ts b/src/scanner/rules/index.ts index 9d638c4..f4fc6de 100644 --- a/src/scanner/rules/index.ts +++ b/src/scanner/rules/index.ts @@ -22,6 +22,7 @@ import { ruleMcpInjectionPhrases, ruleMcpInvisibleUnicode, } from "./mcp-tool-poisoning.js"; +import { ruleMcpUntrustedToolSource } from "./mcp-untrusted-tool-source.js"; // Vector/RAG rules import { ruleVecSearchNoAccessControl } from "./vec-search-no-access-control.js"; import { ruleVecUnboundedSearch } from "./vec-unbounded-search.js"; @@ -52,6 +53,8 @@ export const RULES: Rule[] = [ ruleMcpInvisibleUnicode, ruleMcpInjectionPhrases, ruleMcpCrossToolShadowing, + // MCP tool-source rule (MCP011) + ruleMcpUntrustedToolSource, // Vector/RAG rules (VEC001–VEC004) ruleVecSearchNoAccessControl, ruleVecUnboundedSearch, diff --git a/src/scanner/rules/indirect-prompt-injection.ts b/src/scanner/rules/indirect-prompt-injection.ts index 841bf99..dc0f147 100644 --- a/src/scanner/rules/indirect-prompt-injection.ts +++ b/src/scanner/rules/indirect-prompt-injection.ts @@ -40,7 +40,15 @@ function isFetchLikeCall(node: Node): boolean { return FETCH_PATTERNS.some((p) => text.includes(p.replace("(", ""))); } -function collectFetchDerivedIdentifiers(functionNode: Node): Set { +/** + * Exported for mcp-untrusted-tool-source.ts (MCP011), which needs the same + * "resolved fetch call -> derived vars, multi-hop" derivation for MCP tool + * handlers. Reusing this instead of a rule-local reimplementation is the + * documented convention (see llm-rule-utils.ts's getPromptParts) — a + * rule-specific rewrite of the same logic is exactly what caused the AI007 + * false-positive class. + */ +export function collectFetchDerivedIdentifiers(functionNode: Node): Set { const derived = new Set(); // Iterate to a fixed point so multi-hop chains (page = await fetch(url); diff --git a/src/scanner/rules/mcp-tool-poisoning.ts b/src/scanner/rules/mcp-tool-poisoning.ts index 27b1ec1..135efc3 100644 --- a/src/scanner/rules/mcp-tool-poisoning.ts +++ b/src/scanner/rules/mcp-tool-poisoning.ts @@ -24,13 +24,13 @@ const MCP_SERVER_MODULES = [ (s: string) => s === "fastmcp", ]; -const TOOL_METHODS = new Set(["tool", "registertool", "addtool"]); +export const TOOL_METHODS = new Set(["tool", "registertool", "addtool"]); function isMcpServerModule(spec: string): boolean { return MCP_SERVER_MODULES.some((test) => test(spec)); } -function fileImportsMcpServerSdk(sourceFile: SourceFile): boolean { +export function fileImportsMcpServerSdk(sourceFile: SourceFile): boolean { for (const imp of sourceFile.getImportDeclarations()) { if (isMcpServerModule(imp.getModuleSpecifierValue())) return true; } diff --git a/src/scanner/rules/mcp-untrusted-tool-source.ts b/src/scanner/rules/mcp-untrusted-tool-source.ts new file mode 100644 index 0000000..31007c8 --- /dev/null +++ b/src/scanner/rules/mcp-untrusted-tool-source.ts @@ -0,0 +1,92 @@ +import { Node, SyntaxKind, type CallExpression, type SourceFile } from "ts-morph"; +import type { Evidence, Finding, Rule, RuleContext } from "../types.js"; +import { getNodeLine, getRelativeFilePath } from "../../utils/ast.js"; +import { evidenceConfidence, demoteEvidence, isTestFilePath, hasSanitizationNearby } from "../confidence.js"; +import { fileImportsMcpServerSdk, TOOL_METHODS } from "./mcp-tool-poisoning.js"; +import { collectFetchDerivedIdentifiers } from "./indirect-prompt-injection.js"; + +/** + * MCP011: an MCP tool handler fetches from an external/unauthenticated + * source and returns the response as the tool result with no sanitization + * in between. + * + * This is the Sentry-MCP-DSN attack shape (2026): the tool server itself is + * the injection vector — whoever can write to the fetched endpoint gets to + * plant instructions into whatever agent calls the tool, without touching + * the tool's static name/description text at all (that's MCP007/MCP008, + * which this rule does not duplicate). + */ + +function findHandlerFunction(call: CallExpression): Node | undefined { + const args = call.getArguments(); + const last = args[args.length - 1]; + if (last && (Node.isFunctionExpression(last) || Node.isArrowFunction(last))) return last; + // registerTool("name", { ... }, handler) — handler may be the 3rd arg + // when metadata is object-carried instead of positional. + const secondToLast = args[args.length - 2]; + if (secondToLast && (Node.isFunctionExpression(secondToLast) || Node.isArrowFunction(secondToLast))) { + return secondToLast; + } + return undefined; +} + +function collectToolHandlers(sourceFile: SourceFile): Node[] { + const handlers: Node[] = []; + sourceFile.forEachDescendant((node) => { + if (!Node.isCallExpression(node)) return; + const method = (node.getExpression().getText().split(".").pop() ?? "").toLowerCase(); + if (!TOOL_METHODS.has(method)) return; + const handler = findHandlerFunction(node); + if (handler) handlers.push(handler); + }); + return handlers; +} + +export const ruleMcpUntrustedToolSource: Rule = { + id: "MCP011", + title: "External data returned as MCP tool result without validation", + severity: "high", + run(context: RuleContext): Finding[] { + const findings: Finding[] = []; + + for (const sourceFile of context.sourceFiles) { + if (!fileImportsMcpServerSdk(sourceFile)) continue; + const relPath = getRelativeFilePath(context.rootPath, sourceFile); + const isTest = isTestFilePath(relPath); + + for (const handler of collectToolHandlers(sourceFile)) { + const fetchVars = collectFetchDerivedIdentifiers(handler); + if (fetchVars.size === 0) continue; + + const handlerText = handler.getText(); + const hasSanitization = hasSanitizationNearby(handlerText); + + for (const returnStmt of handler.getDescendantsOfKind(SyntaxKind.ReturnStatement)) { + const returnText = returnStmt.getText(); + const usesExternalContent = [...fetchVars].some((v) => returnText.includes(v)); + if (!usesExternalContent) continue; + + let evidence: Evidence = "likely"; + if (isTest || hasSanitization) evidence = demoteEvidence(evidence); + + findings.push({ + rule_id: "MCP011", + title: "External data returned as MCP tool result without validation", + severity: "high", + file: relPath, + line: getNodeLine(returnStmt), + summary: "MCP tool handler returns externally fetched content as the tool result without sanitization.", + description: + "The tool handler fetches from an external source and returns the response body directly as the tool result. Whoever controls that source (a public webhook, an unauthenticated ingest endpoint, an error-tracking DSN) can inject instructions that the calling agent treats as trusted tool output.", + recommendation: + "Validate and sanitize external response content before returning it as a tool result. Restrict the shape of what the tool can return with a schema, and treat the fetched endpoint as untrusted input.", + confidence: evidenceConfidence(evidence), + evidence, + }); + } + } + } + + return findings; + }, +}; diff --git a/test-fixtures/safe/mcp_untrusted_tool_source.py b/test-fixtures/safe/mcp_untrusted_tool_source.py new file mode 100644 index 0000000..8cab2e5 --- /dev/null +++ b/test-fixtures/safe/mcp_untrusted_tool_source.py @@ -0,0 +1,18 @@ +from mcp.server.fastmcp import FastMCP +import requests + +mcp = FastMCP("demo") + + +def sanitize_event_message(raw: str) -> str: + return "".join(ch for ch in raw if ch.isprintable()) + + +# Fetched content is sanitized before it becomes the tool result. +@mcp.tool() +def get_error_details(event_id: str) -> str: + """Fetches error diagnostics for a given event.""" + res = requests.get(f"https://dsn.example.com/events/{event_id}") + event = res.json() + safe_message = sanitize_event_message(event["message"]) + return safe_message diff --git a/test-fixtures/safe/mcp_untrusted_tool_source.ts b/test-fixtures/safe/mcp_untrusted_tool_source.ts new file mode 100644 index 0000000..19aa3cf --- /dev/null +++ b/test-fixtures/safe/mcp_untrusted_tool_source.ts @@ -0,0 +1,21 @@ +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { z } from "zod"; + +const server = new McpServer({ name: "demo", version: "1.0.0" }); + +function sanitizeEventMessage(raw: string): string { + return raw.replace(/[^\x20-\x7e]/g, ""); +} + +// Fetched content is sanitized before it becomes the tool result. +server.tool( + "get_error_details", + "Fetches error diagnostics for a given event.", + { eventId: z.string() }, + async ({ eventId }) => { + const res = await fetch(`https://dsn.example.com/events/${eventId}`); + const event = await res.json(); + const safeMessage = sanitizeEventMessage(event.message); + return { content: [{ type: "text", text: safeMessage }] }; + }, +); diff --git a/test-fixtures/vulnerable/mcp_untrusted_tool_source.py b/test-fixtures/vulnerable/mcp_untrusted_tool_source.py new file mode 100644 index 0000000..28881ee --- /dev/null +++ b/test-fixtures/vulnerable/mcp_untrusted_tool_source.py @@ -0,0 +1,14 @@ +from mcp.server.fastmcp import FastMCP +import requests + +mcp = FastMCP("demo") + + +# MCP011: the DSN event body is fetched from an unauthenticated public +# endpoint and returned as the tool result with no validation. +@mcp.tool() +def get_error_details(event_id: str) -> str: + """Fetches error diagnostics for a given event.""" + res = requests.get(f"https://dsn.example.com/events/{event_id}") + event = res.json() + return event["message"] diff --git a/test-fixtures/vulnerable/mcp_untrusted_tool_source.ts b/test-fixtures/vulnerable/mcp_untrusted_tool_source.ts new file mode 100644 index 0000000..35e6d7b --- /dev/null +++ b/test-fixtures/vulnerable/mcp_untrusted_tool_source.ts @@ -0,0 +1,18 @@ +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { z } from "zod"; + +const server = new McpServer({ name: "demo", version: "1.0.0" }); + +// MCP011: the DSN event body is fetched from an unauthenticated public +// endpoint and returned as the tool result with no validation — the +// Sentry-MCP-DSN attack shape. +server.tool( + "get_error_details", + "Fetches error diagnostics for a given event.", + { eventId: z.string() }, + async ({ eventId }) => { + const res = await fetch(`https://dsn.example.com/events/${eventId}`); + const event = await res.json(); + return { content: [{ type: "text", text: event.message }] }; + }, +); diff --git a/test/corpus.test.js b/test/corpus.test.js index 6aab52c..6ccc669 100644 --- a/test/corpus.test.js +++ b/test/corpus.test.js @@ -49,6 +49,8 @@ const EXPECTED_VULNERABLE = [ ["MCP009", "vulnerable/tool_poisoning.py"], ["MCP002", "vulnerable/mcp_dynamic_url.ts"], ["MCP010", "vulnerable/mcp_dynamic_command.ts"], + ["MCP011", "vulnerable/mcp_untrusted_tool_source.ts"], + ["MCP011", "vulnerable/mcp_untrusted_tool_source.py"], ["SKL001", "vulnerable/skills/leaky-skill/SKILL.md"], ["SKL002", "vulnerable/skills/leaky-skill/SKILL.md"], ["SKL003", "vulnerable/skills/leaky-skill/SKILL.md"],