net: avoid using C-style NUL-terminated strings in interfaces

Minimizes the use of c_str() in netbase interfaces, by using
std::string when we're passing arguments instead, and only
converting to a C-style string when interfacing with
getaddrinfo.

Introduces attributes.h for definition of NODISCARD macro
Introduces utilstring.h for definition of ValidAsCString()

Backported from: 9cc0230c (partial), d945c6f5 and 9574de8
Original Author: practicalswift <practicalswift@users.noreply.github.com>
This commit is contained in:
Patrick Lodder 2022-10-29 19:56:16 +02:00
parent f01e36c421
commit 6469f6bea7
No known key found for this signature in database
GPG Key ID: 2D3A345B98D0DC1F
13 changed files with 118 additions and 48 deletions

View File

@ -78,6 +78,7 @@ endif
BITCOIN_CORE_H = \
addrdb.h \
addrman.h \
attributes.h \
auxpow.h \
base58.h \
bloom.h \
@ -156,6 +157,7 @@ BITCOIN_CORE_H = \
util.h \
utilmoneystr.h \
utiltime.h \
utilstr.h \
validation.h \
validationinterface.h \
versionbits.h \

22
src/attributes.h Normal file
View File

@ -0,0 +1,22 @@
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2018 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#ifndef BITCOIN_ATTRIBUTES_H
#define BITCOIN_ATTRIBUTES_H
#if defined(__has_cpp_attribute)
# if __has_cpp_attribute(nodiscard)
# define NODISCARD [[nodiscard]]
# endif
#endif
#ifndef NODISCARD
# if defined(_MSC_VER) && _MSC_VER >= 1700
# define NODISCARD _Check_return_
# else
# define NODISCARD __attribute__((warn_unused_result))
# endif
#endif
#endif // BITCOIN_ATTRIBUTES_H

View File

@ -209,7 +209,7 @@ static bool InitHTTPAllowList()
const std::vector<std::string>& vAllow = mapMultiArgs.at("-rpcallowip");
for (std::string strAllow : vAllow) {
CSubNet subnet;
LookupSubNet(strAllow.c_str(), subnet);
LookupSubNet(strAllow, subnet);
if (!subnet.IsValid()) {
uiInterface.ThreadSafeMessageBox(
strprintf("Invalid -rpcallowip subnet specification: %s. Valid are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24).", strAllow),

View File

@ -1324,7 +1324,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
std::string proxyArg = GetArg("-proxy", "");
SetLimited(NET_TOR);
if (proxyArg != "" && proxyArg != "0") {
CService resolved(LookupNumeric(proxyArg.c_str(), 9050));
CService resolved(LookupNumeric(proxyArg, 9050));
proxyType addrProxy = proxyType(resolved, proxyRandomize);
if (!addrProxy.IsValid())
return InitError(strprintf(_("Invalid -proxy address: '%s'"), proxyArg));
@ -1344,7 +1344,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
if (onionArg == "0") { // Handle -noonion/-onion=0
SetLimited(NET_TOR); // set onions as unreachable
} else {
CService resolved(LookupNumeric(onionArg.c_str(), 9050));
CService resolved(LookupNumeric(onionArg, 9050));
proxyType addrOnion = proxyType(resolved, proxyRandomize);
if (!addrOnion.IsValid())
return InitError(strprintf(_("Invalid -onion address: '%s'"), onionArg));
@ -1364,7 +1364,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
if (mapMultiArgs.count("-bind")) {
BOOST_FOREACH(const std::string& strBind, mapMultiArgs.at("-bind")) {
CService addrBind;
if (!Lookup(strBind.c_str(), addrBind, GetListenPort(), false))
if (!Lookup(strBind, addrBind, GetListenPort(), false))
return InitError(ResolveErrMsg("bind", strBind));
fBound |= Bind(connman, addrBind, (BF_EXPLICIT | BF_REPORT_ERROR));
}
@ -1372,7 +1372,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
if (mapMultiArgs.count("-whitebind")) {
BOOST_FOREACH(const std::string& strBind, mapMultiArgs.at("-whitebind")) {
CService addrBind;
if (!Lookup(strBind.c_str(), addrBind, 0, false))
if (!Lookup(strBind, addrBind, 0, false))
return InitError(ResolveErrMsg("whitebind", strBind));
if (addrBind.GetPort() == 0)
return InitError(strprintf(_("Need to specify a port with -whitebind: '%s'"), strBind));
@ -1392,7 +1392,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
if (mapMultiArgs.count("-externalip")) {
BOOST_FOREACH(const std::string& strAddr, mapMultiArgs.at("-externalip")) {
CService addrLocal;
if (Lookup(strAddr.c_str(), addrLocal, GetListenPort(), fNameLookup) && addrLocal.IsValid())
if (Lookup(strAddr, addrLocal, GetListenPort(), fNameLookup) && addrLocal.IsValid())
AddLocal(addrLocal, LOCAL_MANUAL);
else
return InitError(ResolveErrMsg("externalip", strAddr));

View File

@ -1502,7 +1502,7 @@ void ThreadMapPort()
{
CNetAddr resolved;
if(LookupHost(externalIPAddress, resolved, false)) {
LogPrintf("UPnP: ExternalIPAddress = %s\n", resolved.ToString().c_str());
LogPrintf("UPnP: ExternalIPAddress = %s\n", resolved.ToString());
AddLocal(resolved, LOCAL_UPNP);
}
}
@ -1633,7 +1633,7 @@ void CConnman::ThreadDNSAddressSeed()
std::vector<CNetAddr> vIPs;
std::vector<CAddress> vAdd;
ServiceFlags requiredServiceBits = nRelevantServices;
if (LookupHost(GetDNSHost(seed, &requiredServiceBits).c_str(), vIPs, 0, true))
if (LookupHost(GetDNSHost(seed, &requiredServiceBits), vIPs, 0, true))
{
BOOST_FOREACH(const CNetAddr& ip, vIPs)
{
@ -1653,7 +1653,7 @@ void CConnman::ThreadDNSAddressSeed()
// resolve is not required at all.
if (!vIPs.empty()) {
CService seedSource;
Lookup(seed.name.c_str(), seedSource, 0, true);
Lookup(seed.name, seedSource, 0, true);
addrman.Add(vAdd, seedSource);
}
}
@ -1910,7 +1910,7 @@ std::vector<AddedNodeInfo> CConnman::GetAddedNodeInfo()
}
BOOST_FOREACH(const std::string& strAddNode, lAddresses) {
CService service(LookupNumeric(strAddNode.c_str(), Params().GetDefaultPort()));
CService service(LookupNumeric(strAddNode, Params().GetDefaultPort()));
if (service.IsValid()) {
// strAddNode is an IP:port
auto it = mapConnected.find(service);
@ -1956,7 +1956,7 @@ void CConnman::ThreadOpenAddedConnections()
// If strAddedNode is an IP/port, decode it immediately, so
// OpenNetworkConnection can detect existing connections to that IP/port.
tried = true;
CService service(LookupNumeric(info.strAddedNode.c_str(), Params().GetDefaultPort()));
CService service(LookupNumeric(info.strAddedNode, Params().GetDefaultPort()));
OpenNetworkConnection(CAddress(service, NODE_NONE), false, &grant, info.strAddedNode.c_str(), false, false, true);
if (!interruptNet.sleep_for(std::chrono::milliseconds(500)))
return;

View File

@ -16,6 +16,7 @@
#include "random.h"
#include "util.h"
#include "utilstrencodings.h"
#include "utilstring.h"
#include <atomic>
@ -78,13 +79,17 @@ void SplitHostPort(std::string in, uint16_t &portOut, std::string &hostOut) {
hostOut = in;
}
bool static LookupIntern(const char *pszName, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup)
bool static LookupIntern(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup)
{
vIP.clear();
if (!ValidAsCString(name)) {
return false;
}
{
CNetAddr addr;
if (addr.SetSpecial(std::string(pszName))) {
if (addr.SetSpecial(name)) {
vIP.push_back(addr);
return true;
}
@ -102,7 +107,7 @@ bool static LookupIntern(const char *pszName, std::vector<CNetAddr>& vIP, unsign
aiHint.ai_flags = fAllowLookup ? AI_ADDRCONFIG : AI_NUMERICHOST;
#endif
struct addrinfo *aiRes = NULL;
int nErr = getaddrinfo(pszName, NULL, &aiHint, &aiRes);
int nErr = getaddrinfo(name.c_str(), NULL, &aiHint, &aiRes);
if (nErr)
return false;
@ -130,9 +135,12 @@ bool static LookupIntern(const char *pszName, std::vector<CNetAddr>& vIP, unsign
return (vIP.size() > 0);
}
bool LookupHost(const char *pszName, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup)
bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup)
{
std::string strHost(pszName);
if (!ValidAsCString(name)) {
return false;
}
std::string strHost = name;
if (strHost.empty())
return false;
if (boost::algorithm::starts_with(strHost, "[") && boost::algorithm::ends_with(strHost, "]"))
@ -140,29 +148,34 @@ bool LookupHost(const char *pszName, std::vector<CNetAddr>& vIP, unsigned int nM
strHost = strHost.substr(1, strHost.size() - 2);
}
return LookupIntern(strHost.c_str(), vIP, nMaxSolutions, fAllowLookup);
return LookupIntern(strHost, vIP, nMaxSolutions, fAllowLookup);
}
bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup)
bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup)
{
if (!ValidAsCString(name)) {
return false;
}
std::vector<CNetAddr> vIP;
LookupHost(pszName, vIP, 1, fAllowLookup);
LookupHost(name, vIP, 1, fAllowLookup);
if(vIP.empty())
return false;
addr = vIP.front();
return true;
}
bool Lookup(const char *pszName, std::vector<CService>& vAddr, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions)
bool Lookup(const std::string& name, std::vector<CService>& vAddr, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions)
{
if (pszName[0] == 0)
if (name.empty() || !ValidAsCString(name)) {
return false;
}
uint16_t port = portDefault;
std::string hostname = "";
SplitHostPort(std::string(pszName), port, hostname);
std::string hostname;
SplitHostPort(name, port, hostname);
std::vector<CNetAddr> vIP;
bool fRet = LookupIntern(hostname.c_str(), vIP, nMaxSolutions, fAllowLookup);
bool fRet = LookupIntern(hostname, vIP, nMaxSolutions, fAllowLookup);
if (!fRet)
return false;
vAddr.resize(vIP.size());
@ -171,22 +184,28 @@ bool Lookup(const char *pszName, std::vector<CService>& vAddr, uint16_t portDefa
return true;
}
bool Lookup(const char *pszName, CService& addr, uint16_t portDefault, bool fAllowLookup)
bool Lookup(const std::string& name, CService& addr, uint16_t portDefault, bool fAllowLookup)
{
if (!ValidAsCString(name)) {
return false;
}
std::vector<CService> vService;
bool fRet = Lookup(pszName, vService, portDefault, fAllowLookup, 1);
bool fRet = Lookup(name, vService, portDefault, fAllowLookup, 1);
if (!fRet)
return false;
addr = vService[0];
return true;
}
CService LookupNumeric(const char *pszName, uint16_t portDefault)
CService LookupNumeric(const std::string& name, uint16_t portDefault)
{
if (!ValidAsCString(name)) {
return {};
}
CService addr;
// "1.2:345" will fail to resolve the ip, but will still set the port.
// If the ip fails to resolve, re-init the result.
if(!Lookup(pszName, addr, portDefault, false))
if(!Lookup(name, addr, portDefault, false))
addr = CService();
return addr;
}
@ -591,7 +610,7 @@ bool ConnectSocket(const CService &addrDest, SOCKET& hSocketRet, uint64_t nTimeo
return ConnectSocketDirectly(addrDest, hSocketRet, nTimeout);
}
bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest, uint16_t portDefault, uint64_t nTimeout, bool *outProxyConnectionFailed)
bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const std::string& name, uint16_t portDefault, uint64_t nTimeout, bool *outProxyConnectionFailed)
{
std::string strDest;
uint16_t port = portDefault;
@ -599,13 +618,13 @@ bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest
if (outProxyConnectionFailed)
*outProxyConnectionFailed = false;
SplitHostPort(std::string(pszDest), port, strDest);
SplitHostPort(name, port, strDest);
proxyType proxy;
GetNameProxy(proxy);
std::vector<CService> addrResolved;
if (Lookup(strDest.c_str(), addrResolved, port, fNameLookup && !HaveNameProxy(), 256)) {
if (Lookup(strDest, addrResolved, port, fNameLookup && !HaveNameProxy(), 256)) {
if (addrResolved.size() > 0) {
addr = addrResolved[GetRand(addrResolved.size())];
return ConnectSocket(addr, hSocketRet, nTimeout);
@ -619,14 +638,18 @@ bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const char *pszDest
return ConnectThroughProxy(proxy, strDest, port, hSocketRet, nTimeout, outProxyConnectionFailed);
}
bool LookupSubNet(const char* pszName, CSubNet& ret)
bool LookupSubNet(const std::string& strSubnet, CSubNet& ret)
{
std::string strSubnet(pszName);
if (!ValidAsCString(strSubnet)) {
return false;
}
size_t slash = strSubnet.find_last_of('/');
std::vector<CNetAddr> vIP;
std::string strAddress = strSubnet.substr(0, slash);
if (LookupHost(strAddress.c_str(), vIP, 1, false))
// TODO: Use LookupHost(const std::string&, CNetAddr&, bool) instead to just get
// one CNetAddr.
if (LookupHost(strAddress, vIP, 1, false))
{
CNetAddr network = vIP[0];
if (slash != strSubnet.npos)
@ -641,7 +664,7 @@ bool LookupSubNet(const char* pszName, CSubNet& ret)
else // If not a valid number, try full netmask syntax
{
// Never allow lookup for netmask
if (LookupHost(strNetmask.c_str(), vIP, 1, false)) {
if (LookupHost(strNetmask, vIP, 1, false)) {
ret = CSubNet(network, vIP[0]);
return ret.IsValid();
}

View File

@ -46,14 +46,16 @@ bool GetProxy(enum Network net, proxyType &proxyInfoOut);
bool IsProxy(const CNetAddr &addr);
bool SetNameProxy(const proxyType &addrProxy);
bool HaveNameProxy();
bool LookupHost(const char *pszName, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup);
bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup);
bool Lookup(const char *pszName, CService& addr, uint16_t portDefault, bool fAllowLookup);
bool Lookup(const char *pszName, std::vector<CService>& 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 LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup);
bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup);
bool Lookup(const std::string& name, CService& addr, uint16_t portDefault, bool fAllowLookup);
bool Lookup(const std::string& name, std::vector<CService>& vAddr, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions);
CService LookupNumeric(const std::string& name, uint16_t portDefault = 0);
bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet);
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);
bool ConnectSocketByName(CService &addr, SOCKET& hSocketRet, const std::string& name, 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 */

View File

@ -348,7 +348,7 @@ QValidator::State ProxyAddressValidator::validate(QString &input, int &pos) cons
{
Q_UNUSED(pos);
// Validate the proxy
CService serv(LookupNumeric(input.toStdString().c_str(), 9050));
CService serv(LookupNumeric(input.toStdString(), 9050));
proxyType addrProxy = proxyType(serv, true);
if (addrProxy.IsValid())
return QValidator::Acceptable;

View File

@ -1236,7 +1236,7 @@ void RPCConsole::unbanSelectedNode()
QString strNode = nodes.at(i).data().toString();
CSubNet possibleSubnet;
LookupSubNet(strNode.toStdString().c_str(), possibleSubnet);
LookupSubNet(strNode.toStdString(), possibleSubnet);
if (possibleSubnet.IsValid() && g_connman)
{
g_connman->Unban(possibleSubnet);

View File

@ -546,11 +546,11 @@ UniValue setban(const JSONRPCRequest& request)
if (!isSubnet) {
CNetAddr resolved;
LookupHost(request.params[0].get_str().c_str(), resolved, false);
LookupHost(request.params[0].get_str(), resolved, false);
netAddr = resolved;
}
else
LookupSubNet(request.params[0].get_str().c_str(), subNet);
LookupSubNet(request.params[0].get_str(), subNet);
if (! (isSubnet ? subNet.IsValid() : netAddr.IsValid()) )
throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Invalid IP/Subnet");

View File

@ -92,7 +92,7 @@ BOOST_AUTO_TEST_CASE(netbase_splithost)
bool static TestParse(std::string src, std::string canon)
{
CService addr(LookupNumeric(src.c_str(), 65535));
CService addr(LookupNumeric(src, 65535));
return canon == addr.ToString();
}

View File

@ -440,7 +440,7 @@ void TorController::add_onion_cb(TorControlConnection& _conn, const TorControlRe
if ((i = m.find("PrivateKey")) != m.end())
private_key = i->second;
}
service = LookupNumeric(std::string(service_id+".onion").c_str(), GetListenPort());
service = LookupNumeric(std::string(service_id+".onion"), GetListenPort());
LogPrintf("tor: Got service ID %s, advertising service %s\n", service_id, service.ToString());
if (WriteBinaryFile(GetPrivateKeyFile(), private_key)) {
LogPrint("tor", "tor: Cached service private key to %s\n", GetPrivateKeyFile());

21
src/utilstring.h Normal file
View File

@ -0,0 +1,21 @@
// Copyright (c) 2019 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#ifndef BITCOIN_UTIL_STRING_H
#define BITCOIN_UTIL_STRING_H
#include "attributes.h"
#include <cstring>
#include <string>
/**
* Check if a string does not contain any embedded NUL (\0) characters
*/
NODISCARD inline bool ValidAsCString(const std::string& str) noexcept
{
return str.size() == strlen(str.c_str());
}
#endif // BITCOIN_UTIL_STRING_H