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

7a046cdc1423963bdcbcf9bb98560af61fa90b37 tests: Avoid using C-style NUL-terminated strings as arguments (practicalswift)
fefb9165f23fe9d10ad092ec31715f906e0d2ee7 tests: Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters (practicalswift)
9574de86ad703ad942cdd0eca79f48c0d42b102b 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 7a046cdc1423963bdcbcf9bb98560af61fa90b37
  laanwj:
    ACK 7a046cdc1423963bdcbcf9bb98560af61fa90b37

Tree-SHA512: 66556e290db996917b54091acd591df221f72230f6b9f6b167b9195ee870ebef6e26f4cda2f6f54d00e1c362e1743bf56785d0de7cae854e6bf7d26f6caccaba

2 files changed, 2 insertions, 2 deletions

diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp
index 2f612664df..8ee6c947e6 100644
--- a/src/qt/optionsdialog.cpp
+++ b/src/qt/optionsdialog.cpp
@@ -375,7 +375,7 @@ QValidator::State ProxyAddressValidator::validate(QString &input, int &pos) cons
 {
     Q_UNUSED(pos);
     // Validate the proxy
-    CService serv(LookupNumeric(input.toStdString().c_str(), DEFAULT_GUI_PROXY_PORT));
+    CService serv(LookupNumeric(input.toStdString(), DEFAULT_GUI_PROXY_PORT));
     proxyType addrProxy = proxyType(serv, true);
     if (addrProxy.IsValid())
         return QValidator::Acceptable;
diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index 31c9e65140..e1f783b0e5 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -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() && m_node.unban(possibleSubnet))
         {
             clientModel->getBanTableModel()->refresh();