Bound stdio frame reads with max_line_bytes
## Motivation and Context CRuby's `IO#gets` without a limit reads until the next newline with no upper bound. A peer that never emits a newline can therefore grow a single String until the process is killed by the OS OOM killer. This is a denial-of-service exposure (GHSA-7683-3w9x-ch42), identified during a cross-SDK audit of the same unbounded-buffer pattern across the MCP SDKs. It is most relevant when a host application spawns a sandboxed or untrusted MCP server and pipes its stdout into an unconstrained host process, where a memory-capped peer can exhaust the host. Each frame is now capped at a configurable `max_line_bytes` (default 4 MiB, large enough for any realistic frame including base64-embedded images, and the value recommended in the advisory). A frame that reaches the limit without a newline is treated as a transport error: the client raises, and the server reports the error and closes the connection. A short final frame that ends at EOF without a trailing newline is still accepted, so legitimate traffic is unaffected. Both constructors validate `max_line_bytes:` as a positive Integer, since `nil` or a non-positive value makes `IO#gets` read without an upper bound and would silently disable the protection. ## How Has This Been Tested? Server tests cover the `open` read loop stopping and reporting on an over-limit frame, the `send_request` read loop raising on an over-limit response, and a valid final frame without a trailing newline still being processed. Client tests cover `read_response` raising on an over-limit response frame and a normal response still round-tripping within a custom `max_line_bytes`. Both transports also have tests rejecting invalid `max_line_bytes:` values (`nil`, zero, negative, non-Integer) with an `ArgumentError`. The full test suite and RuboCop pass. ## Breaking Changes None. The default 4 MiB limit is far larger than any realistic JSON-RPC frame, so existing well-behaved traffic is unaffected. The new `max_line_bytes:` keyword on both transport constructors is purely additive and optional.
This commit is contained in:
Koichi ITO
parent
5518e8db03
commit
267b8fa628
2 changed files with 285 additions and 5 deletions
|
|
@ -1060,6 +1060,116 @@ module MCP
|
|||
end
|
||||
end
|
||||
|
||||
def test_new_raises_argument_error_when_max_line_bytes_is_not_a_positive_integer
|
||||
[nil, 0, -1, 1.5, "1024"].each do |invalid|
|
||||
error = assert_raises(ArgumentError) do
|
||||
Stdio.new(command: "ruby", args: ["server.rb"], max_line_bytes: invalid)
|
||||
end
|
||||
assert_equal("max_line_bytes must be a positive Integer", error.message)
|
||||
end
|
||||
end
|
||||
|
||||
def test_send_request_raises_when_response_frame_exceeds_max_line_bytes
|
||||
stdin_read, stdin_write = IO.pipe
|
||||
stdout_read, stdout_write = IO.pipe
|
||||
stderr_read, _ = IO.pipe
|
||||
|
||||
Open3.stubs(:popen3).returns([stdin_write, stdout_read, stderr_read, mock_wait_thread])
|
||||
|
||||
transport = Stdio.new(command: "ruby", args: ["server.rb"], max_line_bytes: 1024)
|
||||
|
||||
request = { jsonrpc: "2.0", id: "test-id", method: "tools/list" }
|
||||
|
||||
server_thread = Thread.new do
|
||||
init_line = stdin_read.gets
|
||||
init_request = JSON.parse(init_line)
|
||||
stdout_write.puts(JSON.generate({
|
||||
jsonrpc: "2.0",
|
||||
id: init_request["id"],
|
||||
result: {
|
||||
protocolVersion: "2025-11-25",
|
||||
capabilities: {},
|
||||
serverInfo: { name: "test-server", version: "1.0.0" },
|
||||
},
|
||||
}))
|
||||
stdout_write.flush
|
||||
|
||||
# Read initialized notification.
|
||||
stdin_read.gets
|
||||
|
||||
# Read tools/list request, then stream an unbounded frame with no newline.
|
||||
stdin_read.gets
|
||||
stdout_write.write("A" * 2000)
|
||||
stdout_write.flush
|
||||
end
|
||||
|
||||
transport.connect
|
||||
|
||||
error = assert_raises(RequestHandlerError) do
|
||||
transport.send_request(request: request)
|
||||
end
|
||||
|
||||
assert_match(/exceeds 1024 bytes without a newline/, error.message)
|
||||
assert_equal(:internal_error, error.error_type)
|
||||
ensure
|
||||
server_thread.join
|
||||
stdin_read.close
|
||||
stdin_write.close
|
||||
stdout_read.close
|
||||
stdout_write.close
|
||||
end
|
||||
|
||||
def test_send_request_accepts_response_within_custom_max_line_bytes
|
||||
stdin_read, stdin_write = IO.pipe
|
||||
stdout_read, stdout_write = IO.pipe
|
||||
stderr_read, _ = IO.pipe
|
||||
|
||||
Open3.stubs(:popen3).returns([stdin_write, stdout_read, stderr_read, mock_wait_thread])
|
||||
|
||||
transport = Stdio.new(command: "ruby", args: ["server.rb"], max_line_bytes: 1024)
|
||||
|
||||
request = { jsonrpc: "2.0", id: "test-id", method: "tools/list" }
|
||||
|
||||
server_thread = Thread.new do
|
||||
init_line = stdin_read.gets
|
||||
init_request = JSON.parse(init_line)
|
||||
stdout_write.puts(JSON.generate({
|
||||
jsonrpc: "2.0",
|
||||
id: init_request["id"],
|
||||
result: {
|
||||
protocolVersion: "2025-11-25",
|
||||
capabilities: {},
|
||||
serverInfo: { name: "test-server", version: "1.0.0" },
|
||||
},
|
||||
}))
|
||||
stdout_write.flush
|
||||
|
||||
stdin_read.gets
|
||||
|
||||
tools_line = stdin_read.gets
|
||||
tools_request = JSON.parse(tools_line)
|
||||
stdout_write.puts(JSON.generate({
|
||||
jsonrpc: "2.0",
|
||||
id: tools_request["id"],
|
||||
result: { tools: [] },
|
||||
}))
|
||||
stdout_write.flush
|
||||
end
|
||||
|
||||
transport.connect
|
||||
|
||||
response = transport.send_request(request: request)
|
||||
|
||||
assert_equal("test-id", response["id"])
|
||||
assert_equal({ "tools" => [] }, response["result"])
|
||||
ensure
|
||||
server_thread.join
|
||||
stdin_read.close
|
||||
stdin_write.close
|
||||
stdout_read.close
|
||||
stdout_write.close
|
||||
end
|
||||
|
||||
def test_send_notification_writes_json_line_without_waiting_for_response
|
||||
stdin_read, stdin_write = IO.pipe
|
||||
stdout_read, stdout_write = IO.pipe
|
||||
|
|
|
|||
|
|
@ -441,6 +441,91 @@ module MCP
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
test "raises ArgumentError when max_line_bytes is not a positive Integer" do
|
||||
[nil, 0, -1, 1.5, "1024"].each do |invalid|
|
||||
error = assert_raises(ArgumentError) do
|
||||
StdioTransport.new(@server, max_line_bytes: invalid)
|
||||
end
|
||||
assert_equal("max_line_bytes must be a positive Integer", error.message)
|
||||
end
|
||||
end
|
||||
|
||||
test "open stops and reports when a stdin frame exceeds max_line_bytes without a newline" do
|
||||
transport = StdioTransport.new(@server, max_line_bytes: 64)
|
||||
input = StringIO.new("A" * 200) # No newline: an unbounded frame.
|
||||
output = StringIO.new
|
||||
|
||||
reported = []
|
||||
original_stdin = $stdin
|
||||
original_stdout = $stdout
|
||||
original_reporter = MCP.configuration.exception_reporter
|
||||
|
||||
begin
|
||||
$stdin = input
|
||||
$stdout = output
|
||||
MCP.configuration.exception_reporter = ->(e, ctx) { reported << [e, ctx] }
|
||||
|
||||
transport.open
|
||||
|
||||
refute(transport.instance_variable_get(:@open))
|
||||
assert_equal(1, reported.size)
|
||||
error, _ctx = reported.first
|
||||
assert_instance_of(RequestHandlerError, error)
|
||||
assert_match(/exceeds 64 bytes without a newline/, error.message)
|
||||
ensure
|
||||
$stdin = original_stdin
|
||||
$stdout = original_stdout
|
||||
MCP.configuration.exception_reporter = original_reporter
|
||||
end
|
||||
end
|
||||
|
||||
test "open processes a valid final frame that ends at EOF without a trailing newline" do
|
||||
request = { jsonrpc: "2.0", method: "ping", id: "eof" }
|
||||
input = StringIO.new(JSON.generate(request)) # No trailing newline.
|
||||
output = StringIO.new
|
||||
|
||||
original_stdin = $stdin
|
||||
original_stdout = $stdout
|
||||
|
||||
begin
|
||||
$stdin = input
|
||||
$stdout = output
|
||||
|
||||
@transport.open
|
||||
|
||||
response = JSON.parse(output.string, symbolize_names: true)
|
||||
assert_equal("eof", response[:id])
|
||||
assert_equal({}, response[:result])
|
||||
ensure
|
||||
$stdin = original_stdin
|
||||
$stdout = original_stdout
|
||||
end
|
||||
end
|
||||
|
||||
test "send_request raises when a response frame exceeds max_line_bytes without a newline" do
|
||||
transport = StdioTransport.new(@server, max_line_bytes: 64)
|
||||
input = StringIO.new("A" * 200) # No newline: an unbounded frame.
|
||||
output = StringIO.new
|
||||
|
||||
original_stdin = $stdin
|
||||
original_stdout = $stdout
|
||||
|
||||
begin
|
||||
$stdin = input
|
||||
$stdout = output
|
||||
transport.instance_variable_set(:@open, true)
|
||||
|
||||
error = assert_raises(RequestHandlerError) do
|
||||
transport.send_request("sampling/createMessage", { messages: [] })
|
||||
end
|
||||
|
||||
assert_match(/exceeds 64 bytes without a newline/, error.message)
|
||||
ensure
|
||||
$stdin = original_stdin
|
||||
$stdout = original_stdout
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in a new issue