modelcontextprotocol / modelcontextprotocol/java-sdk

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

Aperta
#1,084 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

bug P3
Lingua principale
Java
Stelle
3.7k
Fork
1.1k
Merge medio
1g 15h
PR unite (30g)
9

Descrizione

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.

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Direzione di ricerca

Inizia con il flusso senza argomenti di McpSyncClient.listTools() e i relativi metodi listResources(), listResourceTemplates() e listPrompts(); Repro.java dimostra la catena di cursori senza limiti. Esamina come SyncSpec e AsyncSpec potrebbero fornire il limite richiesto, quindi verifica il comportamento scelto nelle chiamate sincrone e asincrone. Il lavoro è completo quando le chiamate terminano o falliscono entro un limite documentato e applicabile, oppure quando il comportamento senza argomenti è documentato esplicitamente.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
java
Ambito
api, backend-api-design
Tipo di issue
Bug
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Tranquilla
Chiarezza
Abbastanza chiara
Idoneità per principianti
38/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.