Merge #14835: [0.17] Further Backports

f9db08e8ca qa: Avoid race in p2p_invalid_block by waiting for the block request (MarcoFalke)
79358817e5 Add SAFE_CHARS[SAFE_CHARS_URI]: Chars allowed in URIs (RFC 3986) (practicalswift)
9666dbaf09 rpc: Make HTTP RPC debug logging more informative (practicalswift)
b90157891a add test demonstrating addrLocal UB (Kaz Wesley)
6f04264bbb fix uninitialized read when stringifying an addrLocal (Kaz Wesley)
5782fdcd8c Throw error if CPubKey is invalid during PSBT keypath serialization (Gregory Sanders)
de5e48a461 Bugfix: RPC: Add address_type named param for createmultisig (Luke Dashjr)
df5131bd41 gui: explicitly disable "Dark Mode" appearance on macOS (fanquake)

Pull request description:

  Backports #14593, #14596, #14618, #14690 and #14728 to the 0.17 branch.

Tree-SHA512: fcda4b75fcb71bb80cc8bde2a2b98ff5c0239dfa754ac980b1a91a90409502ac7678326399a4fc03a773074339dbf8b3d11750c91fe4302741a954745acfcca1

8 files changed, 52 insertions, 6 deletions

diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 8962fe6a42..2a76d0d46a 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -224,21 +224,25 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
     }
     std::unique_ptr<HTTPRequest> hreq(new HTTPRequest(req));
 
-    LogPrint(BCLog::HTTP, "Received a %s request for %s from %s\n",
-             RequestMethodString(hreq->GetRequestMethod()), hreq->GetURI(), hreq->GetPeer().ToString());
-
     // Early address-based allow check
     if (!ClientAllowed(hreq->GetPeer())) {
+        LogPrint(BCLog::HTTP, "HTTP request from %s rejected: Client network is not allowed RPC access\n",
+                 hreq->GetPeer().ToString());
         hreq->WriteReply(HTTP_FORBIDDEN);
         return;
     }
 
     // Early reject unknown HTTP methods
     if (hreq->GetRequestMethod() == HTTPRequest::UNKNOWN) {
+        LogPrint(BCLog::HTTP, "HTTP request from %s rejected: Unknown HTTP request method\n",
+                 hreq->GetPeer().ToString());
         hreq->WriteReply(HTTP_BADMETHOD);
         return;
     }
 
+    LogPrint(BCLog::HTTP, "Received a %s request for %s from %s\n",
+             RequestMethodString(hreq->GetRequestMethod()), SanitizeString(hreq->GetURI(), SAFE_CHARS_URI).substr(0, 100), hreq->GetPeer().ToString());
+
     // Find registered handler for prefix
     std::string strURI = hreq->GetURI();
     std::string path;
diff --git a/src/netaddress.cpp b/src/netaddress.cpp
index 778c2700f9..9c6daefef6 100644
--- a/src/netaddress.cpp
+++ b/src/netaddress.cpp
@@ -17,7 +17,6 @@ static const unsigned char g_internal_prefix[] = { 0xFD, 0x6B, 0x88, 0xC0, 0x87,
 CNetAddr::CNetAddr()
 {
     memset(ip, 0, sizeof(ip));
-    scopeId = 0;
 }
 
 void CNetAddr::SetIP(const CNetAddr& ipIn)
diff --git a/src/netaddress.h b/src/netaddress.h
index cc0e4d4f12..dc55d8b1a8 100644
--- a/src/netaddress.h
+++ b/src/netaddress.h
@@ -33,7 +33,7 @@ class CNetAddr
 {
     protected:
         unsigned char ip[16]; // in network byte order
-        uint32_t scopeId; // for scoped/link-local ipv6 addresses
+        uint32_t scopeId{0}; // for scoped/link-local ipv6 addresses
 
     public:
         CNetAddr();
diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
index 0644d7c6c2..88d662878e 100644
--- a/src/rpc/misc.cpp
+++ b/src/rpc/misc.cpp
@@ -474,7 +474,7 @@ static const CRPCCommand commands[] =
     { "control",            "getmemoryinfo",          &getmemoryinfo,          {"mode"} },
     { "control",            "logging",                &logging,                {"include", "exclude"}},
     { "util",               "validateaddress",        &validateaddress,        {"address"} }, /* uses wallet if enabled */
-    { "util",               "createmultisig",         &createmultisig,         {"nrequired","keys"} },
+    { "util",               "createmultisig",         &createmultisig,         {"nrequired","keys","address_type"} },
     { "util",               "verifymessage",          &verifymessage,          {"address","signature","message"} },
     { "util",               "signmessagewithprivkey", &signmessagewithprivkey, {"privkey","message"} },
 
diff --git a/src/script/sign.h b/src/script/sign.h
index 7ade715ee2..245b15410f 100644
--- a/src/script/sign.h
+++ b/src/script/sign.h
@@ -188,6 +188,9 @@ template<typename Stream>
 void SerializeHDKeypaths(Stream& s, const std::map<CPubKey, std::vector<uint32_t>>& hd_keypaths, uint8_t type)
 {
     for (auto keypath_pair : hd_keypaths) {
+        if (!keypath_pair.first.IsValid()) {
+            throw std::ios_base::failure("Invalid CPubKey being serialized");
+        }
         SerializeToVector(s, type, MakeSpan(keypath_pair.first));
         WriteCompactSize(s, keypath_pair.second.size() * sizeof(uint32_t));
         for (auto& path : keypath_pair.second) {
diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index 4c56938ec9..ae4841de5a 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -189,4 +189,42 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
     BOOST_CHECK(pnode2->fFeeler == false);
 }
 
+// prior to PR #14728, this test triggers an undefined behavior
+BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
+{
+    // set up local addresses; all that's necessary to reproduce the bug is
+    // that a normal IPv4 address is among the entries, but if this address is
+    // !IsRoutable the undefined behavior is easier to trigger deterministically
+    {
+        LOCK(cs_mapLocalHost);
+        in_addr ipv4AddrLocal;
+        ipv4AddrLocal.s_addr = 0x0100007f;
+        CNetAddr addr = CNetAddr(ipv4AddrLocal);
+        LocalServiceInfo lsi;
+        lsi.nScore = 23;
+        lsi.nPort = 42;
+        mapLocalHost[addr] = lsi;
+    }
+
+    // create a peer with an IPv4 address
+    in_addr ipv4AddrPeer;
+    ipv4AddrPeer.s_addr = 0xa0b0c001;
+    CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK);
+    std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, 0, INVALID_SOCKET, addr, 0, 0, CAddress{}, std::string{}, false);
+    pnode->fSuccessfullyConnected.store(true);
+
+    // the peer claims to be reaching us via IPv6
+    in6_addr ipv6AddrLocal;
+    memset(ipv6AddrLocal.s6_addr, 0, 16);
+    ipv6AddrLocal.s6_addr[0] = 0xcc;
+    CAddress addrLocal = CAddress(CService(ipv6AddrLocal, 7777), NODE_NETWORK);
+    pnode->SetAddrLocal(addrLocal);
+
+    // before patch, this causes undefined behavior detectable with clang's -fsanitize=memory
+    AdvertiseLocal(&*pnode);
+
+    // suppress no-checks-run warning; if this test fails, it's by triggering a sanitizer
+    BOOST_CHECK(1);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp
index a06d88cb19..3c6e333a62 100644
--- a/src/utilstrencodings.cpp
+++ b/src/utilstrencodings.cpp
@@ -19,6 +19,7 @@ static const std::string SAFE_CHARS[] =
     CHARS_ALPHA_NUM + " .,;-_/:?@()", // SAFE_CHARS_DEFAULT
     CHARS_ALPHA_NUM + " .,;-_?@", // SAFE_CHARS_UA_COMMENT
     CHARS_ALPHA_NUM + ".-_", // SAFE_CHARS_FILENAME
+    CHARS_ALPHA_NUM + "!*'();:@&=+$,/?#[]-_.~%", // SAFE_CHARS_URI
 };
 
 std::string SanitizeString(const std::string& str, int rule)
diff --git a/src/utilstrencodings.h b/src/utilstrencodings.h
index 5f2211b5dc..0a06bc3f85 100644
--- a/src/utilstrencodings.h
+++ b/src/utilstrencodings.h
@@ -25,6 +25,7 @@ enum SafeChars
     SAFE_CHARS_DEFAULT, //!< The full set of allowed chars
     SAFE_CHARS_UA_COMMENT, //!< BIP-0014 subset
     SAFE_CHARS_FILENAME, //!< Chars allowed in filenames
+    SAFE_CHARS_URI, //!< Chars allowed in URIs (RFC 3986)
 };
 
 /**