From 14196d56b3de2343c5d20a905d842999e9362972 Mon Sep 17 00:00:00 2001 From: chromatic Date: Thu, 9 Jun 2022 21:35:11 -0700 Subject: [PATCH] Improve data types for network data While working with other networking code, Patrick and I noticed that we use signed types and wrongly-sized types for several networking related values, including data lengths, timeout durations, and port numbers. This commit corrects several of these types and improves error handling slightly to account for potentially invalid values. --- src/httpserver.cpp | 2 +- src/netbase.cpp | 36 ++++++++++++++++++++---------------- src/netbase.h | 19 ++++++++++--------- src/test/netbase_tests.cpp | 18 +++++++++--------- 4 files changed, 40 insertions(+), 35 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 5c08e676d..a6f72bf6b 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -332,7 +332,7 @@ static bool HTTPBindAddresses(struct evhttp* http) } else if (mapMultiArgs.count("-rpcbind")) { // Specific bind address const std::vector& vbind = mapMultiArgs.at("-rpcbind"); for (std::vector::const_iterator i = vbind.begin(); i != vbind.end(); ++i) { - int port = defaultPort; + uint16_t port = defaultPort; std::string host; SplitHostPort(*i, port, host); endpoints.push_back(std::make_pair(host, port)); diff --git a/src/netbase.cpp b/src/netbase.cpp index 9f007dbc0..789dee5cf 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -1,5 +1,6 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto // Copyright (c) 2009-2016 The Bitcoin Core developers +// Copyright (c) 2022 The Dogecoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -33,7 +34,7 @@ static proxyType proxyInfo[NET_MAX]; static proxyType nameProxy; static CCriticalSection cs_proxyInfos; -int nConnectTimeout = DEFAULT_CONNECT_TIMEOUT; +uint64_t nConnectTimeout = DEFAULT_CONNECT_TIMEOUT; bool fNameLookup = DEFAULT_NAME_LOOKUP; // Need ample time for negotiation for very slow proxies such as Tor (milliseconds) @@ -58,7 +59,7 @@ std::string GetNetworkName(enum Network net) { } } -void SplitHostPort(std::string in, int &portOut, std::string &hostOut) { +void SplitHostPort(std::string in, uint16_t &portOut, std::string &hostOut) { size_t colon = in.find_last_of(':'); // if a : is found, and it either follows a [...], or no other : is in the string, treat it as port separator bool fHaveColon = colon != in.npos; @@ -152,11 +153,11 @@ bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup) return true; } -bool Lookup(const char *pszName, std::vector& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions) +bool Lookup(const char *pszName, std::vector& vAddr, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions) { if (pszName[0] == 0) return false; - int port = portDefault; + uint16_t port = portDefault; std::string hostname = ""; SplitHostPort(std::string(pszName), port, hostname); @@ -170,7 +171,7 @@ bool Lookup(const char *pszName, std::vector& vAddr, int portDefault, return true; } -bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLookup) +bool Lookup(const char *pszName, CService& addr, uint16_t portDefault, bool fAllowLookup) { std::vector vService; bool fRet = Lookup(pszName, vService, portDefault, fAllowLookup, 1); @@ -180,7 +181,7 @@ bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLoo return true; } -CService LookupNumeric(const char *pszName, int portDefault) +CService LookupNumeric(const char *pszName, uint16_t portDefault) { CService addr; // "1.2:345" will fail to resolve the ip, but will still set the port. @@ -190,7 +191,7 @@ CService LookupNumeric(const char *pszName, int portDefault) return addr; } -struct timeval MillisToTimeval(int64_t nTimeout) +struct timeval MillisToTimeval(uint64_t nTimeout) { struct timeval timeout; timeout.tv_sec = nTimeout / 1000; @@ -217,7 +218,7 @@ enum class IntrRecvError { * * @note This function requires that hSocket is in non-blocking mode. */ -static IntrRecvError InterruptibleRecv(char* data, size_t len, int timeout, SOCKET& hSocket) +static IntrRecvError InterruptibleRecv(char* data, size_t len, uint64_t timeout, SOCKET& hSocket) { int64_t curTime = GetTimeMillis(); int64_t endTime = curTime + timeout; @@ -278,7 +279,7 @@ std::string Socks5ErrorString(int err) } /** Connect using SOCKS5 (as described in RFC1928) */ -static bool Socks5(const std::string& strDest, int port, const ProxyCredentials *auth, SOCKET& hSocket) +static bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials *auth, SOCKET& hSocket) { IntrRecvError recvr; LogPrint("net", "SOCKS5 connecting %s\n", strDest); @@ -396,9 +397,12 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials return error("Error reading from proxy"); } int nRecv = pchRet3[0]; - recvr = InterruptibleRecv(pchRet3, nRecv, SOCKS5_RECV_TIMEOUT, hSocket); - break; + if (nRecv > 0) { + recvr = InterruptibleRecv(pchRet3, nRecv, SOCKS5_RECV_TIMEOUT, hSocket); + break; + } } + // Falls through default: CloseSocket(hSocket); return error("Error: malformed proxy response"); } if (recvr != IntrRecvError::OK) { @@ -413,7 +417,7 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials return true; } -bool static ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout) +bool static ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, uint64_t nTimeout) { hSocketRet = INVALID_SOCKET; @@ -550,7 +554,7 @@ bool IsProxy(const CNetAddr &addr) { return false; } -static bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed) +static bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, uint16_t port, SOCKET& hSocketRet, uint64_t nTimeout, bool *outProxyConnectionFailed) { SOCKET hSocket = INVALID_SOCKET; // first connect to proxy server @@ -575,7 +579,7 @@ static bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDe return true; } -bool ConnectSocket(const CService &addrDest, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed) +bool ConnectSocket(const CService &addrDest, SOCKET& hSocketRet, uint64_t nTimeout, bool *outProxyConnectionFailed) { proxyType proxy; if (outProxyConnectionFailed) @@ -587,10 +591,10 @@ bool ConnectSocket(const CService &addrDest, SOCKET& hSocketRet, int nTimeout, b return ConnectSocketDirectly(addrDest, hSocketRet, nTimeout); } -bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest, int portDefault, int nTimeout, bool *outProxyConnectionFailed) +bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest, uint16_t portDefault, uint64_t nTimeout, bool *outProxyConnectionFailed) { std::string strDest; - int port = portDefault; + uint16_t port = portDefault; if (outProxyConnectionFailed) *outProxyConnectionFailed = false; diff --git a/src/netbase.h b/src/netbase.h index dd33b6e47..d7587879e 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -1,4 +1,5 @@ // Copyright (c) 2009-2016 The Bitcoin Core developers +// Copyright (c) 2022 The Dogecoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -17,11 +18,11 @@ #include #include -extern int nConnectTimeout; +extern uint64_t nConnectTimeout; extern bool fNameLookup; //! -timeout default -static const int DEFAULT_CONNECT_TIMEOUT = 5000; +static const uint64_t DEFAULT_CONNECT_TIMEOUT = 5000; //! -dns default static const int DEFAULT_NAME_LOOKUP = true; @@ -39,7 +40,7 @@ public: enum Network ParseNetwork(std::string net); std::string GetNetworkName(enum Network net); -void SplitHostPort(std::string in, int &portOut, std::string &hostOut); +void SplitHostPort(std::string in, uint16_t &portOut, std::string &hostOut); bool SetProxy(enum Network net, const proxyType &addrProxy); bool GetProxy(enum Network net, proxyType &proxyInfoOut); bool IsProxy(const CNetAddr &addr); @@ -47,12 +48,12 @@ bool SetNameProxy(const proxyType &addrProxy); bool HaveNameProxy(); bool LookupHost(const char *pszName, std::vector& vIP, unsigned int nMaxSolutions, bool fAllowLookup); bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup); -bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLookup); -bool Lookup(const char *pszName, std::vector& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); -CService LookupNumeric(const char *pszName, int portDefault = 0); +bool Lookup(const char *pszName, CService& addr, uint16_t portDefault, bool fAllowLookup); +bool Lookup(const char *pszName, std::vector& vAddr, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions); +CService LookupNumeric(const char *pszName, uint16_t portDefault = 0); bool LookupSubNet(const char *pszName, CSubNet& subnet); -bool ConnectSocket(const CService &addr, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed = 0); -bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest, int portDefault, int nTimeout, bool *outProxyConnectionFailed = 0); +bool ConnectSocket(const CService &addr, SOCKET& hSocketRet, uint64_t nTimeout, bool *outProxyConnectionFailed = 0); +bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest, uint16_t portDefault, uint64_t nTimeout, bool *outProxyConnectionFailed = 0); /** Return readable error string for a network error code */ std::string NetworkErrorString(int err); /** Close socket and set hSocket to INVALID_SOCKET */ @@ -62,7 +63,7 @@ bool SetSocketNonBlocking(SOCKET& hSocket, bool fNonBlocking); /** * Convert milliseconds to a struct timeval for e.g. select. */ -struct timeval MillisToTimeval(int64_t nTimeout); +struct timeval MillisToTimeval(uint64_t nTimeout); void InterruptSocks5(bool interrupt); #endif // BITCOIN_NETBASE_H diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 1afef5b1c..eff344dfc 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -62,31 +62,31 @@ BOOST_AUTO_TEST_CASE(netbase_properties) } -bool static TestSplitHost(std::string test, std::string host, int port) +bool static TestSplitHost(std::string test, std::string host, uint16_t port) { std::string hostOut; - int portOut = -1; + uint16_t portOut = 0; SplitHostPort(test, portOut, hostOut); return hostOut == host && port == portOut; } BOOST_AUTO_TEST_CASE(netbase_splithost) { - BOOST_CHECK(TestSplitHost("www.bitcoin.org", "www.bitcoin.org", -1)); - BOOST_CHECK(TestSplitHost("[www.bitcoin.org]", "www.bitcoin.org", -1)); + BOOST_CHECK(TestSplitHost("www.bitcoin.org", "www.bitcoin.org", 0)); + BOOST_CHECK(TestSplitHost("[www.bitcoin.org]", "www.bitcoin.org", 0)); BOOST_CHECK(TestSplitHost("www.bitcoin.org:80", "www.bitcoin.org", 80)); BOOST_CHECK(TestSplitHost("[www.bitcoin.org]:80", "www.bitcoin.org", 80)); - BOOST_CHECK(TestSplitHost("127.0.0.1", "127.0.0.1", -1)); + BOOST_CHECK(TestSplitHost("127.0.0.1", "127.0.0.1", 0)); BOOST_CHECK(TestSplitHost("127.0.0.1:8333", "127.0.0.1", 8333)); - BOOST_CHECK(TestSplitHost("[127.0.0.1]", "127.0.0.1", -1)); + BOOST_CHECK(TestSplitHost("[127.0.0.1]", "127.0.0.1", 0)); BOOST_CHECK(TestSplitHost("[127.0.0.1]:8333", "127.0.0.1", 8333)); - BOOST_CHECK(TestSplitHost("::ffff:127.0.0.1", "::ffff:127.0.0.1", -1)); + BOOST_CHECK(TestSplitHost("::ffff:127.0.0.1", "::ffff:127.0.0.1", 0)); BOOST_CHECK(TestSplitHost("[::ffff:127.0.0.1]:8333", "::ffff:127.0.0.1", 8333)); BOOST_CHECK(TestSplitHost("[::]:8333", "::", 8333)); - BOOST_CHECK(TestSplitHost("::8333", "::8333", -1)); + BOOST_CHECK(TestSplitHost("::8333", "::8333", 0)); BOOST_CHECK(TestSplitHost(":8333", "", 8333)); BOOST_CHECK(TestSplitHost("[]:8333", "", 8333)); - BOOST_CHECK(TestSplitHost("", "", -1)); + BOOST_CHECK(TestSplitHost("", "", 0)); } bool static TestParse(std::string src, std::string canon)