tor: torcontrol disconnect on too many lines to avoid OOM
What the developer wrote
Authored by David Gumberg
tor: torcontrol disconnect on too many lines to avoid OOM
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`.
What changed, and why it matters
This change fixes a memory safety issue in Bitcoin Core's connection to the Tor control port. A malicious or misbehaving Tor control server could send an endless stream of reply lines, causing Bitcoin Core to keep allocating memory until the system ran out. The patch caps the number of lines accepted in a single reply at 1,000, after which the connection is closed. The commit message explicitly says this is to avoid out-of-memory (OOM) exhaustion.
Treat as a security hardening / DoS-fix commit. Backport to maintained release branches if the unbounded m_message behavior exists there. No immediate emergency response is indicated because the Tor control port is normally localhost-only, but operators should ensure Tor control access is restricted to trusted users/processes.
Security signals we found
Out-of-memory (OOM) protection via bounded buffer
Untrusted network input validation
Denial-of-service mitigation against Tor control connection
Explicit commit-message security rationale ('avoid OOM', 'memory exhaustion')
New regression test for the boundary condition
Evidence from the diff
TorControlConnection::ProcessBuffer() appends parsed lines to m_message.lines without an upper bound. A peer on the Tor control port (typically localhost, but reachable in some configurations) could cause unbounded memory growth by sending many continuation lines. The patch adds MAX_LINE_COUNT = 1000 and throws a runtime_error disconnecting the control connection when that limit is exceeded. The commit message frames this as a belt-and-suspenders limit to prevent memory exhaustion. A functional test is added to verify both the non-disconnect at 999 lines and disconnect at 1001 lines.
Changed components
src/torcontrol.cpp: TorControlConnection::ProcessBuffer()src/torcontrol.cpp: TorControlConnection::m_message buffertest/functional/feature_torcontrol.pyInspect captured patch +29 / −0
diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
index 17b37f6f..0893596d 100644
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -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;
diff --git a/test/functional/feature_torcontrol.py b/test/functional/feature_torcontrol.py
index 9693030b..485ffc44 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()
Why this scored 62/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.