Follow Bitcoin PRs:

- rpc/wallet: initialize nFeeRequired to avoid using garbage value on failure bitcoin#18274 - httpserver: use own HTTP status codes bitcoin#18168 - tests: Add fuzzing harnesses for various Base{32,58,64} and hex related functions bitcoin#17229 - util: Don't allow Base32/64-decoding or ParseMoney(…) on strings with embedded NUL characters. Add tests. bitcoin#17753 - util: Fail to parse empty string in ParseMoney bitcoin#18225 - util: Fail to parse whitespace-only strings in ParseMoney(...) (instead of parsing as zero) bitcoin#18270 - Replace the LogPrint function with a macro bitcoin#17218 - Fix wallet unload race condition bitcoin#18338 - qt: Fix Window -> Minimize menu item bitcoin#18549 - windows: Enable heap terminate-on-corruption bitcoin#17916
litecoinz-core · Apr 9, 2020 · 3ec6a5c · 3ec6a5c
1 parent 40db1e1
commit 3ec6a5c
Show file tree

Hide file tree

Showing 27 changed files with 268 additions and 84 deletions.
diff --git a/src/Makefile.test.include b/src/Makefile.test.include
@@ -7,6 +7,7 @@ FUZZ_TARGETS = \
   test/fuzz/address_deserialize \
   test/fuzz/addrman_deserialize \
   test/fuzz/banentry_deserialize \
+  test/fuzz/base_encode_decode \
   test/fuzz/block_deserialize \
   test/fuzz/blockheader_deserialize \
   test/fuzz/blocklocator_deserialize \
@@ -17,6 +18,7 @@ FUZZ_TARGETS = \
   test/fuzz/bloomfilter_deserialize \
   test/fuzz/coins_deserialize \
   test/fuzz/diskblockindex_deserialize \
+  test/fuzz/hex \
   test/fuzz/inv_deserialize \
   test/fuzz/messageheader_deserialize \
   test/fuzz/netaddr_deserialize \
@@ -245,6 +247,12 @@ test_fuzz_banentry_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
 test_fuzz_banentry_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
 test_fuzz_banentry_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON)
 
+test_fuzz_base_encode_decode_SOURCES = $(FUZZ_SUITE) test/fuzz/base_encode_decode.cpp
+test_fuzz_base_encode_decode_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
+test_fuzz_base_encode_decode_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
+test_fuzz_base_encode_decode_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
+test_fuzz_base_encode_decode_LDADD = $(FUZZ_SUITE_LD_COMMON)
+
 test_fuzz_txundo_deserialize_SOURCES = $(FUZZ_SUITE) test/fuzz/deserialize.cpp
 test_fuzz_txundo_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DTXUNDO_DESERIALIZE=1
 test_fuzz_txundo_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
@@ -293,6 +301,12 @@ test_fuzz_address_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
 test_fuzz_address_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
 test_fuzz_address_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON)
 
+test_fuzz_hex_SOURCES = $(FUZZ_SUITE) test/fuzz/hex.cpp
+test_fuzz_hex_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
+test_fuzz_hex_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
+test_fuzz_hex_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS)
+test_fuzz_hex_LDADD = $(FUZZ_SUITE_LD_COMMON)
+
 test_fuzz_inv_deserialize_SOURCES = $(FUZZ_SUITE) test/fuzz/deserialize.cpp
 test_fuzz_inv_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DINV_DESERIALIZE=1
 test_fuzz_inv_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)

diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp
@@ -18,9 +18,8 @@ static void WalletBalance(benchmark::State& state, const bool set_dirty, const b
     {
         bool first_run;
         if (wallet.LoadWallet(first_run) != DBErrors::LOAD_OK) assert(false);
-        wallet.handleNotifications();
     }
-
+    auto handler = chain->handleNotifications({ &wallet, [](CWallet*) {} });
 
     const Optional<std::string> address_mine{add_mine ? Optional<std::string>{getnewaddress(wallet)} : nullopt};
     if (add_watchonly) importaddress(wallet, ADDRESS_WATCHONLY);

diff --git a/src/chainparams.cpp b/src/chainparams.cpp
@@ -373,7 +373,7 @@ class CRegTestParams : public CChainParams {
         bip44CoinType = 1;
         consensus.nApproxReleaseHeight = 200000;
         consensus.fCoinbaseMustBeShielded = false;
-        consensus.nSubsidyHalvingInterval = 150;
+        consensus.nSubsidyHalvingInterval = 1500;
         consensus.BIP16Enabled = true;
         consensus.BIP34Enabled = false;
         consensus.BIP65Enabled = false;

diff --git a/src/httpserver.cpp b/src/httpserver.cpp
@@ -237,7 +237,7 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
     if (hreq->GetRequestMethod() == HTTPRequest::UNKNOWN) {
         LogPrint(BCLog::HTTP, "HTTP request from %s rejected: Unknown HTTP request method\n",
                  hreq->GetPeer().ToString());
-        hreq->WriteReply(HTTP_BADMETHOD);
+        hreq->WriteReply(HTTP_BAD_METHOD);
         return;
     }
 
@@ -269,10 +269,10 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
             item.release(); /* if true, queue took ownership */
         else {
             LogPrintf("WARNING: request rejected because http work queue depth exceeded, it can be increased with the -rpcworkqueue= setting\n");
-            item->req->WriteReply(HTTP_INTERNAL, "Work queue depth exceeded");
+            item->req->WriteReply(HTTP_INTERNAL_SERVER_ERROR, "Work queue depth exceeded");
         }
     } else {
-        hreq->WriteReply(HTTP_NOTFOUND);
+        hreq->WriteReply(HTTP_NOT_FOUND);
     }
 }
 
@@ -520,7 +520,7 @@ HTTPRequest::~HTTPRequest()
     if (!replySent) {
         // Keep track of whether reply was sent to avoid request leaks
         LogPrintf("%s: Unhandled request\n", __func__);
-        WriteReply(HTTP_INTERNAL, "Unhandled request");
+        WriteReply(HTTP_INTERNAL_SERVER_ERROR, "Unhandled request");
     }
     // evhttpd cleans up the request, as long as a reply was sent.
 }

diff --git a/src/init.cpp b/src/init.cpp
@@ -977,8 +977,8 @@ bool AppInitBasicSetup()
     _set_abort_behavior(0, _WRITE_ABORT_MSG | _CALL_REPORTFAULT);
 #endif
 #ifdef WIN32
-    // Enable Data Execution Prevention (DEP)
-    SetProcessDEPPolicy(PROCESS_DEP_ENABLE);
+    // Enable heap terminate-on-corruption
+    HeapSetInformation(nullptr, HeapEnableTerminationOnCorruption, nullptr, 0);
 #endif
 
     if (!SetupNetworking())

diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
@@ -156,22 +156,12 @@ class LockImpl : public Chain::Lock, public UniqueLock<CCriticalSection>
     using UniqueLock::UniqueLock;
 };
 
-class NotificationsHandlerImpl : public Handler, CValidationInterface
+class NotificationsProxy : public CValidationInterface
 {
 public:
-    explicit NotificationsHandlerImpl(Chain& chain, Chain::Notifications& notifications)
-        : m_chain(chain), m_notifications(&notifications)
-    {
-        RegisterValidationInterface(this);
-    }
-    ~NotificationsHandlerImpl() override { disconnect(); }
-    void disconnect() override
-    {
-        if (m_notifications) {
-            m_notifications = nullptr;
-            UnregisterValidationInterface(this);
-        }
-    }
+    explicit NotificationsProxy(std::shared_ptr<Chain::Notifications> notifications)
+        : m_notifications(std::move(notifications)) {}
+    virtual ~NotificationsProxy() = default;
     void TransactionAddedToMempool(const CTransactionRef& tx) override
     {
         m_notifications->TransactionAddedToMempool(tx);
@@ -199,8 +189,26 @@ class NotificationsHandlerImpl : public Handler, CValidationInterface
         m_notifications->UpdatedBlockTip();
     }
     void ChainStateFlushed(const CBlockLocator& locator) override { m_notifications->ChainStateFlushed(locator); }
-    Chain& m_chain;
-    Chain::Notifications* m_notifications;
+    std::shared_ptr<Chain::Notifications> m_notifications;
+};
+
+class NotificationsHandlerImpl : public Handler
+{
+public:
+    explicit NotificationsHandlerImpl(std::shared_ptr<Chain::Notifications> notifications)
+        : m_proxy(std::make_shared<NotificationsProxy>(std::move(notifications)))
+    {
+        RegisterSharedValidationInterface(m_proxy);
+    }
+    ~NotificationsHandlerImpl() override { disconnect(); }
+    void disconnect() override
+    {
+        if (m_proxy) {
+            UnregisterSharedValidationInterface(m_proxy);
+            m_proxy.reset();
+        }
+    }
+    std::shared_ptr<NotificationsProxy> m_proxy;
 };
 
 class RpcHandlerImpl : public Handler
@@ -358,9 +366,9 @@ class ChainImpl : public Chain
     {
         ::uiInterface.ShowProgress(title, progress, resume_possible);
     }
-    std::unique_ptr<Handler> handleNotifications(Notifications& notifications) override
+    std::unique_ptr<Handler> handleNotifications(std::shared_ptr<Notifications> notifications) override
     {
-        return MakeUnique<NotificationsHandlerImpl>(*this, notifications);
+        return MakeUnique<NotificationsHandlerImpl>(std::move(notifications));
     }
     void waitForNotificationsIfNewBlocksConnected(const uint256& old_tip) override
     {

diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
@@ -232,7 +232,7 @@ class Chain
     };
 
     //! Register handler for notifications.
-    virtual std::unique_ptr<Handler> handleNotifications(Notifications& notifications) = 0;
+    virtual std::unique_ptr<Handler> handleNotifications(std::shared_ptr<Notifications> notifications) = 0;
 
     //! Wait for pending notifications to be processed unless block hash points to the current
     //! chain tip, or to a possible descendant of the current chain tip that isn't currently

diff --git a/src/logging.h b/src/logging.h
@@ -158,12 +158,13 @@ static inline void LogPrintf(const char* fmt, const Args&... args)
     }
 }
 
-template <typename... Args>
-static inline void LogPrint(const BCLog::LogFlags& category, const Args&... args)
-{
-    if (LogAcceptCategory((category))) {
-        LogPrintf(args...);
-    }
-}
+// Use a macro instead of a function for conditional logging to prevent
+// evaluating arguments when logging for the category is not enabled.
+#define LogPrint(category, ...)              \
+    do {                                     \
+        if (LogAcceptCategory((category))) { \
+            LogPrintf(__VA_ARGS__);          \
+        }                                    \
+    } while (0)
 
 #endif // BITCOIN_LOGGING_H
diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
@@ -509,7 +509,7 @@ void BitcoinGUI::createMenuBar()
     QAction* minimize_action = window_menu->addAction(tr("Minimize"));
     minimize_action->setShortcut(QKeySequence(Qt::CTRL + Qt::Key_M));
     connect(minimize_action, &QAction::triggered, [] {
-        qApp->focusWindow()->showMinimized();
+        QApplication::activeWindow()->showMinimized();
     });
     connect(qApp, &QApplication::focusWindowChanged, [minimize_action] (QWindow* window) {
         minimize_action->setEnabled(window != nullptr && (window->flags() & Qt::Dialog) != Qt::Dialog && window->windowState() != Qt::WindowMinimized);

diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp
@@ -137,7 +137,7 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wal
     const bool called = QMetaObject::invokeMethod(wallet_model, "startPollBalance");
     assert(called);
 
-    connect(wallet_model, &WalletModel::unload, [this, wallet_model] {
+    connect(wallet_model, &WalletModel::unload, this, [this, wallet_model] {
         // Defer removeAndDeleteWallet when no modal widget is active.
         // TODO: remove this workaround by removing usage of QDiallog::exec.
         if (QApplication::activeModalWidget()) {
@@ -149,7 +149,7 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wal
         } else {
             removeAndDeleteWallet(wallet_model);
         }
-    });
+    }, Qt::QueuedConnection);
 
     // Re-emit coinsSent signal from wallet model.
     connect(wallet_model, &WalletModel::coinsSent, this, &WalletController::coinsSent);

diff --git a/src/test/base32_tests.cpp b/src/test/base32_tests.cpp
@@ -20,6 +20,17 @@ BOOST_AUTO_TEST_CASE(base32_testvectors)
         std::string strDec = DecodeBase32(vstrOut[i]);
         BOOST_CHECK_EQUAL(strDec, vstrIn[i]);
     }
+
+    // Decoding strings with embedded NUL characters should fail
+    bool failure;
+    (void)DecodeBase32(std::string("invalid", 7), &failure);
+    BOOST_CHECK_EQUAL(failure, true);
+    (void)DecodeBase32(std::string("AWSX3VPP", 8), &failure);
+    BOOST_CHECK_EQUAL(failure, false);
+    (void)DecodeBase32(std::string("AWSX3VPP\0invalid", 16), &failure);
+    BOOST_CHECK_EQUAL(failure, true);
+    (void)DecodeBase32(std::string("AWSX3VPPinvalid", 15), &failure);
+    BOOST_CHECK_EQUAL(failure, true);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/base64_tests.cpp b/src/test/base64_tests.cpp
@@ -20,6 +20,17 @@ BOOST_AUTO_TEST_CASE(base64_testvectors)
         std::string strDec = DecodeBase64(strEnc);
         BOOST_CHECK_EQUAL(strDec, vstrIn[i]);
     }
+
+    // Decoding strings with embedded NUL characters should fail
+    bool failure;
+    (void)DecodeBase64(std::string("invalid", 7), &failure);
+    BOOST_CHECK_EQUAL(failure, true);
+    (void)DecodeBase64(std::string("nQB/pZw=", 8), &failure);
+    BOOST_CHECK_EQUAL(failure, false);
+    (void)DecodeBase64(std::string("nQB/pZw=\0invalid", 16), &failure);
+    BOOST_CHECK_EQUAL(failure, true);
+    (void)DecodeBase64(std::string("nQB/pZw=invalid", 15), &failure);
+    BOOST_CHECK_EQUAL(failure, true);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/fuzz/base_encode_decode.cpp b/src/test/fuzz/base_encode_decode.cpp
@@ -0,0 +1,47 @@
+// Copyright (c) 2019 The Bitcoin Core developers
+// 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 <base58.h>
+#include <util/string.h>
+#include <util/strencodings.h>
+
+#include <cassert>
+#include <cstdint>
+#include <string>
+#include <vector>
+
+void test_one_input(const std::vector<uint8_t>& buffer)
+{
+    const std::string random_encoded_string(buffer.begin(), buffer.end());
+
+    std::vector<unsigned char> decoded;
+    if (DecodeBase58(random_encoded_string, decoded, 100)) {
+        const std::string encoded_string = EncodeBase58(decoded);
+        assert(encoded_string == TrimString(encoded_string));
+        assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string)));
+    }
+
+    if (DecodeBase58Check(random_encoded_string, decoded, 100)) {
+        const std::string encoded_string = EncodeBase58Check(decoded);
+        assert(encoded_string == TrimString(encoded_string));
+        assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string)));
+    }
+
+    bool pf_invalid;
+    std::string decoded_string = DecodeBase32(random_encoded_string, &pf_invalid);
+    if (!pf_invalid) {
+        const std::string encoded_string = EncodeBase32(decoded_string);
+        assert(encoded_string == TrimString(encoded_string));
+        assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string)));
+    }
+
+    decoded_string = DecodeBase64(random_encoded_string, &pf_invalid);
+    if (!pf_invalid) {
+        const std::string encoded_string = EncodeBase64(decoded_string);
+        assert(encoded_string == TrimString(encoded_string));
+        assert(ToLower(encoded_string) == ToLower(TrimString(random_encoded_string)));
+    }
+}
diff --git a/src/test/fuzz/hex.cpp b/src/test/fuzz/hex.cpp
@@ -0,0 +1,22 @@
+// Copyright (c) 2019 The Bitcoin Core developers
+// 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 <util/strencodings.h>
+
+#include <cassert>
+#include <cstdint>
+#include <string>
+#include <vector>
+
+void test_one_input(const std::vector<uint8_t>& buffer)
+{
+    const std::string random_hex_string(buffer.begin(), buffer.end());
+    const std::vector<unsigned char> data = ParseHex(random_hex_string);
+    const std::string hex_data = HexStr(data);
+    if (IsHex(random_hex_string)) {
+        assert(ToLower(random_hex_string) == hex_data);
+    }
+}
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
@@ -975,6 +975,12 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney)
     BOOST_CHECK_EQUAL(ret, COIN);
     BOOST_CHECK(ParseMoney("1", ret));
     BOOST_CHECK_EQUAL(ret, COIN);
+    BOOST_CHECK(ParseMoney("   1", ret));
+    BOOST_CHECK_EQUAL(ret, COIN);
+    BOOST_CHECK(ParseMoney("1   ", ret));
+    BOOST_CHECK_EQUAL(ret, COIN);
+    BOOST_CHECK(ParseMoney("  1 ", ret));
+    BOOST_CHECK_EQUAL(ret, COIN);
     BOOST_CHECK(ParseMoney("0.1", ret));
     BOOST_CHECK_EQUAL(ret, COIN/10);
     BOOST_CHECK(ParseMoney("0.01", ret));
@@ -991,12 +997,37 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney)
     BOOST_CHECK_EQUAL(ret, COIN/10000000);
     BOOST_CHECK(ParseMoney("0.00000001", ret));
     BOOST_CHECK_EQUAL(ret, COIN/100000000);
+    BOOST_CHECK(ParseMoney(" 0.00000001 ", ret));
+    BOOST_CHECK_EQUAL(ret, COIN/100000000);
+    BOOST_CHECK(ParseMoney("0.00000001 ", ret));
+    BOOST_CHECK_EQUAL(ret, COIN/100000000);
+    BOOST_CHECK(ParseMoney(" 0.00000001", ret));
+    BOOST_CHECK_EQUAL(ret, COIN/100000000);
+
+    // Parsing amount that can not be represented in ret should fail
+    BOOST_CHECK(!ParseMoney("0.000000001", ret));
+
+    // Parsing empty string should fail
+    BOOST_CHECK(!ParseMoney("", ret));
+    BOOST_CHECK(!ParseMoney(" ", ret));
+    BOOST_CHECK(!ParseMoney("  ", ret));
+
+    // Parsing two numbers should fail
+    BOOST_CHECK(!ParseMoney("1 2", ret));
+    BOOST_CHECK(!ParseMoney(" 1 2 ", ret));
+    BOOST_CHECK(!ParseMoney(" 1.2 3 ", ret));
+    BOOST_CHECK(!ParseMoney(" 1 2.3 ", ret));
 
     // Attempted 63 bit overflow should fail
     BOOST_CHECK(!ParseMoney("92233720368.54775808", ret));
 
     // Parsing negative amounts must fail
     BOOST_CHECK(!ParseMoney("-1", ret));
+
+    // Parsing strings with embedded NUL characters should fail
+    BOOST_CHECK(!ParseMoney(std::string("\0-1", 3), ret));
+    BOOST_CHECK(!ParseMoney(std::string("\01", 2), ret));
+    BOOST_CHECK(!ParseMoney(std::string("1\0", 2), ret));
 }
 
 BOOST_AUTO_TEST_CASE(util_IsHex)