net: harden v5.9.22 networking changes — strict parser, tests, debug logs

Three pure helper functions extracted from ThreadHTTPSeedFetch2 into netbase.{h,cpp} so the HTTPS seed-list code path can be unit-tested without the SSL/Tor network stack: int DechunkTransferEncoding(const std::string& body, std::string& out) std::vector<std::string> ParseSeedListBody(const std::string& body) bool IsValidSocksNegotiationTimeout(int nMs) DechunkTransferEncoding is now strict (was lenient): - Hex validation: every byte of the chunk-size line is checked with isxdigit() before strtoull. Old code passed a raw strtoul() result which silently accepted leading '+', '-', and whitespace. - strtoull + errno + size_t bounds check replaces the silent 'if (pos+chunkSize > body.size()) chunkSize = body.size()-pos' clamp. The old behavior would mask truncated network reads. - Empty size lines, '+5' / '-5' / ' 5', and unsigned overflow all return DECHUNK_INVALID_HEX (or DECHUNK_OVERSIZE_CHUNK for the bounds case) instead of being treated as 0/last-chunk. - Missing CRLF after chunk data returns DECHUNK_MISSING_DATA_CRLF rather than being read as the next chunk-size line. - Body without a '0\r\n' last-chunk terminator returns DECHUNK_NO_CHUNK_TERMINATOR instead of silently being accepted. - Chunk extensions ('5;foo=bar') are still preserved — the ';' delimiter is stripped from the size line, not from the framing. ParseSeedListBody is a 1:1 extraction of the old loop. Same behavior on every input. Trims inline '#' comments, splits on whitespace / comma / semicolon, normalizes CR-only line endings. IsValidSocksNegotiationTimeout is the central policy: 5000..180000 ms inclusive. Replaces the inline 'nTorTimeout >= 5000 && nTorTimeout <= 180000' check in init.cpp's AppInit2. Out-of-range values now emit an InitWarning so the operator sees why their setting was ignored. Six distinct failure-mode log messages in ThreadHTTPSeedFetch2: 1. 'cannot connect to %s through Tor proxy' — connect failure 2. 'malformed response (no header terminator)' — no \r\n\r\n 3. 'malformed chunked transfer encoding (%s)' — DechunkResult enum reason string 4. 'empty response from %s' — 0 bytes read 5. 'parsed response contained zero valid addresses' — body parsed but CService validation dropped all 6. '%d addresses found from HTTPS seed list' — success path Help text for -torconnecttimeout now precisely describes what the value bounds (the SOCKS5 handshake — send/recv of init/auth/connect), not 'time to reach the onion' which was misleading. The onion-resolution time is bounded by Tor's own SocksTimeout (~120s) and is not directly controllable from the daemon. src/test/http_seed_tests.cpp adds 43 new Boost.Test cases covering every scenario in the hardening brief: DechunkTransferEncoding: 16 cases - single chunk, multiple chunks, chunk extensions (one and multiple), uppercase hex, payload containing CRLF, awkward boundary that looks like a chunk-size line, last-chunk with extension - empty body, no CRLF after size, invalid hex, empty size line, oversize chunk, truncated last-chunk marker, missing data CRLF, strtoul overflow, sign in size, whitespace in size, no last chunk ParseSeedListBody: 14 cases - empty, single-per-line, CRLF endings, multiple-per-line (space, comma, semicolon, mixed), inline comments, blank lines, all-comments, portless onion, invalid entry preserved, trailing whitespace, mixed CRLF/LF IsValidSocksNegotiationTimeout: 9 cases - 4999 (out), 5000 (in, exact lower), 60000 (in, default), 180000 (in, exact upper), 180001 (out), 0 (out), -1 (out), INT_MAX (out, guard against wraparound), 3 midrange values Integration: 1 round-trip case - Encode a seed body as chunked, dechunk it, then parse the result. Verifies the two helpers compose correctly. Test results: 183 test cases total, *** No errors detected. Existing onion_v3_tests (8) and netbase_tests (10) still pass.
2026-06-22 00:55:51 -07:00
parent 43db0138c6
commit ff0eeaac89
6 changed files with 738 additions and 44 deletions
@@ -1750,6 +1750,10 @@ bool ThreadHTTPSeedFetch2(void* parg)
        // body then carries hex chunk-size lines interleaved with the data; parsing
        // it raw fuses a chunk marker onto an address and we lose most of the list
        // (the classic "only 1 address" symptom). De-chunk first when present.
+        //
+        // v5.9.22 hardening: the parser is now strict and reports a distinct
+        // failure code for each kind of malformed framing. See DechunkResult in
+        // netbase.h and the unit tests in src/test/http_seed_tests.cpp.
        {
            std::string h = headers;
            for (char& c : h) c = (char)tolower((unsigned char)c);
@@ -1757,22 +1761,20 @@ bool ThreadHTTPSeedFetch2(void* parg)
                h.find("chunked") != std::string::npos)
            {
                std::string decoded;
-                size_t pos = 0;
-                while (pos < body.size()) {
-                    size_t eol = body.find("\r\n", pos);
-                    if (eol == std::string::npos) break;
-                    std::string sizeLine = body.substr(pos, eol - pos);
-                    size_t semi = sizeLine.find(';'); // strip chunk extensions
-                    if (semi != std::string::npos) sizeLine = sizeLine.substr(0, semi);
-                    unsigned long chunkSize = strtoul(sizeLine.c_str(), nullptr, 16);
-                    pos = eol + 2;
-                    if (chunkSize == 0) break;              // last chunk
-                    if (pos + chunkSize > body.size())
-                        chunkSize = body.size() - pos;       // defensive clamp
-                    decoded.append(body, pos, chunkSize);
-                    pos += chunkSize;
-                    if (pos + 2 <= body.size() && body.compare(pos, 2, "\r\n") == 0)
-                        pos += 2;                            // trailing CRLF after data
+                int rc = DechunkTransferEncoding(body, decoded);
+                if (rc != DECHUNK_OK) {
+                    const char* reason = "unknown";
+                    switch (rc) {
+                        case DECHUNK_EMPTY:               reason = "empty body"; break;
+                        case DECHUNK_NO_CHUNK_TERMINATOR: reason = "missing chunk terminator (CRLF)"; break;
+                        case DECHUNK_INVALID_HEX:         reason = "malformed chunk-size (not valid hex)"; break;
+                        case DECHUNK_OVERSIZE_CHUNK:      reason = "chunk size exceeds remaining input (truncated)"; break;
+                        case DECHUNK_MISSING_DATA_CRLF:   reason = "missing CRLF after chunk data"; break;
+                        default:                          reason = "unknown"; break;
+                    }
+                    printf("HTTPS seed fetch: malformed chunked transfer encoding (%s) from %s\n",
+                           reason, seedHost.c_str());
+                    return false;
                }
                body.swap(decoded);
            }
@@ -1783,7 +1785,11 @@ bool ThreadHTTPSeedFetch2(void* parg)

        // Tolerant parse: accept one-per-line OR several addresses on one line
        // (whitespace / comma / semicolon separated), and ignore inline '#' comments.
+        // v5.9.22: the splitting logic is now a pure function in netbase.cpp so
+        // we can unit-test every line format. The CNetAddr/CService/addrman
+        // validation stays here because it touches globals.
        int found = 0;
+        int skipped = 0;

        auto addSeed = [&](std::string addrStr) -> void {
            while (!addrStr.empty() && (addrStr.back()=='\r' || addrStr.back()==' ' || addrStr.back()=='\t'))
@@ -1811,38 +1817,34 @@ bool ThreadHTTPSeedFetch2(void* parg)
                addrman.Add(addr, service);
                printf("HTTPS seed: added %s:%d\n", addrStr.c_str(), port);
                found++;
+            } else {
+                skipped++;
            }
        };

-        std::istringstream lines(body);
-        std::string line;
-        while (std::getline(lines, line))
+        // Use the pure helper to split the body. If it returns nothing, that
+        // means the body was entirely comments / blank lines / whitespace —
+        // distinct failure mode worth logging separately from "no valid
+        // addresses after parsing".
+        std::vector<std::string> tokens = ParseSeedListBody(body);
+        if (tokens.empty()) {
+            printf("HTTPS seed fetch: parsed response contained zero valid addresses from %s\n", seedHost.c_str());
+            return false;
+        }
+
+        for (const std::string& tok : tokens)
        {
            if (fShutdown)
                return false;
-
-            // Strip inline comments (everything from '#' onward)
-            size_t hashPos = line.find('#');
-            if (hashPos != std::string::npos)
-                line = line.substr(0, hashPos);
-
-            // Split on whitespace / comma / semicolon so multiple addresses on
-            // one line are all captured.
-            size_t start = 0;
-            while (start <= line.size()) {
-                size_t sep = line.find_first_of(" \t,;", start);
-                std::string tok = (sep == std::string::npos)
-                    ? line.substr(start)
-                    : line.substr(start, sep - start);
-                if (!tok.empty())
-                    addSeed(tok);
-                if (sep == std::string::npos) break;
-                start = sep + 1;
-            }
+            addSeed(tok);
        }

        printf("%d addresses found from HTTPS seed list (%s)\n", found, seedHost.c_str());
-        return found > 0;
+        if (found == 0) {
+            printf("HTTPS seed fetch: parsed response contained zero valid addresses from %s\n", seedHost.c_str());
+            return false;
+        }
+        return true;

    } catch (std::exception& e) {
        printf("HTTPS seed fetch failed: %s\n", e.what());