From 8c5e229ac90f8f880a4b2fb37ae187bc1cea5335 Mon Sep 17 00:00:00 2001 From: Pavol Rusnak Date: Tue, 28 Jul 2015 01:58:25 +0200 Subject: [PATCH 1/3] implement uacomment config parameter which can add comments to user agent as per BIP-0014 --- src/net.cpp | 2 +- src/rpcnet.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 961db1ff8..c7c1beac0 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -435,7 +435,7 @@ void CNode::PushVersion() else LogPrint("net", "send version message: version %d, blocks=%d, us=%s, peer=%d\n", PROTOCOL_VERSION, nBestHeight, addrMe.ToString(), id); PushMessage("version", PROTOCOL_VERSION, nLocalServices, nTime, addrYou, addrMe, - nLocalHostNonce, FormatSubVersion(CLIENT_NAME, CLIENT_VERSION, std::vector()), nBestHeight, true); + nLocalHostNonce, FormatSubVersion(CLIENT_NAME, CLIENT_VERSION, mapMultiArgs.count("-uacomment") ? mapMultiArgs["-uacomment"] : std::vector()), nBestHeight, true); } diff --git a/src/rpcnet.cpp b/src/rpcnet.cpp index 95fc8ff31..ec923e360 100644 --- a/src/rpcnet.cpp +++ b/src/rpcnet.cpp @@ -470,7 +470,7 @@ UniValue getnetworkinfo(const UniValue& params, bool fHelp) UniValue obj(UniValue::VOBJ); obj.push_back(Pair("version", CLIENT_VERSION)); obj.push_back(Pair("subversion", - FormatSubVersion(CLIENT_NAME, CLIENT_VERSION, std::vector()))); + FormatSubVersion(CLIENT_NAME, CLIENT_VERSION, mapMultiArgs.count("-uacomment") ? mapMultiArgs["-uacomment"] : std::vector()))); obj.push_back(Pair("protocolversion",PROTOCOL_VERSION)); obj.push_back(Pair("localservices", strprintf("%016x", nLocalServices))); obj.push_back(Pair("timeoffset", GetTimeOffset())); From 2bc62dc4e308e833c0535f69118906cac1dee84b Mon Sep 17 00:00:00 2001 From: Pavol Rusnak Date: Fri, 31 Jul 2015 18:05:42 +0200 Subject: [PATCH 2/3] limit total length of user agent comments Reworked-By: Wladimir J. van der Laan --- src/init.cpp | 7 +++++++ src/main.cpp | 2 +- src/net.cpp | 3 ++- src/net.h | 5 +++++ src/rpcnet.cpp | 3 +-- 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 7893afab9..cea1864ba 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1206,6 +1206,13 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) RegisterNodeSignals(GetNodeSignals()); + // format user agent, check total size + strSubVersion = FormatSubVersion(CLIENT_NAME, CLIENT_VERSION, mapMultiArgs.count("-uacomment") ? mapMultiArgs["-uacomment"] : std::vector()); + if (strSubVersion.size() > MAX_SUBVERSION_LENGTH) { + return InitError(strprintf("Total length of network version string %i exceeds maximum of %i characters. Reduce the number and/or size of uacomments.", + strSubVersion.size(), MAX_SUBVERSION_LENGTH)); + } + if (mapArgs.count("-onlynet")) { std::set nets; BOOST_FOREACH(const std::string& snet, mapMultiArgs["-onlynet"]) { diff --git a/src/main.cpp b/src/main.cpp index 587032547..c6658f3d7 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4768,7 +4768,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, if (!vRecv.empty()) vRecv >> addrFrom >> nNonce; if (!vRecv.empty()) { - vRecv >> LIMITED_STRING(pfrom->strSubVer, 256); + vRecv >> LIMITED_STRING(pfrom->strSubVer, MAX_SUBVERSION_LENGTH); pfrom->cleanSubVer = SanitizeString(pfrom->strSubVer); } if (!vRecv.empty()) diff --git a/src/net.cpp b/src/net.cpp index c7c1beac0..f605ecd06 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -73,6 +73,7 @@ static std::vector vhListenSocket; CAddrMan addrman; int nMaxConnections = DEFAULT_MAX_PEER_CONNECTIONS; bool fAddressesInitialized = false; +std::string strSubVersion; vector vNodes; CCriticalSection cs_vNodes; @@ -435,7 +436,7 @@ void CNode::PushVersion() else LogPrint("net", "send version message: version %d, blocks=%d, us=%s, peer=%d\n", PROTOCOL_VERSION, nBestHeight, addrMe.ToString(), id); PushMessage("version", PROTOCOL_VERSION, nLocalServices, nTime, addrYou, addrMe, - nLocalHostNonce, FormatSubVersion(CLIENT_NAME, CLIENT_VERSION, mapMultiArgs.count("-uacomment") ? mapMultiArgs["-uacomment"] : std::vector()), nBestHeight, true); + nLocalHostNonce, strSubVersion, nBestHeight, true); } diff --git a/src/net.h b/src/net.h index 55190baa5..16df97a19 100644 --- a/src/net.h +++ b/src/net.h @@ -49,6 +49,8 @@ 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 2 MiB is currently acceptable). */ static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 2 * 1024 * 1024; +/** Maximum length of strSubVer in `version` message */ +static const unsigned int MAX_SUBVERSION_LENGTH = 256; /** -listen default */ static const bool DEFAULT_LISTEN = true; /** The maximum number of entries in mapAskFor */ @@ -156,6 +158,9 @@ extern CCriticalSection cs_vAddedNodes; extern NodeId nLastNodeId; extern CCriticalSection cs_nLastNodeId; +/** Subversion as sent to the P2P network in `version` messages */ +extern std::string strSubVersion; + struct LocalServiceInfo { int nScore; int nPort; diff --git a/src/rpcnet.cpp b/src/rpcnet.cpp index ec923e360..0337e097b 100644 --- a/src/rpcnet.cpp +++ b/src/rpcnet.cpp @@ -469,8 +469,7 @@ UniValue getnetworkinfo(const UniValue& params, bool fHelp) UniValue obj(UniValue::VOBJ); obj.push_back(Pair("version", CLIENT_VERSION)); - obj.push_back(Pair("subversion", - FormatSubVersion(CLIENT_NAME, CLIENT_VERSION, mapMultiArgs.count("-uacomment") ? mapMultiArgs["-uacomment"] : std::vector()))); + obj.push_back(Pair("subversion", strSubVersion)); obj.push_back(Pair("protocolversion",PROTOCOL_VERSION)); obj.push_back(Pair("localservices", strprintf("%016x", nLocalServices))); obj.push_back(Pair("timeoffset", GetTimeOffset())); From 3c1db17064c39245c8488d79e19eb0973313f576 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 9 Sep 2015 14:24:56 +0200 Subject: [PATCH 3/3] [uacomment] Sanitize per BIP-0014 * SanitizeString() can be requested to be more strict * Throw error when SanitizeString() changes uacomments * Fix tests --- src/init.cpp | 11 +++++++++-- src/test/util_tests.cpp | 6 +++--- src/utilstrencodings.cpp | 17 ++++++++++------- src/utilstrencodings.h | 16 +++++++++++++++- 4 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index cea1864ba..c12bae5a1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1206,8 +1206,15 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) RegisterNodeSignals(GetNodeSignals()); - // format user agent, check total size - strSubVersion = FormatSubVersion(CLIENT_NAME, CLIENT_VERSION, mapMultiArgs.count("-uacomment") ? mapMultiArgs["-uacomment"] : std::vector()); + // sanitize comments per BIP-0014, format user agent and check total size + std::vector uacomments; + BOOST_FOREACH(string cmt, mapMultiArgs["-uacomment"]) + { + if (cmt != SanitizeString(cmt, SAFE_CHARS_UA_COMMENT)) + return InitError(strprintf("User Agent comment (%s) contains unsafe characters.", cmt)); + uacomments.push_back(SanitizeString(cmt, SAFE_CHARS_UA_COMMENT)); + } + strSubVersion = FormatSubVersion(CLIENT_NAME, CLIENT_VERSION, uacomments); if (strSubVersion.size() > MAX_SUBVERSION_LENGTH) { return InitError(strprintf("Total length of network version string %i exceeds maximum of %i characters. Reduce the number and/or size of uacomments.", strSubVersion.size(), MAX_SUBVERSION_LENGTH)); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index df8f8f43b..6eb5ce563 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -413,7 +413,7 @@ BOOST_AUTO_TEST_CASE(test_FormatSubVersion) comments.push_back(std::string("comment1")); std::vector comments2; comments2.push_back(std::string("comment1")); - comments2.push_back(std::string("comment2")); + comments2.push_back(SanitizeString(std::string("Comment2; .,_?@; !\"#$%&'()*+-/<=>[]\\^`{|}~"), SAFE_CHARS_UA_COMMENT)); // Semicolon is discouraged but not forbidden by BIP-0014 BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99900, std::vector()), std::string("/Test:0.9.99-beta1/")); BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99924, std::vector()), std::string("/Test:0.9.99-beta25/")); BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99925, std::vector()), std::string("/Test:0.9.99-rc1/")); @@ -423,8 +423,8 @@ BOOST_AUTO_TEST_CASE(test_FormatSubVersion) BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99999, std::vector()), std::string("/Test:0.9.99-49/")); BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99900, comments), std::string("/Test:0.9.99-beta1(comment1)/")); BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99950, comments), std::string("/Test:0.9.99(comment1)/")); - BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99900, comments2), std::string("/Test:0.9.99-beta1(comment1; comment2)/")); - BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99950, comments2), std::string("/Test:0.9.99(comment1; comment2)/")); + BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99900, comments2), std::string("/Test:0.9.99-beta1(comment1; Comment2; .,_?@; )/")); + BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99950, comments2), std::string("/Test:0.9.99(comment1; Comment2; .,_?@; )/")); } BOOST_AUTO_TEST_CASE(test_ParseFixedPoint) diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index 0a5fbb3d2..af09c5564 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -15,17 +15,20 @@ using namespace std; -string SanitizeString(const string& str) +static const string CHARS_ALPHA_NUM = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + +static const string SAFE_CHARS[] = +{ + CHARS_ALPHA_NUM + " .,;_/:?@()", // SAFE_CHARS_DEFAULT + CHARS_ALPHA_NUM + " .,;_?@" // SAFE_CHARS_UA_COMMENT +}; + +string SanitizeString(const string& str, int rule) { - /** - * safeChars chosen to allow simple messages/URLs/email addresses, but avoid anything - * even possibly remotely dangerous like & or > - */ - static string safeChars("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ01234567890 .,;_/:?@()"); string strResult; for (std::string::size_type i = 0; i < str.size(); i++) { - if (safeChars.find(str[i]) != std::string::npos) + if (SAFE_CHARS[rule].find(str[i]) != std::string::npos) strResult.push_back(str[i]); } return strResult; diff --git a/src/utilstrencodings.h b/src/utilstrencodings.h index ccdc6a76b..2375d0c4f 100644 --- a/src/utilstrencodings.h +++ b/src/utilstrencodings.h @@ -22,8 +22,22 @@ /** This is needed because the foreach macro can't get over the comma in pair */ #define PAIRTYPE(t1, t2) std::pair +/** Used by SanitizeString() */ +enum SafeChars +{ + SAFE_CHARS_DEFAULT, //!< The full set of allowed chars + SAFE_CHARS_UA_COMMENT //!< BIP-0014 subset +}; + std::string SanitizeFilename(const std::string& str); -std::string SanitizeString(const std::string& str); +/** +* Remove unsafe chars. Safe chars chosen to allow simple messages/URLs/email +* addresses, but avoid anything even possibly remotely dangerous like & or > +* @param[in] str The string to sanitize +* @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 HexInt(uint32_t val); uint32_t ParseHexToUInt32(const std::string& str); std::vector ParseHex(const char* psz);