Skip to content

Commit

Permalink
Merge bitcoin#21043: net: Avoid UBSan warning in ProcessMessage(...)
Browse files Browse the repository at this point in the history
3ddbf22 util: Disallow negative mocktime (MarcoFalke)
f5f2f97 net: Avoid UBSan warning in ProcessMessage(...) (practicalswift)

Pull request description:

  Avoid UBSan warning in `ProcessMessage(...)`.

  Context: bitcoin#20380 (comment) (thanks Crypt-iQ!)

ACKs for top commit:
  MarcoFalke:
    re-ACK 3ddbf22 only change is adding patch written by me
  ajtowns:
    ACK 3ddbf22 -- code review only

Tree-SHA512: e8d7af0457ca86872b75a4e406c0a93aafd841c2962e244e147e748cc7ca118c56be0fdafe53765f4b291410030b2c3cc8f76f733b37a955d34fc885ab6037b9
  • Loading branch information
MarcoFalke authored and vijaydasmp committed Dec 9, 2023
1 parent 583503d commit f70dd4e
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 3 deletions.
3 changes: 3 additions & 0 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2865,6 +2865,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)
{
Expand Down
7 changes: 5 additions & 2 deletions src/rpc/misc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{""},
Expand All @@ -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<NodeContext>(request.context)) {
for (const auto& chain_client : node_context->chain_clients) {
Expand Down
5 changes: 4 additions & 1 deletion src/util/time.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include <compat.h>
#include <util/time.h>

#include <util/check.h>

#include <atomic>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <ctime>
Expand All @@ -19,7 +21,7 @@

void UninterruptibleSleep(const std::chrono::microseconds& n) { std::this_thread::sleep_for(n); }

static std::atomic<int64_t> nMockTime(0); //!< For unit testing
static std::atomic<int64_t> nMockTime(0); //!< For testing

int64_t GetTime()
{
Expand Down Expand Up @@ -47,6 +49,7 @@ template std::chrono::microseconds GetTime();

void SetMockTime(int64_t nMockTimeIn)
{
Assert(nMockTimeIn >= 0);
nMockTime.store(nMockTimeIn, std::memory_order_relaxed);
}

Expand Down
5 changes: 5 additions & 0 deletions test/functional/rpc_uptime.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import time

from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_raises_rpc_error


class UptimeTest(BitcoinTestFramework):
Expand All @@ -18,8 +19,12 @@ def set_test_params(self):
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))
Expand Down

0 comments on commit f70dd4e

Please sign in to comment.