From cff1c7b2c3b2cd74067bad2476e1ddfe6ad01a06 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 11 Feb 2021 12:39:57 +0100 Subject: [PATCH] Merge #21043: net: Avoid UBSan warning in ProcessMessage(...) 3ddbf22ed179a2db733af4b521bec5d2b13ebf4b util: Disallow negative mocktime (MarcoFalke) f5f2f9716885e7548809e77f46b493c896a019bf net: Avoid UBSan warning in ProcessMessage(...) (practicalswift) Pull request description: Avoid UBSan warning in `ProcessMessage(...)`. Context: https://github.com/bitcoin/bitcoin/pull/20380#issuecomment-770427182 (thanks Crypt-iQ!) ACKs for top commit: MarcoFalke: re-ACK 3ddbf22ed179a2db733af4b521bec5d2b13ebf4b only change is adding patch written by me ajtowns: ACK 3ddbf22ed179a2db733af4b521bec5d2b13ebf4b -- code review only Tree-SHA512: e8d7af0457ca86872b75a4e406c0a93aafd841c2962e244e147e748cc7ca118c56be0fdafe53765f4b291410030b2c3cc8f76f733b37a955d34fc885ab6037b9 --- src/net_processing.cpp | 3 +++ src/rpc/misc.cpp | 7 +++++-- src/util/time.cpp | 5 ++++- test/functional/rpc_uptime.py | 5 +++++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b16a9945f6..292ff224e7 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2850,6 +2850,9 @@ void PeerManagerImpl::ProcessMessage( vRecv >> nVersion >> nServiceInt >> nTime >> addrMe; nSendVersion = std::min(nVersion, PROTOCOL_VERSION); + if (nTime < 0) { + nTime = 0; + } nServices = ServiceFlags(nServiceInt); if (!pfrom.fInbound) { diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index c4cf607f61..fcab8d4b1b 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -522,7 +522,7 @@ static UniValue setmocktime(const JSONRPCRequest& request) "\nSet the local time to given timestamp (-regtest only)\n", { {"timestamp", RPCArg::Type::NUM, RPCArg::Optional::NO, UNIX_EPOCH_TIME + "\n" - " Pass 0 to go back to using the system time."}, + "Pass 0 to go back to using the system time."}, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{""}, @@ -540,7 +540,10 @@ static UniValue setmocktime(const JSONRPCRequest& request) LOCK(cs_main); RPCTypeCheck(request.params, {UniValue::VNUM}); - int64_t time = request.params[0].get_int64(); + const int64_t time{request.params[0].get_int64()}; + if (time < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Mocktime can not be negative: %s.", time)); + } SetMockTime(time); if (auto* node_context = GetContext(request.context)) { for (const auto& chain_client : node_context->chain_clients) { diff --git a/src/util/time.cpp b/src/util/time.cpp index 900f8777f2..718e13fded 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -10,6 +10,8 @@ #include #include +#include + #include #include #include @@ -19,7 +21,7 @@ void UninterruptibleSleep(const std::chrono::microseconds& n) { std::this_thread::sleep_for(n); } -static std::atomic nMockTime(0); //!< For unit testing +static std::atomic nMockTime(0); //!< For testing int64_t GetTime() { @@ -47,6 +49,7 @@ template std::chrono::microseconds GetTime(); void SetMockTime(int64_t nMockTimeIn) { + Assert(nMockTimeIn >= 0); nMockTime.store(nMockTimeIn, std::memory_order_relaxed); } diff --git a/test/functional/rpc_uptime.py b/test/functional/rpc_uptime.py index f7c9e4611d..645d76cdfa 100755 --- a/test/functional/rpc_uptime.py +++ b/test/functional/rpc_uptime.py @@ -10,6 +10,7 @@ Test corresponds to code in rpc/server.cpp. import time from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_raises_rpc_error class UptimeTest(BitcoinTestFramework): @@ -18,8 +19,12 @@ class UptimeTest(BitcoinTestFramework): self.setup_clean_chain = True def run_test(self): + self._test_negative_time() self._test_uptime() + def _test_negative_time(self): + assert_raises_rpc_error(-8, "Mocktime can not be negative: -1.", self.nodes[0].setmocktime, -1) + def _test_uptime(self): wait_time = 10 self.nodes[0].setmocktime(int(time.time() + wait_time))