diff options
-rwxr-xr-x | contrib/devtools/security-check.py | 10 | ||||
-rwxr-xr-x | contrib/devtools/test-security-check.py | 29 | ||||
-rw-r--r-- | doc/reduce-memory.md | 3 | ||||
-rw-r--r-- | doc/reduce-traffic.md | 16 | ||||
-rw-r--r-- | src/init.cpp | 2 | ||||
-rw-r--r-- | src/interfaces/chain.cpp | 9 | ||||
-rw-r--r-- | src/protocol.cpp | 9 | ||||
-rw-r--r-- | src/protocol.h | 4 | ||||
-rw-r--r-- | src/rpc/mining.cpp | 3 | ||||
-rw-r--r-- | src/rpc/util.cpp | 10 | ||||
-rw-r--r-- | src/sync.cpp | 4 | ||||
-rw-r--r-- | src/test/fuzz/integer.cpp | 4 | ||||
-rw-r--r-- | src/test/fuzz/locale.cpp | 12 | ||||
-rw-r--r-- | src/util/strencodings.cpp | 10 | ||||
-rw-r--r-- | src/util/strencodings.h | 4 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 2 | ||||
-rw-r--r-- | src/wallet/wallet.h | 3 | ||||
-rw-r--r-- | test/util/data/bitcoin-util-test.json | 6 |
18 files changed, 91 insertions, 49 deletions
diff --git a/contrib/devtools/security-check.py b/contrib/devtools/security-check.py index 21d64e893d..c05c38d513 100755 --- a/contrib/devtools/security-check.py +++ b/contrib/devtools/security-check.py @@ -197,6 +197,15 @@ def check_MACHO_NOUNDEFS(executable) -> bool: return True return False +def check_MACHO_NX(executable) -> bool: + ''' + Check for no stack execution + ''' + flags = get_MACHO_executable_flags(executable) + if 'ALLOW_STACK_EXECUTION' in flags: + return False + return True + CHECKS = { 'ELF': [ ('PIE', check_ELF_PIE), @@ -212,6 +221,7 @@ CHECKS = { 'MACHO': [ ('PIE', check_MACHO_PIE), ('NOUNDEFS', check_MACHO_NOUNDEFS), + ('NX', check_MACHO_NX) ] } diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py index 438d5f6bf0..e2a8154f16 100755 --- a/contrib/devtools/test-security-check.py +++ b/contrib/devtools/test-security-check.py @@ -43,16 +43,35 @@ class TestSecurityChecks(unittest.TestCase): self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE']), (0, '')) - def test_64bit_PE(self): + def test_PE(self): source = 'test1.c' executable = 'test1.exe' cc = 'x86_64-w64-mingw32-gcc' write_testcode(source) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--no-nxcompat','-Wl,--no-dynamicbase','-Wl,--no-high-entropy-va']), (1, executable+': failed DYNAMIC_BASE HIGH_ENTROPY_VA NX')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--no-dynamicbase','-Wl,--no-high-entropy-va']), (1, executable+': failed DYNAMIC_BASE HIGH_ENTROPY_VA')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--dynamicbase','-Wl,--no-high-entropy-va']), (1, executable+': failed HIGH_ENTROPY_VA')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--dynamicbase','-Wl,--high-entropy-va']), (0, '')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--no-nxcompat','-Wl,--no-dynamicbase','-Wl,--no-high-entropy-va']), + (1, executable+': failed DYNAMIC_BASE HIGH_ENTROPY_VA NX')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--no-dynamicbase','-Wl,--no-high-entropy-va']), + (1, executable+': failed DYNAMIC_BASE HIGH_ENTROPY_VA')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--dynamicbase','-Wl,--no-high-entropy-va']), + (1, executable+': failed HIGH_ENTROPY_VA')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,--nxcompat','-Wl,--dynamicbase','-Wl,--high-entropy-va']), + (0, '')) + + def test_MACHO(self): + source = 'test1.c' + executable = 'test1' + cc = 'clang' + write_testcode(source) + + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie','-Wl,-flat_namespace', '-Wl,-allow_stack_execute']), + (1, executable+': failed PIE NOUNDEFS NX')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie','-Wl,-flat_namespace']), + (1, executable+': failed PIE NOUNDEFS')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-no_pie']), + (1, executable+': failed PIE')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-pie']), + (0, '')) if __name__ == '__main__': unittest.main() diff --git a/doc/reduce-memory.md b/doc/reduce-memory.md index b0faf0825a..34a5b19b12 100644 --- a/doc/reduce-memory.md +++ b/doc/reduce-memory.md @@ -24,8 +24,7 @@ The size of some in-memory caches can be reduced. As caches trade off memory usa ## Number of peers -- `-maxconnections=<n>` - the maximum number of connections, this defaults to `125`. Each active connection takes up some memory. Only significant if incoming - connections are enabled, otherwise the number of connections will never be more than `8`. +- `-maxconnections=<n>` - the maximum number of connections, this defaults to `125`. Each active connection takes up some memory. Only significant if incoming connections are enabled, otherwise the number of connections will never be more than `10`. Of the 10 outbound peers, there can be 8 full outgoing connections and 2 -blocksonly peers, in which case they are block/addr peers, but not tx peers. ## Thread configuration diff --git a/doc/reduce-traffic.md b/doc/reduce-traffic.md index 5a71f62e0f..7debb0a16e 100644 --- a/doc/reduce-traffic.md +++ b/doc/reduce-traffic.md @@ -3,8 +3,10 @@ Reduce Traffic Some node operators need to deal with bandwidth caps imposed by their ISPs. -By default, Bitcoin Core allows up to 125 connections to different peers, 8 of -which are outbound. You can therefore, have at most 117 inbound connections. +By default, Bitcoin Core allows up to 125 connections to different peers, 10 of +which are outbound. You can therefore, have at most 115 inbound connections. +Of the 10 outbound peers, there can be 8 full outgoing connections and 2 with +the -blocksonly mode turned on. You can therefore, have at most 115 inbound connections. The default settings can result in relatively significant traffic consumption. @@ -26,7 +28,7 @@ calculating the target. ## 2. Disable "listening" (`-listen=0`) -Disabling listening will result in fewer nodes connected (remember the maximum of 8 +Disabling listening will result in fewer nodes connected (remember the maximum of 10 outbound peers). Fewer nodes will result in less traffic usage as you are relaying blocks and transactions to fewer nodes. @@ -44,7 +46,11 @@ with other peers, you can disable transaction relay. Be reminded of the effects of this setting. - Fee estimation will no longer work. -- Not relaying other's transactions could hurt your privacy if used while a - wallet is loaded or if you use the node to broadcast transactions. +- It sets the flag "-walletbroadcast" to be "0", only if it is currently unset. + Doing so disables the automatic broadcasting of transactions from wallet. Not + relaying other's transactions could hurt your privacy if used while a wallet + is loaded or if you use the node to broadcast transactions. +- If a peer is whitelisted and "-whitelistforcerelay" is set to "1" (which will + also set "whitelistrelay" to "1"), we will still receive and relay their transactions. - It makes block propagation slower because compact block relay can only be used when transaction relay is enabled. diff --git a/src/init.cpp b/src/init.cpp index eb5329df66..e08ab5be7e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -372,7 +372,7 @@ void SetupServerArgs() gArgs.AddArg("-blocknotify=<cmd>", "Execute command when the best block changes (%s in cmd is replaced by block hash)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); #endif gArgs.AddArg("-blockreconstructionextratxn=<n>", strprintf("Extra transactions to keep in memory for compact block reconstructions (default: %u)", DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-blocksonly", strprintf("Whether to reject transactions from network peers. Transactions from the wallet, RPC and relay whitelisted inbound peers are not affected. (default: %u)", DEFAULT_BLOCKSONLY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + gArgs.AddArg("-blocksonly", strprintf("Whether to reject transactions from network peers. Automatic broadcast and rebroadcast of any transactions from inbound peers is disabled, unless '-whitelistforcerelay' is '1', in which case whitelisted peers' transactions will be relayed. RPC transactions are not affected. (default: %u)", DEFAULT_BLOCKSONLY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); gArgs.AddArg("-conf=<file>", strprintf("Specify configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); gArgs.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); gArgs.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS); diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 9dc0d37cd9..775a89f4cf 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -234,11 +234,10 @@ public: explicit ChainImpl(NodeContext& node) : m_node(node) {} std::unique_ptr<Chain::Lock> lock(bool try_lock) override { - auto result = MakeUnique<LockImpl>(::cs_main, "cs_main", __FILE__, __LINE__, try_lock); - if (try_lock && result && !*result) return {}; - // std::move necessary on some compilers due to conversion from - // LockImpl to Lock pointer - return std::move(result); + auto lock = MakeUnique<LockImpl>(::cs_main, "cs_main", __FILE__, __LINE__, try_lock); + if (try_lock && lock && !*lock) return {}; + std::unique_ptr<Chain::Lock> result = std::move(lock); // Temporary to avoid CWG 1579 + return result; } bool findBlock(const uint256& hash, CBlock* block, int64_t* time, int64_t* time_max) override { diff --git a/src/protocol.cpp b/src/protocol.cpp index e49e5523ac..bd3ed25a8a 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -85,8 +85,13 @@ CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn) CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* pszCommand, unsigned int nMessageSizeIn) { memcpy(pchMessageStart, pchMessageStartIn, MESSAGE_START_SIZE); - memset(pchCommand, 0, sizeof(pchCommand)); - strncpy(pchCommand, pszCommand, COMMAND_SIZE); + + // Copy the command name, zero-padding to COMMAND_SIZE bytes + size_t i = 0; + for (; i < COMMAND_SIZE && pszCommand[i] != 0; ++i) pchCommand[i] = pszCommand[i]; + assert(pszCommand[i] == 0); // Assert that the command name passed in is not longer than COMMAND_SIZE + for (; i < COMMAND_SIZE; ++i) pchCommand[i] = 0; + nMessageSize = nMessageSizeIn; memset(pchChecksum, 0, CHECKSUM_SIZE); } diff --git a/src/protocol.h b/src/protocol.h index db07efb9f9..6639ae2aac 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -37,6 +37,10 @@ public: typedef unsigned char MessageStartChars[MESSAGE_START_SIZE]; explicit CMessageHeader(const MessageStartChars& pchMessageStartIn); + + /** Construct a P2P message header from message-start characters, a command and the size of the message. + * @note Passing in a `pszCommand` longer than COMMAND_SIZE will result in a run-time assertion error. + */ CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* pszCommand, unsigned int nMessageSizeIn); std::string GetCommand() const; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 1bbb5c4bee..bde19d8e79 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -27,6 +27,7 @@ #include <univalue.h> #include <util/fees.h> #include <util/strencodings.h> +#include <util/string.h> #include <util/system.h> #include <validation.h> #include <validationinterface.h> @@ -694,7 +695,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) result.pushKV("transactions", transactions); result.pushKV("coinbaseaux", aux); result.pushKV("coinbasevalue", (int64_t)pblock->vtx[0]->vout[0].nValue); - result.pushKV("longpollid", ::ChainActive().Tip()->GetBlockHash().GetHex() + i64tostr(nTransactionsUpdatedLast)); + result.pushKV("longpollid", ::ChainActive().Tip()->GetBlockHash().GetHex() + ToString(nTransactionsUpdatedLast)); result.pushKV("target", hashTarget.GetHex()); result.pushKV("mintime", (int64_t)pindexPrev->GetMedianTimePast()+1); result.pushKV("mutable", aMutable); diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 4ba84d2515..32e0e1ec27 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -293,7 +293,7 @@ UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_s struct Section { Section(const std::string& left, const std::string& right) : m_left{left}, m_right{right} {} - const std::string m_left; + std::string m_left; const std::string m_right; }; @@ -645,6 +645,10 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const } if (m_type == Type::ARR) { sections.PushSection({indent_next + "...", ""}); + } else { + CHECK_NONFATAL(!m_inner.empty()); + // Remove final comma, which would be invalid JSON + sections.m_sections.back().m_left.pop_back(); } sections.PushSection({indent + "]" + maybe_separator, ""}); return; @@ -658,6 +662,10 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const if (m_type == Type::OBJ_DYN) { // If the dictionary keys are dynamic, use three dots for continuation sections.PushSection({indent_next + "...", ""}); + } else { + CHECK_NONFATAL(!m_inner.empty()); + // Remove final comma, which would be invalid JSON + sections.m_sections.back().m_left.pop_back(); } sections.PushSection({indent + "}" + maybe_separator, ""}); return; diff --git a/src/sync.cpp b/src/sync.cpp index 71657a7439..b86c57e498 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -13,9 +13,9 @@ #include <util/strencodings.h> #include <util/threadnames.h> -#include <system_error> #include <map> #include <set> +#include <system_error> #ifdef DEBUG_LOCKCONTENTION #if !defined(HAVE_THREAD_LOCAL) @@ -57,7 +57,7 @@ struct CLockLocation { { return strprintf( "%s %s:%s%s (in thread %s)", - mutexName, sourceFile, itostr(sourceLine), (fTry ? " (TRY)" : ""), m_thread_name); + mutexName, sourceFile, sourceLine, (fTry ? " (TRY)" : ""), m_thread_name); } std::string Name() const diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index 63b9296574..bd2e20030d 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -27,6 +27,7 @@ #include <uint256.h> #include <util/moneystr.h> #include <util/strencodings.h> +#include <util/string.h> #include <util/system.h> #include <util/time.h> #include <version.h> @@ -93,11 +94,10 @@ void test_one_input(const std::vector<uint8_t>& buffer) // (void)GetVirtualTransactionSize(i64, i64, u32); // function defined only for a subset of int64_t/uint32_t inputs (void)HexDigit(ch); (void)MoneyRange(i64); - (void)i64tostr(i64); + (void)ToString(i64); (void)IsDigit(ch); (void)IsSpace(ch); (void)IsSwitchChar(ch); - (void)itostr(i32); (void)memusage::DynamicUsage(ch); (void)memusage::DynamicUsage(i16); (void)memusage::DynamicUsage(i32); diff --git a/src/test/fuzz/locale.cpp b/src/test/fuzz/locale.cpp index c8288123e8..09580c0c52 100644 --- a/src/test/fuzz/locale.cpp +++ b/src/test/fuzz/locale.cpp @@ -2,10 +2,11 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <test/fuzz/fuzz.h> #include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> #include <tinyformat.h> #include <util/strencodings.h> +#include <util/string.h> #include <cassert> #include <clocale> @@ -54,9 +55,8 @@ void test_one_input(const std::vector<uint8_t>& buffer) const int atoi_without_locale = atoi(random_string); const int64_t atoi64c_without_locale = atoi64(random_string.c_str()); const int64_t random_int64 = fuzzed_data_provider.ConsumeIntegral<int64_t>(); - const std::string i64tostr_without_locale = i64tostr(random_int64); + const std::string tostring_without_locale = ToString(random_int64); const int32_t random_int32 = fuzzed_data_provider.ConsumeIntegral<int32_t>(); - const std::string itostr_without_locale = itostr(random_int32); const std::string strprintf_int_without_locale = strprintf("%d", random_int64); const double random_double = fuzzed_data_provider.ConsumeFloatingPoint<double>(); const std::string strprintf_double_without_locale = strprintf("%f", random_double); @@ -82,10 +82,8 @@ void test_one_input(const std::vector<uint8_t>& buffer) assert(atoi64c_without_locale == atoi64c_with_locale); const int atoi_with_locale = atoi(random_string); assert(atoi_without_locale == atoi_with_locale); - const std::string i64tostr_with_locale = i64tostr(random_int64); - assert(i64tostr_without_locale == i64tostr_with_locale); - const std::string itostr_with_locale = itostr(random_int32); - assert(itostr_without_locale == itostr_with_locale); + const std::string tostring_with_locale = ToString(random_int64); + assert(tostring_without_locale == tostring_with_locale); const std::string strprintf_int_with_locale = strprintf("%d", random_int64); assert(strprintf_int_without_locale == strprintf_int_with_locale); const std::string strprintf_double_with_locale = strprintf("%f", random_double); diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index eec1a52e95..16917ccb47 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -407,16 +407,6 @@ std::string FormatParagraph(const std::string& in, size_t width, size_t indent) return out.str(); } -std::string i64tostr(int64_t n) -{ - return strprintf("%d", n); -} - -std::string itostr(int n) -{ - return strprintf("%d", n); -} - int64_t atoi64(const char* psz) { #ifdef _MSC_VER diff --git a/src/util/strencodings.h b/src/util/strencodings.h index ccc4edac12..11317f0432 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -54,9 +54,7 @@ std::string DecodeBase32(const std::string& str, bool* pf_invalid = nullptr); std::string EncodeBase32(const unsigned char* pch, size_t len); std::string EncodeBase32(const std::string& str); -void SplitHostPort(std::string in, int &portOut, std::string &hostOut); -std::string i64tostr(int64_t n); -std::string itostr(int n); +void SplitHostPort(std::string in, int& portOut, std::string& hostOut); int64_t atoi64(const char* psz); int64_t atoi64(const std::string& str); int atoi(const std::string& str); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 0eb7ed2b71..5d34e592db 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2390,8 +2390,6 @@ static UniValue getbalances(const JSONRPCRequest& request) auto locked_chain = wallet.chain().lock(); LOCK(wallet.cs_wallet); - UniValue obj(UniValue::VOBJ); - const auto bal = wallet.GetBalance(); UniValue balances{UniValue::VOBJ}; { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 75fd14a80e..9a83c84fc4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -16,6 +16,7 @@ #include <ui_interface.h> #include <util/message.h> #include <util/strencodings.h> +#include <util/string.h> #include <util/system.h> #include <validationinterface.h> #include <wallet/coinselection.h> @@ -215,7 +216,7 @@ static inline void WriteOrderPos(const int64_t& nOrderPos, mapValue_t& mapValue) { if (nOrderPos == -1) return; - mapValue["n"] = i64tostr(nOrderPos); + mapValue["n"] = ToString(nOrderPos); } struct COutputEntry diff --git a/test/util/data/bitcoin-util-test.json b/test/util/data/bitcoin-util-test.json index 16dfa8d77f..99cd4ab695 100644 --- a/test/util/data/bitcoin-util-test.json +++ b/test/util/data/bitcoin-util-test.json @@ -219,6 +219,12 @@ "description": "Parses a transaction with no inputs and a single output script (output in json)" }, { "exec": "./bitcoin-tx", + "args": ["-create", "outscript=0:123badscript"], + "return_code": 1, + "error_txt": "error: script parse error", + "description": "Create a new transaction with an invalid output script" + }, + { "exec": "./bitcoin-tx", "args": ["-create", "outscript=0:OP_DROP", "nversion=1"], "output_cmp": "txcreatescript1.hex", "description": "Create a new transaction with a single output script (OP_DROP)" |