From b4e1b3c1b1314646a7856d85d829bed649396cd1 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Wed, 21 Mar 2018 16:25:14 -0400 Subject: [PATCH] Remove undefined behavior from calls: For the functions defined in the C standard requires that the value of the int argument be in the range of an unsigned char, or be EOF. Violation of this requirement results in undefined behavior. --- src/ripple/basics/StringUtilities.h | 3 +++ src/ripple/basics/base_uint.h | 14 +++++----- src/ripple/basics/impl/ResolverAsio.cpp | 2 +- src/ripple/basics/impl/StringUtilities.cpp | 9 +++++++ src/ripple/beast/core/LexicalCast.h | 6 ++++- src/ripple/beast/core/osx_ObjCHelpers.h | 2 +- src/ripple/beast/net/detail/Parse.h | 2 +- src/ripple/beast/net/impl/IPEndpoint.cpp | 2 +- src/ripple/beast/rfc2616.h | 3 ++- src/ripple/crypto/impl/RFC1751.cpp | 4 +-- src/ripple/json/impl/json_reader.cpp | 2 +- src/ripple/net/impl/RPCCall.cpp | 27 ------------------- .../nodestore/impl/DatabaseShardImp.cpp | 6 ++++- src/ripple/protocol/impl/UintTypes.cpp | 6 ++++- src/ripple/rpc/handlers/PayChanClaim.cpp | 27 ------------------- src/ripple/rpc/handlers/Tx.cpp | 2 +- src/ripple/rpc/impl/ServerHandlerImp.cpp | 6 ++++- 17 files changed, 49 insertions(+), 74 deletions(-) diff --git a/src/ripple/basics/StringUtilities.h b/src/ripple/basics/StringUtilities.h index f4cc83f57..911d95170 100644 --- a/src/ripple/basics/StringUtilities.h +++ b/src/ripple/basics/StringUtilities.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -95,6 +96,8 @@ bool parseUrl (parsedURL& pUrl, std::string const& strUrl); std::string trim_whitespace (std::string str); +boost::optional to_uint64(std::string const& s); + } // ripple #endif diff --git a/src/ripple/basics/base_uint.h b/src/ripple/basics/base_uint.h index cfa686765..9fbb04a56 100644 --- a/src/ripple/basics/base_uint.h +++ b/src/ripple/basics/base_uint.h @@ -354,19 +354,19 @@ public: */ bool SetHex (const char* psz, bool bStrict = false) { + // Find beginning. + auto pBegin = reinterpret_cast(psz); // skip leading spaces if (!bStrict) - while (isspace (*psz)) - psz++; + while (isspace(*pBegin)) + pBegin++; // skip 0x - if (!bStrict && psz[0] == '0' && tolower (psz[1]) == 'x') - psz += 2; - - const unsigned char* pEnd = reinterpret_cast (psz); - const unsigned char* pBegin = pEnd; + if (!bStrict && pBegin[0] == '0' && tolower(pBegin[1]) == 'x') + pBegin += 2; // Find end. + auto pEnd = pBegin; while (charUnHex(*pEnd) != -1) pEnd++; diff --git a/src/ripple/basics/impl/ResolverAsio.cpp b/src/ripple/basics/impl/ResolverAsio.cpp index 541e54e8a..674594570 100644 --- a/src/ripple/basics/impl/ResolverAsio.cpp +++ b/src/ripple/basics/impl/ResolverAsio.cpp @@ -279,7 +279,7 @@ public: // Attempt to find the first and last valid port separators auto const find_port_separator = [](char const c) -> bool { - if (std::isspace (c)) + if (std::isspace (static_cast(c))) return true; if (c == ':') diff --git a/src/ripple/basics/impl/StringUtilities.cpp b/src/ripple/basics/impl/StringUtilities.cpp index 83e77bc31..db045f23f 100644 --- a/src/ripple/basics/impl/StringUtilities.cpp +++ b/src/ripple/basics/impl/StringUtilities.cpp @@ -123,4 +123,13 @@ std::string trim_whitespace (std::string str) return str; } +boost::optional +to_uint64(std::string const& s) +{ + std::uint64_t result; + if (beast::lexicalCastChecked (result, s)) + return result; + return boost::none; +} + } // ripple diff --git a/src/ripple/beast/core/LexicalCast.h b/src/ripple/beast/core/LexicalCast.h index 5d9a1d2ca..03cd75077 100644 --- a/src/ripple/beast/core/LexicalCast.h +++ b/src/ripple/beast/core/LexicalCast.h @@ -184,7 +184,11 @@ struct LexicalCast operator () (bool& out, std::string in) const { // Convert the input to lowercase - std::transform(in.begin (), in.end (), in.begin (), ::tolower); + std::transform(in.begin (), in.end (), in.begin (), + [](auto c) + { + return ::tolower(static_cast(c)); + }); if (in == "1" || in == "true") { diff --git a/src/ripple/beast/core/osx_ObjCHelpers.h b/src/ripple/beast/core/osx_ObjCHelpers.h index 4b9599e98..aed00fb58 100644 --- a/src/ripple/beast/core/osx_ObjCHelpers.h +++ b/src/ripple/beast/core/osx_ObjCHelpers.h @@ -42,7 +42,7 @@ namespace // in the range [0-127] is the identity. We are more // strict and require only printable characters. for (auto const& c : s) - assert (isprint(c)); + assert (isprint(static_cast(c))); #endif return [NSString stringWithUTF8String: s.c_str()]; diff --git a/src/ripple/beast/net/detail/Parse.h b/src/ripple/beast/net/detail/Parse.h index 0a5946c43..33c72e25a 100644 --- a/src/ripple/beast/net/detail/Parse.h +++ b/src/ripple/beast/net/detail/Parse.h @@ -49,7 +49,7 @@ template bool expect_whitespace (InputStream& is) { char c; - if (is.get(c) && isspace(c)) + if (is.get(c) && isspace(static_cast(c))) return true; is.unget(); is.setstate (std::ios_base::failbit); diff --git a/src/ripple/beast/net/impl/IPEndpoint.cpp b/src/ripple/beast/net/impl/IPEndpoint.cpp index 767b08201..506decb65 100644 --- a/src/ripple/beast/net/impl/IPEndpoint.cpp +++ b/src/ripple/beast/net/impl/IPEndpoint.cpp @@ -86,7 +86,7 @@ Endpoint Endpoint::from_string_altform (std::string const& s) { char c; is.get(c); - if (!isspace (c)) + if (!isspace (static_cast(c))) { is.unget(); break; diff --git a/src/ripple/beast/rfc2616.h b/src/ripple/beast/rfc2616.h index b2c22d403..73d689acd 100644 --- a/src/ripple/beast/rfc2616.h +++ b/src/ripple/beast/rfc2616.h @@ -49,7 +49,8 @@ struct ci_equal_pred bool operator()(char c1, char c2) { // VFALCO TODO Use a table lookup here - return std::tolower(c1) == std::tolower(c2); + return std::tolower(static_cast(c1)) == + std::tolower(static_cast(c2)); } }; diff --git a/src/ripple/crypto/impl/RFC1751.cpp b/src/ripple/crypto/impl/RFC1751.cpp index 009083c0c..3bac8bd32 100644 --- a/src/ripple/crypto/impl/RFC1751.cpp +++ b/src/ripple/crypto/impl/RFC1751.cpp @@ -347,8 +347,8 @@ void RFC1751::standard (std::string& strWord) { for (auto& letter : strWord) { - if (islower (letter)) - letter = toupper (letter); + if (islower (static_cast(letter))) + letter = toupper (static_cast(letter)); else if (letter == '1') letter = 'L'; else if (letter == '0') diff --git a/src/ripple/json/impl/json_reader.cpp b/src/ripple/json/impl/json_reader.cpp index 948fb4b35..07052323b 100644 --- a/src/ripple/json/impl/json_reader.cpp +++ b/src/ripple/json/impl/json_reader.cpp @@ -392,7 +392,7 @@ Reader::readNumber () while ( current_ != end_ ) { - if (!std::isdigit (*current_)) + if (!std::isdigit (static_cast(*current_))) { auto ret = std::find (std::begin (extended_tokens), std::end (extended_tokens), *current_); diff --git a/src/ripple/net/impl/RPCCall.cpp b/src/ripple/net/impl/RPCCall.cpp index 6bb52231c..3ac739121 100644 --- a/src/ripple/net/impl/RPCCall.cpp +++ b/src/ripple/net/impl/RPCCall.cpp @@ -84,33 +84,6 @@ std::string createHTTPPost ( return s.str (); } -static -boost::optional -to_uint64(std::string const& s) -{ - if (s.empty()) - return boost::none; - - for (auto c : s) - { - if (!isdigit(c)) - return boost::none; - } - - try - { - std::size_t pos{}; - auto const drops = std::stoul(s, &pos); - if (s.size() != pos) - return boost::none; - return drops; - } - catch (std::exception const&) - { - return boost::none; - } -} - class RPCParser { private: diff --git a/src/ripple/nodestore/impl/DatabaseShardImp.cpp b/src/ripple/nodestore/impl/DatabaseShardImp.cpp index 6160d4a66..9122abc80 100644 --- a/src/ripple/nodestore/impl/DatabaseShardImp.cpp +++ b/src/ripple/nodestore/impl/DatabaseShardImp.cpp @@ -95,7 +95,11 @@ DatabaseShardImp::init() if (!is_directory(d)) continue; auto dirName = d.path().stem().string(); - if (!std::all_of(dirName.begin(), dirName.end(), ::isdigit)) + if (!std::all_of(dirName.begin(), dirName.end(), + [](auto c) + { + return ::isdigit(static_cast(c)); + })) continue; auto const shardIndex {std::stoul(dirName)}; if (shardIndex < earliestShardIndex()) diff --git a/src/ripple/protocol/impl/UintTypes.cpp b/src/ripple/protocol/impl/UintTypes.cpp index 349557a96..eee35a5ae 100644 --- a/src/ripple/protocol/impl/UintTypes.cpp +++ b/src/ripple/protocol/impl/UintTypes.cpp @@ -78,7 +78,11 @@ bool to_currency(Currency& currency, std::string const& code) { Blob codeBlob (CURRENCY_CODE_LENGTH); - std::transform (code.begin (), code.end (), codeBlob.begin (), ::toupper); + std::transform (code.begin (), code.end (), codeBlob.begin (), + [](auto c) + { + return ::toupper(static_cast(c)); + }); Serializer s; diff --git a/src/ripple/rpc/handlers/PayChanClaim.cpp b/src/ripple/rpc/handlers/PayChanClaim.cpp index 16ac28586..b1f32b21d 100644 --- a/src/ripple/rpc/handlers/PayChanClaim.cpp +++ b/src/ripple/rpc/handlers/PayChanClaim.cpp @@ -35,33 +35,6 @@ namespace ripple { -static -boost::optional -to_uint64(std::string const& s) -{ - if (s.empty()) - return boost::none; - - for (auto c : s) - { - if (!isdigit(c)) - return boost::none; - } - - try - { - std::size_t pos{}; - auto const drops = std::stoul(s, &pos); - if (s.size() != pos) - return boost::none; - return drops; - } - catch (std::exception const&) - { - return boost::none; - } -} - // { // secret_key: // channel_id: 256-bit channel id diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index 09012b6ca..3ed5423c5 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -45,7 +45,7 @@ isHexTxID (std::string const& txid) auto const ret = std::find_if (txid.begin (), txid.end (), [](std::string::value_type c) { - return !std::isxdigit (c); + return !std::isxdigit (static_cast(c)); }); return (ret == txid.end ()); diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index 688b9b2b6..8090bb099 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -267,7 +267,11 @@ build_map(beast::http::fields const& h) { auto key (e.name_string().to_string()); // TODO Replace with safe C++14 version - std::transform (key.begin(), key.end(), key.begin(), ::tolower); + std::transform (key.begin(), key.end(), key.begin(), + [](auto c) + { + return ::tolower(static_cast(c)); + }); c [key] = e.value().to_string(); } return c;