modelcontextprotocol / modelcontextprotocol/java-sdk

listTools() and the other no-arg list*() methods follow the cursor chain without any bound

未关闭
#1,084 1 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

bug P3
主要语言
Java
星标
3.7k
派生
1.1k
平均合并
1 天 15 小时
30 天内合并 PR
9

描述

Bug description

The no-arg listTools() does not issue one request — it calls listTools(FIRST_PAGE) and then
Mono.expand(...) over the nextCursor chain, reducing every page into one result. There is no
page limit and no aggregate deadline, so a server that always returns a non-null, non-empty
nextCursor keeps the client requesting forever while the accumulated list grows until the heap
is exhausted. McpSyncClient.listTools() blocks on it with no duration, so the calling thread
never returns.

Same shape in listResources(), listResourceTemplates() and listPrompts().

requestTimeout does not help: it bounds each individual request, and every request here
completes promptly. It is the number of requests that is unbounded.

Not a duplicate of #575. That one (with #628, #615) is about nextCursor arriving as an
empty string, and #954 fixed it by tightening the guard to next != null && !next.isEmpty().
That stops a server which signals the end badly; it does not bound a server which never
signals the end
. The reproduction below sends a distinct, non-empty cursor every time
(page-1, page-2, …), so it passes the #954 guard on every iteration. A server that repeats a
cursor it already returned is also still unhandled.

Environment

  • io.modelcontextprotocol.sdk:mcp:2.0.0
  • JDK 21.0.8 (Temurin), Windows 11
  • HttpClientStreamableHttpTransport (stdio uses the same client code path)

Numbers below are from released 2.0.0, which predates #954. I have not built main — my reading
of the #954 diff is that it only tightens the guard and leaves the unbounded
expand(...).reduce(...) in place, but if I have misread that, this is moot and I would rather
be told so.

Steps to reproduce

  1. Start an MCP server that answers tools/list with a fresh non-empty nextCursor every time.
  2. Call client.listTools() once.

Expected behavior

The call terminates, or fails, within some bound the caller can rely on. Any of these would do:

  1. A configurable page cap on SyncSpec / AsyncSpec with a sane default.
  2. An aggregate deadline, separate from requestTimeout.
  3. Documentation only, if the behaviour is intentional — a javadoc note on the no-arg overloads
    saying they follow the cursor chain without limit, so callers talking to untrusted servers
    should drive list*(String cursor) themselves.

Option 3 alone would have been enough for me. The trap is that the no-arg overload reads like
"one request, first page" and is the obvious method to reach for; nothing at the call site
suggests it is a loop.

Actual behavior

One listTools() call issued 141,000 requests in 45 seconds and was still going when killed.

Under -Xmx32m it reached ~49,000 pages and threw OutOfMemoryError — but not to the caller:

Exception: java.lang.OutOfMemoryError thrown from the UncaughtExceptionHandler in thread "HTTP-Dispatcher"
Exception: java.lang.OutOfMemoryError thrown from the UncaughtExceptionHandler in thread "HttpClient-1-Worker-3"

It surfaced on transport worker threads and the process still had to be killed. So the caller can
neither bound it nor catch it.

Minimal Complete Reproducible example

Single file, no dependencies beyond the SDK. The only hostile line is the one marked.

Repro.javajavac -cp <mcp jars> Repro.java && java -Xmx32m -cp "<mcp jars>;." Repro
import com.sun.net.httpserver.HttpServer;
import io.modelcontextprotocol.client.McpClient;
import io.modelcontextprotocol.client.McpSyncClient;
import io.modelcontextprotocol.client.transport.HttpClientStreamableHttpTransport;
import io.modelcontextprotocol.spec.McpSchema;

import java.io.InputStream;
import java.io.OutputStream;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class Repro {

    static final Pattern ID = Pattern.compile("\"id\"\\s*:\\s*(\"(?:[^\"\\\\]|\\\\.)*\"|\\d+)");
    static final Pattern METHOD = Pattern.compile("\"method\"\\s*:\\s*\"([^\"]+)\"");
    static final Pattern VERSION = Pattern.compile("\"protocolVersion\"\\s*:\\s*\"([^\"]+)\"");
    static final AtomicInteger pages = new AtomicInteger();

    public static void main(String[] args) throws Exception {
        HttpServer server = HttpServer.create(
                new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0);
        server.createContext("/mcp", exchange -> {
            String body;
            try (InputStream in = exchange.getRequestBody()) {
                body = new String(in.readAllBytes(), StandardCharsets.UTF_8);
            }
            if (!"POST".equalsIgnoreCase(exchange.getRequestMethod())) {
                exchange.sendResponseHeaders(405, -1);
                exchange.close();
                return;
            }
            String method = group(METHOD, body, "");
            if (method.startsWith("notifications/")) {
                exchange.sendResponseHeaders(202, -1);
                exchange.close();
                return;
            }
            String id = group(ID, body, "\"1\"");
            String response = switch (method) {
                case "initialize" -> "{\"jsonrpc\":\"2.0\",\"id\":" + id + ",\"result\":{"
                        + "\"protocolVersion\":\"" + group(VERSION, body, "2025-06-18") + "\","
                        + "\"capabilities\":{\"tools\":{}},"
                        + "\"serverInfo\":{\"name\":\"endless\",\"version\":\"0.0.1\"}}}";
                case "tools/list" -> {
                    int page = pages.incrementAndGet();
                    if (page % 1000 == 0) {
                        System.out.println("served " + page + " pages of tools/list...");
                    }
                    // The only hostile line: nextCursor is non-null, non-empty, and never ends.
                    yield "{\"jsonrpc\":\"2.0\",\"id\":" + id + ",\"result\":{\"tools\":[{"
                            + "\"name\":\"tool_" + page + "\",\"description\":\"Page " + page + "\","
                            + "\"inputSchema\":{\"type\":\"object\",\"properties\":{}}}],"
                            + "\"nextCursor\":\"page-" + page + "\"}}";
                }
                default -> "{\"jsonrpc\":\"2.0\",\"id\":" + id
                        + ",\"error\":{\"code\":-32601,\"message\":\"Method not found\"}}";
            };
            byte[] bytes = response.getBytes(StandardCharsets.UTF_8);
            exchange.getResponseHeaders().set("Content-Type", "application/json");
            exchange.sendResponseHeaders(200, bytes.length);
            try (OutputStream out = exchange.getResponseBody()) {
                out.write(bytes);
            }
        });
        server.start();

        String url = "http://" + server.getAddress().getHostString() + ":" + server.getAddress().getPort();
        McpSyncClient client = McpClient
                .sync(HttpClientStreamableHttpTransport.builder(url).endpoint("/mcp").build())
                .requestTimeout(Duration.ofSeconds(5)) // bounds each request, not the call below
                .build();
        client.initialize();

        System.out.println("calling listTools() — expected: returns; actual: never returns");
        McpSchema.ListToolsResult result = client.listTools();
        System.out.println("unreachable: " + result.tools().size());
    }

    static String group(Pattern pattern, String text, String fallback) {
        Matcher matcher = pattern.matcher(text);
        return matcher.find() ? matcher.group(1) : fallback;
    }
}

Impact

Not remote code execution, and not exploitable against a server the client already trusts — you
have to connect to a hostile or buggy server first. It matters for clients that connect to
servers because they are untrusted: registry crawlers, marketplace indexers, and scanners that
inspect a third-party MCP server before a user installs it. For those, a server can hang or OOM
the client with a four-line handler.

I hit this writing a security scanner. My workaround is to never call the no-arg overloads: drive
list*(String cursor) a page at a time, cap the pages, and stop early if a cursor repeats.

Happy to open a PR for option 1 or 3 if you say which you would take.

贡献指南

打开贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

调研方向

从不带参数的 McpSyncClient.listTools() 流程及其对应的 listResources()、listResourceTemplates() 和 listPrompts() 方法开始;Repro.java 演示了无界的游标链。检查 SyncSpec 和 AsyncSpec 如何提供所请求的上限,然后验证同步调用和异步调用中的所选行为。完成的标准是:调用在已记录且可强制执行的上限内终止或失败,或者明确记录无参数行为。

由索引模型根据 Issue 内容生成。

评估

技术栈
java
领域
api, backend-api-design
Issue 类型
缺陷
难度
4/5
预计耗时
3-5 天
活跃度
冷清
描述清晰度
基本清楚
新手友好度
38/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。