highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-12 23:18:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge #17754: net: Don't allow resolving of std::string with embedded NUL characters. Add tests.

Browse Source 
7a046cdc14 tests: Avoid using C-style NUL-terminated strings as arguments (practicalswift)
fefb9165f2 tests: Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters (practicalswift)
9574de86ad net: Avoid using C-style NUL-terminated strings as arguments in the netbase interface (practicalswift)

Pull request description:

  Don't allow resolving of `std::string`:s with embedded `NUL` characters.

  Avoid using C-style `NUL`-terminated strings as arguments in the `netbase` interface

  Add tests.

  The only place in where C-style `NUL`-terminated strings are actually needed is here:

  ```diff
  +    if (!ValidAsCString(name)) {
  +        return false;
  +    }
  ...
  -    int nErr = getaddrinfo(pszName, nullptr, &aiHint, &aiRes);
  +    int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes);
       if (nErr)
           return false;
  ```

  Interface changes:

  ```diff
  -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);

  -bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup);
  +bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup);

  -bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLookup);
  +bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup);

  -bool Lookup(const char *pszName, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions);
  +bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions);

  -bool LookupSubNet(const char *pszName, CSubNet& subnet);
  +bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet);

  -CService LookupNumeric(const char *pszName, int portDefault = 0);
  +CService LookupNumeric(const std::string& name, int portDefault = 0);

  -bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed);
  +bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool& outProxyConnectionFailed);
  ```

  It should be noted that the `ConnectThroughProxy` change (from `bool *outProxyConnectionFailed` to `bool& outProxyConnectionFailed`) has nothing to do with `NUL` handling but I thought it was worth doing when touching this file :)

ACKs for top commit:
  EthanHeilman:
    ACK 7a046cdc14
  laanwj:
    ACK 7a046cdc14

Tree-SHA512: 66556e290db996917b54091acd591df221f72230f6b9f6b167b9195ee870ebef6e26f4cda2f6f54d00e1c362e1743bf56785d0de7cae854e6bf7d26f6caccaba
This commit is contained in:
Wladimir J. van der Laan
2020-01-22 20:14:12 +01:00
parent 04f78b818f 7a046cdc14
commit 1ae46dce60
13 changed files with 99 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/init.cpp
View File

@@ -1358,7 +1358,7 @@ bool AppInitMain(NodeContext& node)
SetReachable(NET_ONION, false);
if (proxyArg != "" && proxyArg != "0") {
CService proxyAddr;
if (!Lookup(proxyArg.c_str(), proxyAddr, 9050, fNameLookup)) {
if (!Lookup(proxyArg, proxyAddr, 9050, fNameLookup)) {
return InitError(strprintf(_("Invalid -proxy address or hostname: '%s'").translated, proxyArg));
}
@@ -1382,7 +1382,7 @@ bool AppInitMain(NodeContext& node)
SetReachable(NET_ONION, false);
} else {
CService onionProxy;
if (!Lookup(onionArg.c_str(), onionProxy, 9050, fNameLookup)) {
if (!Lookup(onionArg, onionProxy, 9050, fNameLookup)) {
return InitError(strprintf(_("Invalid -onion address or hostname: '%s'").translated, onionArg));
}
proxyType addrOnion = proxyType(onionProxy, proxyRandomize);
@@ -1400,7 +1400,7 @@ bool AppInitMain(NodeContext& node)
for (const std::string& strAddr : gArgs.GetArgs("-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));
@@ -1780,7 +1780,7 @@ bool AppInitMain(NodeContext& node)
for (const std::string& strBind : gArgs.GetArgs("-bind")) {
CService addrBind;
if (!Lookup(strBind.c_str(), addrBind, GetListenPort(), false)) {
if (!Lookup(strBind, addrBind, GetListenPort(), false)) {
return InitError(ResolveErrMsg("bind", strBind));
}
connOptions.vBinds.push_back(addrBind);