From 903ac8d37c144c46dbd84880ddfa1b5ba25e3ab2 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 13 Jul 2017 08:25:38 +0000 Subject: [PATCH 1/4] net: Drop CNode::strSubVer since it isn't used or useful --- src/net.cpp | 1 - src/net.h | 13 ++++++------- src/net_processing.cpp | 1 - 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 301cf58b870..d09ebd53a40 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2718,7 +2718,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn nTimeOffset = 0; addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn; nVersion = 0; - strSubVer = ""; fWhitelisted = false; fOneShot = false; fAddnode = false; diff --git a/src/net.h b/src/net.h index dc25e7a5dd6..336be513cdf 100644 --- a/src/net.h +++ b/src/net.h @@ -55,7 +55,7 @@ static const unsigned int MAX_INV_SZ = 50000; static const unsigned int MAX_ADDR_TO_SEND = 1000; /** Maximum length of incoming protocol messages (no message over 4 MB is currently acceptable). */ static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 4 * 1000 * 1000; -/** Maximum length of strSubVer in `version` message */ +/** Maximum length of subversion in `version` message */ static const unsigned int MAX_SUBVERSION_LENGTH = 256; /** Maximum number of automatic outgoing nodes */ static const int MAX_OUTBOUND_CONNECTIONS = 8; @@ -595,12 +595,11 @@ class CNode // Bind address of our side of the connection const CAddress addrBind; std::atomic nVersion; - // strSubVer is whatever byte array we read from the wire. However, this field is intended - // to be printed out, displayed to humans in various forms and so on. So we sanitize it and - // store the sanitized version in cleanSubVer. The original should be used when dealing with - // the network or wire types and the cleaned string used when displayed or logged. - std::string strSubVer, cleanSubVer; - CCriticalSection cs_SubVer; // used for both cleanSubVer and strSubVer + // The subversion is intended to be printed out, displayed to humans in + // various forms and so on. So we sanitize it and store the sanitized + // version in cleanSubVer. Additional escaping is done when logging. + std::string cleanSubVer; + CCriticalSection cs_SubVer; // used for cleanSubVer bool fWhitelisted; // This peer can bypass DoS banning. bool fFeeler; // If true this node is being used as a short lived feeler. bool fOneShot; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 4d832f37113..0465ac30fbf 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1305,7 +1305,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr pfrom->SetAddrLocal(addrMe); { LOCK(pfrom->cs_SubVer); - pfrom->strSubVer = strSubVer; pfrom->cleanSubVer = cleanSubVer; } pfrom->nStartingHeight = nStartingHeight; From 1e588b4df2c9a46f087fdf4b936c4847dbeccf06 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 13 Jul 2017 08:26:56 +0000 Subject: [PATCH 2/4] SanitizeString: Support optional escaping of disallowed characters --- src/utilstrencodings.cpp | 7 +++++-- src/utilstrencodings.h | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index 93abaec04b1..796bb431817 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -21,13 +21,16 @@ static const std::string SAFE_CHARS[] = CHARS_ALPHA_NUM + ".-_", // SAFE_CHARS_FILENAME }; -std::string SanitizeString(const std::string& str, int rule) +std::string SanitizeString(const std::string& str, int rule, bool escape) { std::string strResult; for (std::string::size_type i = 0; i < str.size(); i++) { - if (SAFE_CHARS[rule].find(str[i]) != std::string::npos) + if (SAFE_CHARS[rule].find(str[i]) != std::string::npos) { strResult.push_back(str[i]); + } else if (escape) { + strResult += strprintf("%%%02X", str[i]); + } } return strResult; } diff --git a/src/utilstrencodings.h b/src/utilstrencodings.h index 8b37fe12e08..baf3c72ac79 100644 --- a/src/utilstrencodings.h +++ b/src/utilstrencodings.h @@ -34,7 +34,7 @@ enum SafeChars * @param[in] rule The set of safe chars to choose (default: least restrictive) * @return A new string without unsafe chars */ -std::string SanitizeString(const std::string& str, int rule = SAFE_CHARS_DEFAULT); +std::string SanitizeString(const std::string& str, int rule = SAFE_CHARS_DEFAULT, bool escape = false); std::vector ParseHex(const char* psz); std::vector ParseHex(const std::string& str); signed char HexDigit(char c); From 69577aec48ce51bf67e885f858e30b08c50b1dac Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Mon, 3 Jul 2017 09:56:35 +0000 Subject: [PATCH 3/4] net_processing: Escape rather than remove any printable characters in UAs --- src/net_processing.cpp | 4 ++-- src/utilstrencodings.cpp | 1 + src/utilstrencodings.h | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0465ac30fbf..58e266a9651 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1275,7 +1275,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr vRecv >> addrFrom >> nNonce; if (!vRecv.empty()) { vRecv >> LIMITED_STRING(strSubVer, MAX_SUBVERSION_LENGTH); - cleanSubVer = SanitizeString(strSubVer); + cleanSubVer = SanitizeString(strSubVer, SAFE_CHARS_PRINTABLE); } if (!vRecv.empty()) { vRecv >> nStartingHeight; @@ -1362,7 +1362,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr remoteAddr = ", peeraddr=" + pfrom->addr.ToString(); LogPrintf("receive version message: %s: version %d, blocks=%d, us=%s, peer=%d%s\n", - cleanSubVer, pfrom->nVersion, + SanitizeString(cleanSubVer, SAFE_CHARS_DEFAULT, true), pfrom->nVersion, pfrom->nStartingHeight, addrMe.ToString(), pfrom->GetId(), remoteAddr); diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index 796bb431817..77fc74a5ef0 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_PRINTABLE }; std::string SanitizeString(const std::string& str, int rule, bool escape) diff --git a/src/utilstrencodings.h b/src/utilstrencodings.h index baf3c72ac79..19ebbe96fbf 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_PRINTABLE, //!< The full set of printable chars }; /** From c1ff31f0e2d81dedbfe1f5b0916f9b1cc4835f60 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Mon, 17 Jul 2017 06:30:13 +0000 Subject: [PATCH 4/4] SanitizeString: If escaping, ensure '%' itself is always escaped --- src/utilstrencodings.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index 77fc74a5ef0..edf848da8f7 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -27,7 +27,7 @@ std::string SanitizeString(const std::string& str, int rule, bool escape) std::string strResult; for (std::string::size_type i = 0; i < str.size(); i++) { - if (SAFE_CHARS[rule].find(str[i]) != std::string::npos) { + if (SAFE_CHARS[rule].find(str[i]) != std::string::npos || (str[i] == '%' && escape)) { strResult.push_back(str[i]); } else if (escape) { strResult += strprintf("%%%02X", str[i]);