diff --git a/bridge/cutter/rea_bridge.py b/bridge/cutter/rea_bridge.py index be229f081d..adaaa20ef9 100644 --- a/bridge/cutter/rea_bridge.py +++ b/bridge/cutter/rea_bridge.py @@ -625,12 +625,14 @@ class _BridgeServer(threading.Thread): data = bytearray() deadline = time.monotonic() + AUTH_FRAME_TIMEOUT_SECONDS while b"\n" not in data: - if len(data) >= MAX_FRAME_BYTES: + if len(data) > MAX_FRAME_BYTES: raise ValueError("frame-limit-exceeded") remaining = deadline - time.monotonic() if remaining <= 0: raise socket.timeout("authentication frame timed out") connection.settimeout(remaining) + # MAX_FRAME_BYTES limits the JSON payload; the terminating LF is + # framing and may be the one byte beyond that payload budget. part = connection.recv(min(65536, MAX_FRAME_BYTES + 1 - len(data))) if not part: raise ValueError("incomplete authentication frame") diff --git a/bridge/hopper_bridge.py b/bridge/hopper_bridge.py index 73504b620d..3636c6b440 100644 --- a/bridge/hopper_bridge.py +++ b/bridge/hopper_bridge.py @@ -14,6 +14,9 @@ from typing import Any, Optional, Protocol, Sequence BAD_ADDRESSES = (-1, 0xFFFFFFFFFFFFFFFF, None) # The native label setter truncates NSString names at this UTF-16 extent. HOPPER_SYMBOL_NAME_UTF16_UNITS = 1024 +# Maximum request JSON payload size in bytes, excluding its trailing LF. +# Matches the Cutter bridge's MAX_FRAME_BYTES payload ceiling. +MAX_REQUEST_BYTES = 16 * 1024 * 1024 _rea_session_document = None _search_inventory_cache = {} _pseudocode_cache = {} @@ -1060,9 +1063,23 @@ def _serve_connection(connection): should_stop = False try: while True: - line = file.readline() + # Bound the read itself. The payload budget excludes LF, so permit + # one additional byte for the delimiter while still refusing an + # oversized unterminated peer without waiting for EOF. + line = file.readline(MAX_REQUEST_BYTES + 1) if not line: break + if not line.endswith(b"\n"): + message = ( + "Bridge request exceeds the %d-byte payload limit" + % MAX_REQUEST_BYTES + if len(line) > MAX_REQUEST_BYTES + else "Incomplete bridge request frame" + ) + _write_message(file, {"id": 0, "error": _safe_diagnostic( + InvalidRequestError(message) + )}) + break request_id = None authenticated = False should_stop = False diff --git a/docs/reverse-engineering-tools.md b/docs/reverse-engineering-tools.md index ab2e0c83ed..8b2ad94af1 100644 --- a/docs/reverse-engineering-tools.md +++ b/docs/reverse-engineering-tools.md @@ -71,7 +71,11 @@ Cutter commands use the local REA Python bridge and authenticated loopback messa The client and plugin must support the current v2 handshake; older peers are rejected before dispatch because they cannot preserve cross-caller recovery proof. The client checks the final encoded request against the bridge's 16 MiB frame -limit before dispatch; an oversized command does not retain a cleanup owner. +limit before dispatch; the 16 MiB budget applies to request payload bytes and +excludes the terminating LF. The bridge accepts that full payload size and +rejects larger or incomplete frames while bounding the read; an oversized +command does not retain a cleanup owner. Hopper uses the same request payload +budget. Each bridge request waits 25000 ms. Discovery admits at most 256 descriptor candidates and reads and probes at most eight concurrently. A larger directory reports `bridge_directory_capacity_exceeded`. This release has no runtime diff --git a/tests/boundary/providers/hopper/hopperBridgeFrameLimit.test.ts b/tests/boundary/providers/hopper/hopperBridgeFrameLimit.test.ts new file mode 100644 index 0000000000..f07264b867 --- /dev/null +++ b/tests/boundary/providers/hopper/hopperBridgeFrameLimit.test.ts @@ -0,0 +1,87 @@ +import { execFile } from "node:child_process"; +import { fileURLToPath } from "node:url"; +import { promisify } from "node:util"; + +import { expect, it } from "vitest"; +import { z } from "zod"; + +const execute = promisify(execFile); + +const bridgePath = new URL( + "../../../../bridge/hopper_bridge.py", + import.meta.url, +); +const probePath = new URL( + "../../../fixtures/hopperBridgeFrameLimitProbe.py", + import.meta.url, +); + +/** Mirrors the Cutter bridge's frame ceiling so both bridges bound identically. */ +const MAX_REQUEST_BYTES = 16 * 1024 * 1024; + +const exchangeSchema = z.strictObject({ + reply: z.unknown(), + next: z.unknown().optional(), + thread_released: z.boolean(), + dispatch_calls: z.number().int().nonnegative(), +}); + +const probeResultSchema = z.strictObject({ + limit: z.number().int().positive(), + legit: exchangeSchema, + at_limit: exchangeSchema, + past_limit: exchangeSchema, + stalled_line: exchangeSchema, + eof_partial: exchangeSchema, +}); + +it("serves requests up to the frame limit and refuses a line that exceeds it", async () => { + const { stdout } = await execute("python3", [ + fileURLToPath(probePath), + fileURLToPath(bridgePath), + ]); + const result = probeResultSchema.parse(JSON.parse(stdout)); + expect(result.limit).toBe(MAX_REQUEST_BYTES); + + // Legitimate traffic is unaffected, including a request that fills the limit. + for (const exchange of [result.legit, result.at_limit]) + expect(exchange.reply).toMatchObject({ + result: { run_id: "owned-run", ready: true }, + }); + + // A peer that keeps a line open past the limit is answered without reading + // the line to completion. The refusal can be consumed by the peer's own + // reset, so this asserts boundedness rather than the diagnostic's arrival. + expect(result.past_limit.reply).not.toEqual({ + socket_error: "TimeoutError", + }); + expect(result.past_limit.thread_released).toBe(true); + + // An unterminated payload one byte over the limit is refused with a typed + // diagnostic naming the limit and the connection is closed, instead of + // resynchronizing mid-line. The peer stays connected, so this pins the + // payload ceiling without charging the LF against it. + expect(result.stalled_line.reply).toMatchObject({ + error: { + code: -32000, + type: "invalid_request", + message: expect.stringContaining(String(MAX_REQUEST_BYTES)), + }, + }); + expect(result.stalled_line.next).toEqual({ closed: true }); + expect(result.stalled_line.thread_released).toBe(true); + + // A valid authenticated payload without LF is still an incomplete frame. + // Half-closing the write side supplies EOF; the bridge rejects without + // dispatching the operation, then closes its response connection. + expect(result.eof_partial.reply).toMatchObject({ + error: { + code: -32000, + type: "invalid_request", + message: "Incomplete bridge request frame", + }, + }); + expect(result.eof_partial.next).toEqual({ closed: true }); + expect(result.eof_partial.dispatch_calls).toBe(0); + expect(result.eof_partial.thread_released).toBe(true); +}); diff --git a/tests/fixtures/cutter_bridge_protocol_test.py b/tests/fixtures/cutter_bridge_protocol_test.py index d6ea183ea1..af6b502185 100644 --- a/tests/fixtures/cutter_bridge_protocol_test.py +++ b/tests/fixtures/cutter_bridge_protocol_test.py @@ -592,6 +592,27 @@ class ProtocolProducerTest(unittest.TestCase): receiver.close() +class RequestFrameLimitTest(unittest.TestCase): + def test_payload_limit_excludes_the_line_terminator(self): + class BufferedSocket: + def __init__(self, data): + self.data = data + + def settimeout(self, _timeout): + pass + + def recv(self, count): + part, self.data = self.data[:count], self.data[count:] + return part + + payload = b"{}" + b" " * (bridge.MAX_FRAME_BYTES - 2) + self.assertEqual( + bridge._BridgeServer._receive_frame(BufferedSocket(payload + b"\n")), {} + ) + with self.assertRaisesRegex(ValueError, "frame-limit-exceeded"): + bridge._BridgeServer._receive_frame(BufferedSocket(payload + b" " + b"\n")) + + if __name__ == "__main__": if sys.argv[1:] == ["--serve"]: run_serve_mode() diff --git a/tests/fixtures/hopperBridgeFrameLimitProbe.py b/tests/fixtures/hopperBridgeFrameLimitProbe.py new file mode 100644 index 0000000000..e59b68a561 --- /dev/null +++ b/tests/fixtures/hopperBridgeFrameLimitProbe.py @@ -0,0 +1,132 @@ +"""Exercise the shipped bridge's request-frame bound over real socket pairs. + +Four cases share one loaded bridge module: + +- ``legit``: an ordinary authenticated request is served. +- ``at_limit``: a request with an exactly-limit JSON payload plus LF is served, + so framing does not consume a payload byte. +- ``past_limit``: a peer writes one byte past the limit and never terminates the + line. An unbounded line read cannot answer this at all, which is what makes + Hopper allocate for as long as the peer keeps writing. +- ``stalled_line``: a peer exceeds the payload limit without a terminator, + stops writing, and keeps the connection open. The bridge must refuse it + without waiting for EOF, then close. +- ``eof_partial``: an authenticated, otherwise valid short JSON request has no + LF and the peer half-closes its write side. The bridge must reject the + incomplete frame without dispatching it, then close. +""" +import json +import socket +import sys +import threading + +from hopperBridgeFacadeProbe import load_bridge + +READ_TIMEOUT_SECONDS = 5 + + +def _next_response(file): + while True: + line = file.readline() + if not line: + return {"closed": True} + value = json.loads(line.decode("utf-8")) + if "event" not in value: + return value + + +def serve(bridge, line, read_after, half_close_write=False): + """Send one raw line and record the bridge's answer.""" + server, client = socket.socketpair() + worker = threading.Thread(target=bridge["_serve_connection"], args=(server,)) + worker.daemon = True + worker.start() + file = client.makefile("rwb") + client.settimeout(READ_TIMEOUT_SECONDS) + outcome = {} + dispatch_count = len(bridge["_probe_dispatches"]) + try: + file.write(line) + file.flush() + if half_close_write: + client.shutdown(socket.SHUT_WR) + outcome["reply"] = _next_response(file) + if read_after: + outcome["next"] = _next_response(file) + except OSError as error: + outcome["reply"] = {"socket_error": type(error).__name__} + finally: + try: + file.close() + finally: + client.close() + worker.join(2) + outcome["thread_released"] = not worker.is_alive() + outcome["dispatch_calls"] = ( + len(bridge["_probe_dispatches"]) - dispatch_count + ) + return outcome + + +def request_line(bridge, params): + request = { + "id": 1, + "token": bridge["REA_TOKEN"], + "method": "health", + "params": params, + } + return json.dumps(request, separators=(",", ":")).encode("utf-8") + + +def main(): + bridge = load_bridge(sys.argv[1]) + bridge.update(REA_TOKEN="owned-token", REA_RUN_ID="owned-run") + bridge["_probe_dispatches"] = [] + + def dispatch(method, params): + bridge["_probe_dispatches"].append((method, params)) + return {"run_id": bridge["REA_RUN_ID"], "ready": True} + + bridge["_dispatch"] = dispatch + limit = bridge.get("MAX_REQUEST_BYTES") + if not isinstance(limit, int) or isinstance(limit, bool) or limit <= 0: + json.dump({"limit": limit}, sys.stdout) + return + + legit = serve(bridge, request_line(bridge, {}) + b"\n", False) + + # The payload may fill the limit; the framing LF is additional. + padding = limit - len(request_line(bridge, {"padding": ""})) + at_limit_line = request_line(bridge, {"padding": "p" * padding}) + b"\n" + assert len(at_limit_line) == limit + 1, len(at_limit_line) + at_limit = serve(bridge, at_limit_line, False) + + # One byte past the limit, still unterminated and still open. + past_limit = serve(bridge, b"x" * (limit + 1), False) + + # The payload limit plus one byte, without a terminator. The peer stays + # connected so the bounded read must reject it without waiting for EOF. + stalled_line = serve(bridge, b"x" * (limit + 1), True) + + # This is a valid authenticated request, but omitting LF makes it an + # incomplete frame. EOF must terminate the read and produce a rejection; + # the application dispatch must never see it. + eof_partial = serve( + bridge, request_line(bridge, {}), True, half_close_write=True + ) + + json.dump( + { + "limit": limit, + "legit": legit, + "at_limit": at_limit, + "past_limit": past_limit, + "stalled_line": stalled_line, + "eof_partial": eof_partial, + }, + sys.stdout, + ) + + +if __name__ == "__main__": + main()