What the developer wrote
Authored by Fabian Jahr
fuzz: Improve torcontrol fuzz test
Gets rid of the Dummy class and adds coverage of get_socks_cb.
Also explicitly handles an exception case within Torcontrol rather than
relying on a guard in the fuzz test.
What changed, and why it matters
This commit improves a fuzz test for Bitcoin Core's Tor control code and adds a small safety check in the real Tor control logic. The production change guards against an empty reply from Tor during the SAFECOOKIE authentication challenge, preventing a potential out-of-bounds read or crash. The rest of the change is test cleanup and expanded fuzzing coverage.
No urgent action required. The change is a minor hardening improvement. Users running Tor hidden services should ensure they are on a version containing this commit, but there is no indication of an exploitable vulnerability in the wild.
Security signals we found
Defensive bounds/empty check added before vector index access
Fuzz test expanded to cover additional callback paths
Removal of test-only guard in favor of production-side handling
Evidence from the diff
The patch removes a DummyTorControlConnection class from the torcontrol fuzz target and uses a real TorControlConnection with a CThreadInterrupt. It also adds fuzz coverage for get_socks_cb and removes a test-side guard that broke the loop on empty reply lines. In src/torcontrol.cpp, it adds an explicit empty-check before accessing reply.lines[0] in TorController::authchallenge_cb, logging a warning and returning early if the AUTHCHALLENGE reply has no lines. This is a defensive hardening change.
Changed components
src/torcontrol.cppsrc/test/fuzz/torcontrol.cppTorController::authchallenge_cbInspect captured patch +16 / −33
diff --git a/src/test/fuzz/torcontrol.cpp b/src/test/fuzz/torcontrol.cpp
index 47874ddf..e45f1e62 100644
--- a/src/test/fuzz/torcontrol.cpp
+++ b/src/test/fuzz/torcontrol.cpp
@@ -12,30 +12,6 @@
#include <string>
#include <vector>
-class DummyTorControlConnection : public TorControlConnection
-{
- CThreadInterrupt m_dummy_interrupt;
-
-public:
- DummyTorControlConnection() : TorControlConnection{m_dummy_interrupt}
- {
- }
-
- bool Connect(const std::string&)
- {
- return true;
- }
-
- void Disconnect()
- {
- }
-
- bool Command(const std::string&, const ReplyHandlerCB&)
- {
- return true;
- }
-};
-
void initialize_torcontrol()
{
static const auto testing_setup = MakeNoLogFileContext<>();
@@ -46,6 +22,9 @@ FUZZ_TARGET(torcontrol, .init = initialize_torcontrol)
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
TorController tor_controller;
+ CThreadInterrupt interrupt;
+ TorControlConnection conn{interrupt};
+
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
TorControlReply tor_control_reply;
CallOneOf(
@@ -63,26 +42,26 @@ FUZZ_TARGET(torcontrol, .init = initialize_torcontrol)
tor_control_reply.code = fuzzed_data_provider.ConsumeIntegral<int>();
});
tor_control_reply.lines = ConsumeRandomLengthStringVector(fuzzed_data_provider);
- if (tor_control_reply.lines.empty()) {
- break;
- }
- DummyTorControlConnection dummy_tor_control_connection;
+
CallOneOf(
fuzzed_data_provider,
[&] {
- tor_controller.add_onion_cb(dummy_tor_control_connection, tor_control_reply, /*pow_was_enabled=*/true);
+ tor_controller.add_onion_cb(conn, tor_control_reply, /*pow_was_enabled=*/true);
+ },
+ [&] {
+ tor_controller.add_onion_cb(conn, tor_control_reply, /*pow_was_enabled=*/false);
},
[&] {
- tor_controller.add_onion_cb(dummy_tor_control_connection, tor_control_reply, /*pow_was_enabled=*/false);
+ tor_controller.auth_cb(conn, tor_control_reply);
},
[&] {
- tor_controller.auth_cb(dummy_tor_control_connection, tor_control_reply);
+ tor_controller.authchallenge_cb(conn, tor_control_reply);
},
[&] {
- tor_controller.authchallenge_cb(dummy_tor_control_connection, tor_control_reply);
+ tor_controller.protocolinfo_cb(conn, tor_control_reply);
},
[&] {
- tor_controller.protocolinfo_cb(dummy_tor_control_connection, tor_control_reply);
+ tor_controller.get_socks_cb(conn, tor_control_reply);
});
}
}
diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
index ecc75f15..17b37f6f 100644
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -602,6 +602,10 @@ void TorController::authchallenge_cb(TorControlConnection& _conn, const TorContr
{
if (reply.code == TOR_REPLY_OK) {
LogDebug(BCLog::TOR, "SAFECOOKIE authentication challenge successful");
+ if (reply.lines.empty()) {
+ LogWarning("tor: AUTHCHALLENGE reply was empty");
+ return;
+ }
std::pair<std::string,std::string> l = SplitTorReplyLine(reply.lines[0]);
if (l.first == "AUTHCHALLENGE") {
std::map<std::string,std::string> m = ParseTorReplyMapping(l.second);
Why this scored 18/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.