From ab5889796f7fdf0d24d985c0594c00db35461fa7 Mon Sep 17 00:00:00 2001 From: David Gumberg Date: Thu, 16 Apr 2026 18:10:14 -0700 Subject: [PATCH 1/3] refactor: torcontrol add connection checks to restart_with_mock --- test/functional/feature_torcontrol.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/test/functional/feature_torcontrol.py b/test/functional/feature_torcontrol.py index 2148fc10352..5f8cf9f68ee 100755 --- a/test/functional/feature_torcontrol.py +++ b/test/functional/feature_torcontrol.py @@ -119,6 +119,11 @@ class TorControlTest(BitcoinTestFramework): "-debug=tor", ]) + # Wait for connection and PROTOCOLINFO command + mock_tor.conn_ready.wait(timeout=10) + self.wait_until(lambda: len(mock_tor.received_commands) >= 1, timeout=10) + assert_equal(mock_tor.received_commands[0], "PROTOCOLINFO 1") + def test_basic(self): self.log.info("Test Tor control basic functionality") @@ -144,11 +149,6 @@ class TorControlTest(BitcoinTestFramework): mock_tor = MockTorControlServer(self.next_port(), manual_mode=True) self.restart_with_mock(mock_tor) - # Wait for connection and PROTOCOLINFO command - mock_tor.conn_ready.wait(timeout=10) - self.wait_until(lambda: len(mock_tor.received_commands) >= 1, timeout=10) - assert_equal(mock_tor.received_commands[0], "PROTOCOLINFO 1") - # Send partial response (no \r\n on last line) mock_tor.send_raw( "250-PROTOCOLINFO 1\r\n" @@ -207,11 +207,6 @@ class TorControlTest(BitcoinTestFramework): mock_tor = MockTorControlServer(self.next_port(), manual_mode=True) self.restart_with_mock(mock_tor) - # Wait for connection and PROTOCOLINFO command. - mock_tor.conn_ready.wait(timeout=10) - self.wait_until(lambda: len(mock_tor.received_commands) >= 1, timeout=10) - assert_equal(mock_tor.received_commands[0], "PROTOCOLINFO 1") - # Send a single line longer than MAX_LINE_LENGTH. The node should disconnect. MAX_LINE_LENGTH = 100000 mock_tor.send_raw("250-" + ("A" * (MAX_LINE_LENGTH + 1)) + "\r\n") From 8b68287bf9a50016519f7eba9d1a9186b94db516 Mon Sep 17 00:00:00 2001 From: David Gumberg Date: Wed, 15 Apr 2026 10:54:34 -0700 Subject: [PATCH 2/3] test: Make torcontrol max line length test stricter and test boundaries. Adds a check that at the boundary of MAX_LINE_LENGTH, no disconnect occurs. Also makes the overlength test message exactly MAX_LINE_LENGTH + 1 to test the boundary. Drops the redundant node liveness check, which is covered by the later check that the node reconnects. --- test/functional/feature_torcontrol.py | 35 +++++++++++++++++++++------ 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/test/functional/feature_torcontrol.py b/test/functional/feature_torcontrol.py index 5f8cf9f68ee..9693030b8ac 100755 --- a/test/functional/feature_torcontrol.py +++ b/test/functional/feature_torcontrol.py @@ -3,6 +3,7 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test torcontrol functionality with a mock Tor control server.""" +from contextlib import contextmanager import socket import threading from test_framework.test_framework import BitcoinTestFramework @@ -124,6 +125,20 @@ class TorControlTest(BitcoinTestFramework): self.wait_until(lambda: len(mock_tor.received_commands) >= 1, timeout=10) assert_equal(mock_tor.received_commands[0], "PROTOCOLINFO 1") + @contextmanager + def expect_disconnect(self, expect, mock_tor): + initial_len = len(mock_tor.received_commands) + yield + + if expect: + # Expect to receive a PROTOCOLINFO 1 on reconnect, bumping the received + # commands length. + self.wait_until(lambda: len(mock_tor.received_commands) == initial_len + 1) + assert_equal(mock_tor.received_commands[initial_len], "PROTOCOLINFO 1") + else: + # No disconnect, so no reconnect message + ensure_for(duration=2, f=lambda: len(mock_tor.received_commands) == initial_len) + def test_basic(self): self.log.info("Test Tor control basic functionality") @@ -202,19 +217,23 @@ class TorControlTest(BitcoinTestFramework): mock_tor.stop() def test_oversized_line(self): - self.log.info("Test that Tor control disconnects on oversized response lines") - mock_tor = MockTorControlServer(self.next_port(), manual_mode=True) self.restart_with_mock(mock_tor) - # Send a single line longer than MAX_LINE_LENGTH. The node should disconnect. MAX_LINE_LENGTH = 100000 - mock_tor.send_raw("250-" + ("A" * (MAX_LINE_LENGTH + 1)) + "\r\n") - ensure_for(duration=2, f=lambda: self.nodes[0].process.poll() is None) - # Connection should be dropped and retried, causing another PROTOCOLINFO. - self.wait_until(lambda: len(mock_tor.received_commands) >= 2, timeout=10) - assert_equal(mock_tor.received_commands[1], "PROTOCOLINFO 1") + self.log.info("Test that Tor control does not disconnect with a MAX_LINE_LENGTH line.") + with self.expect_disconnect(False, mock_tor): + msg = "250-" + ("A" * (MAX_LINE_LENGTH - 5)) + "\r" + assert_equal(len(msg), MAX_LINE_LENGTH) + # The \n is not counted in line length. + mock_tor.send_raw(msg + "\n") + + self.log.info("Test that Tor control disconnects with a MAX_LINE_LENGTH + 1 line") + with self.expect_disconnect(True, mock_tor): + msg = "250-" + ("A" * (MAX_LINE_LENGTH - 4)) + "\r" + assert_equal(len(msg), MAX_LINE_LENGTH + 1) + mock_tor.send_raw(msg + "\n") mock_tor.stop() From 9fe5896a446d5a1acb5834561ffbca841f8a970a Mon Sep 17 00:00:00 2001 From: David Gumberg Date: Tue, 14 Apr 2026 17:17:23 -0700 Subject: [PATCH 3/3] tor: torcontrol disconnect on too many lines to avoid OOM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit ensures the `TorControlConnection::m_message` buffer doesn't grow unbounded and exhaust memory, by limiting the number of lines handled by `TorControlConnection::ProcessBuffer()` to `MAX_LINE_COUNT = 1000`. Now the most memory that can be occupied by `m_message` is on the order of `MAX_LINE_LENGTH * MAX_LINE_COUNT= 100MB` Although this is not compliant with the tor control protocol in general, where commands like `GETINFO ns/all` will likely return thousands of lines, it is more than sufficient for handling the replies from the commands that are used by a node: `AUTHENTICATE`: 1 line: The server responds with 250 OK on success or 515 Bad authentication if the authentication cookie is incorrect. Tor closes the connection on an authentication failure. https://spec.torproject.org/control-spec/commands.html#authenticate `GETINFO net/listener/socks`: 2 lines A quoted, space-separated list of the locations where Tor is listening... https://spec.torproject.org/control-spec/commands.html#getinfo `AUTHCHALLENGE SAFECOOKIE`: 1 line If the server accepts the command, the server reply format is: ``` "250 AUTHCHALLENGE" SP "SERVERHASH=" ServerHash SP "SERVERNONCE=" ServerNonce CRLF ``` https://spec.torproject.org/control-spec/commands.html#authenticate `PROTOCOLINFO`: 4-5 lines The server reply format is: ``` 250-PROTOCOLINFO" SP PIVERSION CRLF \*InfoLine "250 OK" CRLF InfoLine = AuthLine / VersionLine / OtherLine ``` (https://spec.torproject.org/control-spec/commands.html#protocolinfo) `ADD_ONION`: 2-3 lines for Bitcoin Core's tor control client. The server reply format is: ``` "250-ServiceID=" ServiceID CRLF ["250-PrivateKey=" KeyType ":" KeyBlob CRLF] *("250-ClientAuth=" ClientName ":" ClientBlob CRLF) "250 OK" CRLF ``` ... The server response will only include a private key if the server was requested to generate a new keypair ... If client authorization is enabled using the “BasicAuth” flag (which is v2 only), the service will not be accessible to clients without valid authorization data (configured with the “HidServAuth” option). The list of authorized clients is specified with one or more “ClientAuth” parameters. If “ClientBlob” is not specified for a client, a new credential will be randomly generated and returned." https://spec.torproject.org/control-spec/commands.html#add_onion We don't set the `BasicAuth` flag, so the response will not include any `ClientAuthLines`. --- src/torcontrol.cpp | 8 ++++++++ test/functional/feature_torcontrol.py | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 17b37f6fdd4..0893596d37d 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -63,6 +63,11 @@ constexpr std::chrono::duration RECONNECT_TIMEOUT_MAX{600.0}; * this is belt-and-suspenders sanity limit to prevent memory exhaustion. */ constexpr int MAX_LINE_LENGTH = 100000; +/** Maximum number of lines received on TorControlConnection per reply to avoid + * memory exhaustion. The largest expected now is 5 (PROTOCOLINFO), but future + * changes to this file might need to re-evaluate MAX_LINE_COUNT. + */ +constexpr int MAX_LINE_COUNT = 1000; /** Timeout for socket operations */ constexpr auto SOCKET_SEND_TIMEOUT = 10s; @@ -177,6 +182,9 @@ bool TorControlConnection::ProcessBuffer() auto start = reader.it; while (auto line = reader.ReadLine()) { + if (m_message.lines.size() == MAX_LINE_COUNT) { + throw std::runtime_error(strprintf("Control port reply exceeded %d lines, disconnecting", MAX_LINE_COUNT)); + } // Skip short lines if (line->size() < 4) continue; diff --git a/test/functional/feature_torcontrol.py b/test/functional/feature_torcontrol.py index 9693030b8ac..485ffc44559 100755 --- a/test/functional/feature_torcontrol.py +++ b/test/functional/feature_torcontrol.py @@ -237,11 +237,32 @@ class TorControlTest(BitcoinTestFramework): mock_tor.stop() + def test_overmany_lines(self): + mock_tor = MockTorControlServer(self.next_port(), manual_mode=True) + self.restart_with_mock(mock_tor) + + MAX_LINE_COUNT = 1000 + + self.log.info("Test that Tor control does not disconnect on receiving MAX_LINE_COUNT lines.") + with self.expect_disconnect(False, mock_tor): + for _ in range(MAX_LINE_COUNT - 1): + mock_tor.send_raw("250-Continuing\r\n") + mock_tor.send_raw("250 OK\r\n") + + self.log.info("Test that Tor control disconnects on receiving MAX_LINE_COUNT + 1 lines.") + with self.expect_disconnect(True, mock_tor): + for _ in range(MAX_LINE_COUNT + 1): + mock_tor.send_raw("250-Continuing\r\n") + + mock_tor.stop() + def run_test(self): self.test_basic() self.test_partial_data() self.test_pow_fallback() self.test_oversized_line() + self.test_overmany_lines() + if __name__ == '__main__': TorControlTest(__file__).main()