highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-06-01 00:34:01 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#35087: tor: limit torcontrol line size that is processed to prevent OOM

Browse Source 
9fe5896a44 tor: torcontrol disconnect on too many lines to avoid OOM (David Gumberg)
8b68287bf9 test: Make torcontrol max line length test stricter and test boundaries. (David Gumberg)
ab5889796f refactor: torcontrol add connection checks to restart_with_mock (David Gumberg)

Pull request description:

  LLM disclosure: Found with the help of Claude Opus 4.6, fix, test, description, and commit messages written by me.

  ------

  This fixes a low-severity issue where a misbehaving Tor control daemon can cause
  bitcoind to OOM by sending continuation lines without sending `250 OK` or
  similar.

  This issue is not that serious because if your tor control daemon is malicious you are already in all kinds of trouble, but as a matter of robustness this should be fixed.

  The fix is to prevent the `TorControlConnection::m_message` buffer from growing
  without bound by 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:

  <details>

  <summary>

  #### Tor control commands used by Bitcoin Core

  </summary>

  `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`.

  </details>

  ## Reproduce

  To reproduce this issue, the following script or similar can be used as the
  misbehaving Tor control daemon:

  ```python
  #!/usr/bin/env python3
  """
  A fake Tor control service that never finishes its reply. Sends unlimited
  continuation lines ("250-...") without ever sending the final "250 ...".
  Each line accumulates in m_message.lines with no cap. Bitcoind OOMs.
  """

  import socket
  import time

  PORT = 19191

  server = socket.create_server(("127.0.0.1", PORT))
  conn, _ = server.accept()
  conn.recv(4096)  # Receive PROTOCOLINFO

  time_start = time.time()

  try:
      while True:
          conn.sendall(b"250-Ceaseless\r\n" * 10000)
  except (BrokenPipeError, ConnectionResetError):
      elapsed = time.time() - time_start
      print(f"Node disconnected after {elapsed:.2f}s")
  ```

  **🟡¡This will OOM, run in a container, VM, or some sandbox with memory limits!🟡**
  Start a node with `-torcontrol=127.0.0.1=19191`.

  E.g. with systemd:

  ```bash
  systemd-run --user --scope -p MemoryMax=2G -p MemorySwapMax=0 bitcoind -regtest -torcontrol=127.0.0.1:19191
  ```

ACKs for top commit:
  fjahr:
    ACK 9fe5896a44
  danielabrozzoni:
    Code review ACK 9fe5896a44
  janb84:
    ACK. 9fe5896a44
  sedited:
    ACK 9fe5896a44

Tree-SHA512: ccbeba40c096e1fa3911c75c49e3a5c403712f646d77329de48017a19d1f0caa2ee4cc148b6c6473f68e55d7da04f17eb67748b5bf4dede3579b944ee5370cf5
This commit is contained in:
merge-script
2026-04-21 16:17:21 +02:00
parent e32bc7f817 9fe5896a44
commit 875faa29e1
2 changed files with 61 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/torcontrol.cpp
View File

@@ -63,6 +63,11 @@ constexpr std::chrono::duration<double> 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;

71
test/functional/feature_torcontrol.py
View File

@@ -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
@@ -119,6 +120,25 @@ 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")
@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")
@@ -144,11 +164,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"
@@ -202,24 +217,42 @@ 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)
# 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")
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()
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()
@@ -228,6 +261,8 @@ class TorControlTest(BitcoinTestFramework):
self.test_partial_data()
self.test_pow_fallback()
self.test_oversized_line()
self.test_overmany_lines()
if __name__ == '__main__':
TorControlTest(__file__).main()