Escape rather than remove any printable characters in UAs

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;

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<int> 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;

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;
@@ -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;
@@ -1363,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);
 

src/utilstrencodings.cpp

@@ -19,15 +19,19 @@ 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)
+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]);
+        }
     }
     return strResult;
 }

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
 };
 
 /**
@@ -34,7 +35,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<unsigned char> ParseHex(const char* psz);
 std::vector<unsigned char> ParseHex(const std::string& str);
 signed char HexDigit(char c);