Merge #10977: [net] Fix use of uninitialized value in getnetworkinfo(const JSONRPCRequest&)

11dd29b [net] Fix use of uninitialized value in getnetworkinfo(const JSONRPCRequest& request) (practicalswift)

Pull request description:

  When running `test_bitcoin` under Valgrind I found the following issue:

  ```
  $ valgrind src/test/test_bitcoin
  ...
  ==10465== Use of uninitialised value of size 8
  ==10465==    at 0x6D09B61: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
  ==10465==    by 0x6D0B1BB: std::ostreambuf_iterator<char, std::char_traits<char> > std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::_M_insert_int<unsigned long>(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
  ==10465==    by 0x6D0B36C: std::num_put<char, std::ostreambuf_iterator<char, std::char_traits<char> > >::do_put(std::ostreambuf_iterator<char, std::char_traits<char> >, std::ios_base&, char, unsigned long) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
  ==10465==    by 0x6D17699: std::ostream& std::ostream::_M_insert<unsigned long>(unsigned long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
  ==10465==    by 0x4CAAD7: operator<< (ostream:171)
  ==10465==    by 0x4CAAD7: formatValue<ServiceFlags> (tinyformat.h:345)
  ==10465==    by 0x4CAAD7: void tinyformat::detail::FormatArg::formatImpl<ServiceFlags>(std::ostream&, char const*, char const*, int, void const*) (tinyformat.h:523)
  ==10465==    by 0x1924D4: format (tinyformat.h:510)
  ==10465==    by 0x1924D4: tinyformat::detail::formatImpl(std::ostream&, char const*, tinyformat::detail::FormatArg const*, int) (tinyformat.h:803)
  ==10465==    by 0x553A55: vformat (tinyformat.h:947)
  ==10465==    by 0x553A55: format<ServiceFlags> (tinyformat.h:957)
  ==10465==    by 0x553A55: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<ServiceFlags>(char const*, ServiceFlags const&) (tinyformat.h:966)
  ==10465==    by 0x54C952: getnetworkinfo(JSONRPCRequest const&) (net.cpp:462)
  ==10465==    by 0x28EDB5: CallRPC(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (rpc_tests.cpp:31)
  ==10465==    by 0x293947: rpc_tests::rpc_togglenetwork::test_method() (rpc_tests.cpp:88)
  ==10465==    by 0x2950E5: rpc_tests::rpc_togglenetwork_invoker() (rpc_tests.cpp:84)
  ==10465==    by 0x182496: invoke<void (*)()> (callback.hpp:56)
  ==10465==    by 0x182496: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89)
  ...
  ```

  The read of the uninitialized variable `nLocalServices` is triggered by `g_connman->GetLocalServices()` in `getnetworkinfo(const JSONRPCRequest& request)` (`net.cpp:462`):

  ```c++
  UniValue getnetworkinfo(const JSONRPCRequest& request)
  {
  ...
      if(g_connman)
          obj.push_back(Pair("localservices", strprintf("%016x", g_connman->GetLocalServices())));
  ...
  }
  ```

  The reason for the uninitialized `nLocalServices` is that `CConnman::Start(...)` is not called
  by the tests, and hence the initialization normally performed by `CConnman::Start(...)` is
  not done.

  This commit adds a method `Init(const Options& connOptions)` which is called by both the
  constructor and `CConnman::Start(...)`. This method initializes `nLocalServices` and the other
  relevant values from the supplied `Options` object.

Tree-SHA512: d8742363acffd03b2ee081cc56840275569e17edc6fa4bb1dee4a5971ffe4b8ab1d2fe7b68f98a086bf133b7ec46f4e471243ca08b45bf82356e8c831a5a5f21
This commit is contained in:
Wladimir J. van der Laan 2017-08-05 13:23:10 +02:00
commit 02f4c4a42e
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
2 changed files with 24 additions and 26 deletions

View file

@ -2210,12 +2210,10 @@ CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In) : nSeed0(nSeed0In), nSe
nReceiveFloodSize = 0;
semOutbound = NULL;
semAddnode = NULL;
nMaxConnections = 0;
nMaxOutbound = 0;
nMaxAddnode = 0;
nBestHeight = 0;
clientInterface = NULL;
flagInterruptMsgProc = false;
Options connOptions;
Init(connOptions);
}
NodeId CConnman::GetNewNodeId()
@ -2254,30 +2252,15 @@ bool CConnman::InitBinds(const std::vector<CService>& binds, const std::vector<C
return fBound;
}
bool CConnman::Start(CScheduler& scheduler, Options connOptions)
bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
{
Init(connOptions);
nTotalBytesRecv = 0;
nTotalBytesSent = 0;
nMaxOutboundTotalBytesSentInCycle = 0;
nMaxOutboundCycleStartTime = 0;
nRelevantServices = connOptions.nRelevantServices;
nLocalServices = connOptions.nLocalServices;
nMaxConnections = connOptions.nMaxConnections;
nMaxOutbound = std::min((connOptions.nMaxOutbound), nMaxConnections);
nMaxAddnode = connOptions.nMaxAddnode;
nMaxFeeler = connOptions.nMaxFeeler;
nSendBufferMaxSize = connOptions.nSendBufferMaxSize;
nReceiveFloodSize = connOptions.nReceiveFloodSize;
nMaxOutboundLimit = connOptions.nMaxOutboundLimit;
nMaxOutboundTimeframe = connOptions.nMaxOutboundTimeframe;
SetBestHeight(connOptions.nBestHeight);
clientInterface = connOptions.uiInterface;
if (fListen && !InitBinds(connOptions.vBinds, connOptions.vWhiteBinds)) {
if (clientInterface) {
clientInterface->ThreadSafeMessageBox(
@ -2287,8 +2270,6 @@ bool CConnman::Start(CScheduler& scheduler, Options connOptions)
return false;
}
vWhitelistedRange = connOptions.vWhitelistedRange;
for (const auto& strDest : connOptions.vSeedNodes) {
AddOneShot(strDest);
}

View file

@ -146,9 +146,26 @@ public:
std::vector<CSubNet> vWhitelistedRange;
std::vector<CService> vBinds, vWhiteBinds;
};
void Init(const Options& connOptions) {
nLocalServices = connOptions.nLocalServices;
nRelevantServices = connOptions.nRelevantServices;
nMaxConnections = connOptions.nMaxConnections;
nMaxOutbound = std::min(connOptions.nMaxOutbound, connOptions.nMaxConnections);
nMaxAddnode = connOptions.nMaxAddnode;
nMaxFeeler = connOptions.nMaxFeeler;
nBestHeight = connOptions.nBestHeight;
clientInterface = connOptions.uiInterface;
nSendBufferMaxSize = connOptions.nSendBufferMaxSize;
nReceiveFloodSize = connOptions.nReceiveFloodSize;
nMaxOutboundTimeframe = connOptions.nMaxOutboundTimeframe;
nMaxOutboundLimit = connOptions.nMaxOutboundLimit;
vWhitelistedRange = connOptions.vWhitelistedRange;
}
CConnman(uint64_t seed0, uint64_t seed1);
~CConnman();
bool Start(CScheduler& scheduler, Options options);
bool Start(CScheduler& scheduler, const Options& options);
void Stop();
void Interrupt();
bool GetNetworkActive() const { return fNetworkActive; };