Merge pull request #4925 from PastaPastaPasta/develop-trivial-2022-07-17

backport: trivial backports

.travis.yml

@@ -233,16 +233,16 @@ after_success:
       env: >-
         FILE_ENV="./ci/test/00_setup_env_i686.sh"
 
+    - stage: test
+      name: 'x86_64 Linux  [GOAL: install]  [CentOS 7]  [no depends, only system libs]'
+      env: >-
+        FILE_ENV="./ci/test/00_setup_env_native_centos.sh"
+
     - stage: test
       name: 'x86_64 Linux  [GOAL: install]  [bionic]  [uses qt5 dev package instead of depends Qt to speed up build and avoid timeout] [unsigned char]'
       env: >-
         FILE_ENV="./ci/test/00_setup_env_native_qt5.sh"
 # x86_64 Linux (xenial, no depends, only system libs, sanitizers: thread (TSan))
-    - stage: test
-      name: 'x86_64 Linux  [GOAL: install]  [trusty]  [no functional tests, no depends, only system libs]'
-      env: >-
-        FILE_ENV="./ci/test/00_setup_env_native_trusty.sh"
-
     - stage: test
       name: 'x86_64 Linux  [GOAL: install]  [xenial]  [no depends, only system libs, sanitizers: thread (TSan), no wallet]'
       env: >-

build-aux/m4/l_socket.m4

@@ -0,0 +1,36 @@
+# Illumos/SmartOS requires linking with -lsocket if
+# using getifaddrs & freeifaddrs
+
+m4_define([_CHECK_SOCKET_testbody], [[
+  #include <sys/types.h>
+  #include <ifaddrs.h>
+
+  int main() {
+    struct ifaddrs *ifaddr;
+    getifaddrs(&ifaddr);
+    freeifaddrs(ifaddr);
+  }
+]])
+
+AC_DEFUN([CHECK_SOCKET], [
+
+  AC_LANG_PUSH(C++)
+
+  AC_MSG_CHECKING([whether ifaddrs funcs can be used without link library])
+
+  AC_LINK_IFELSE([AC_LANG_SOURCE([_CHECK_SOCKET_testbody])],[
+      AC_MSG_RESULT([yes])
+    ],[
+      AC_MSG_RESULT([no])
+      LIBS="$LIBS -lsocket"
+      AC_MSG_CHECKING([whether getifaddrs needs -lsocket])
+      AC_LINK_IFELSE([AC_LANG_SOURCE([_CHECK_SOCKET_testbody])],[
+          AC_MSG_RESULT([yes])
+        ],[
+          AC_MSG_RESULT([no])
+          AC_MSG_FAILURE([cannot figure out how to use getifaddrs])
+        ])
+    ])
+
+  AC_LANG_POP
+])

ci/test/00_setup_env_native_centos.sh

@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+#
+# 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.
+
+export LC_ALL=C.UTF-8
+
+export DOCKER_NAME_TAG=centos:7
+export DOCKER_PACKAGES="gcc-c++ libtool make git python3 python36-zmq"
+export PACKAGES="boost-devel libevent-devel libdb4-devel libdb4-cxx-devel miniupnpc-devel zeromq-devel qt5-qtbase-devel qt5-qttools-devel qrencode-devel"
+export NO_DEPENDS=1
+export GOAL="install"
+export BITCOIN_CONFIG="--enable-reduce-exports"

ci/test/04_install.sh

@@ -6,12 +6,17 @@
 
 export LC_ALL=C.UTF-8
 
+if [[ $DOCKER_NAME_TAG == centos* ]]; then
+  export LC_ALL=en_US.utf8
+fi
+
 if [ "$TRAVIS_OS_NAME" == "osx" ]; then
   set +o errexit
   pushd /usr/local/Homebrew || exit 1
   git reset --hard origin/master
   popd || exit 1
   set -o errexit
+  ${CI_RETRY_EXE} brew unlink python@2
   ${CI_RETRY_EXE} brew update
   # brew upgrade returns an error if any of the packages is already up to date
   # Failure is safe to ignore, unless we really need an update.
@@ -81,7 +86,10 @@ if [ -n "$DPKG_ADD_ARCH" ]; then
   DOCKER_EXEC dpkg --add-architecture "$DPKG_ADD_ARCH"
 fi
 
-if [ "$TRAVIS_OS_NAME" != "osx" ]; then
+if [[ $DOCKER_NAME_TAG == centos* ]]; then
+  ${CI_RETRY_EXE} DOCKER_EXEC yum -y install epel-release
+  ${CI_RETRY_EXE} DOCKER_EXEC yum -y install $DOCKER_PACKAGES $PACKAGES
+elif [ "$TRAVIS_OS_NAME" != "osx" ]; then
   ${CI_RETRY_EXE} DOCKER_EXEC apt-get update
   ${CI_RETRY_EXE} DOCKER_EXEC apt-get install --no-install-recommends --no-upgrade -y $PACKAGES $DOCKER_PACKAGES
 fi

configure.ac

@@ -935,7 +935,7 @@ fi
 
 AC_CHECK_HEADERS([endian.h sys/endian.h byteswap.h stdio.h stdlib.h unistd.h strings.h sys/types.h sys/stat.h sys/select.h sys/prctl.h sys/sysctl.h vm/vm_param.h sys/vmmeter.h sys/resources.h])
 
-AC_CHECK_DECLS([getifaddrs, freeifaddrs],,,
+AC_CHECK_DECLS([getifaddrs, freeifaddrs],[CHECK_SOCKET],,
     [#include <sys/types.h>
     #include <ifaddrs.h>]
 )

contrib/devtools/test_deterministic_coverage.sh

@@ -17,7 +17,6 @@ GCOV_EXECUTABLE="gcov"
 NON_DETERMINISTIC_TESTS=(
     "blockfilter_index_tests/blockfilter_index_initial_sync"  # src/checkqueue.h: In CCheckQueue::Loop(): while (queue.empty()) { ... }
     "coinselector_tests/knapsack_solver_test"                 # coinselector_tests.cpp: if (equal_sets(setCoinsRet, setCoinsRet2))
-    "denialofservice_tests/DoS_mapOrphans"                    # denialofservice_tests.cpp: it = mapOrphanTransactions.lower_bound(InsecureRand256());
     "fs_tests/fsbridge_fstream"                               # deterministic test failure?
     "miner_tests/CreateNewBlock_validity"                     # validation.cpp: if (GetMainSignals().CallbacksPending() > 10)
     "scheduler_tests/manythreads"                             # scheduler.cpp: CScheduler::serviceQueue()

src/net.cpp

@@ -38,6 +38,10 @@
 #include <fcntl.h>
 #endif
 
+#if HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS
+#include <ifaddrs.h>
+#endif
+
 #ifdef USE_POLL
 #include <poll.h>
 #endif

src/randomenv.cpp

@@ -38,7 +38,7 @@
 #include <sys/utsname.h>
 #include <unistd.h>
 #endif
-#if HAVE_DECL_GETIFADDRS
+#if HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS
 #include <ifaddrs.h>
 #endif
 #if HAVE_SYSCTL
@@ -361,7 +361,7 @@ void RandAddStaticEnv(CSHA512& hasher)
         hasher.Write((const unsigned char*)hname, strnlen(hname, 256));
     }
 
-#if HAVE_DECL_GETIFADDRS
+#if HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS
     // Network interfaces
     struct ifaddrs *ifad = NULL;
     getifaddrs(&ifad);

src/streams.h

@@ -819,18 +819,6 @@ public:
         return true;
     }
 
-    bool Seek(uint64_t nPos) {
-        long nLongPos = nPos;
-        if (nPos != (uint64_t)nLongPos)
-            return false;
-        if (fseek(src, nLongPos, SEEK_SET))
-            return false;
-        nLongPos = ftell(src);
-        nSrcPos = nLongPos;
-        nReadPos = nLongPos;
-        return true;
-    }
-
     //! prevent reading beyond a certain position
     //! no argument removes the limit
     bool SetLimit(uint64_t nPos = std::numeric_limits<uint64_t>::max()) {

src/test/denialofservice_tests.cpp

@@ -4,10 +4,12 @@
 
 // Unit tests for denial-of-service detection/prevention code
 
+#include <arith_uint256.h>
 #include <banman.h>
 #include <chainparams.h>
 #include <net.h>
 #include <net_processing.h>
+#include <pubkey.h>
 #include <script/sign.h>
 #include <script/signingprovider.h>
 #include <script/standard.h>
@@ -361,10 +363,26 @@ static CTransactionRef RandomOrphan()
     return it->second.tx;
 }
 
+static void MakeNewKeyWithFastRandomContext(CKey& key)
+{
+    std::vector<unsigned char> keydata;
+    keydata = g_insecure_rand_ctx.randbytes(32);
+    key.Set(keydata.data(), keydata.data() + keydata.size(), /*fCompressedIn*/ true);
+    assert(key.IsValid());
+}
+
 BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
 {
+    // This test had non-deterministic coverage due to
+    // randomly selected seeds.
+    // This seed is chosen so that all branches of the function
+    // ecdsa_signature_parse_der_lax are executed during this test.
+    // Specifically branches that run only when an ECDSA
+    // signature's R and S values have leading zeros.
+    g_insecure_rand_ctx = FastRandomContext(ArithToUint256(arith_uint256(33)));
+
     CKey key;
-    key.MakeNewKey(true);
+    MakeNewKeyWithFastRandomContext(key);
     FillableSigningProvider keystore;
     BOOST_CHECK(keystore.AddKey(key));
 

src/test/fuzz/buffered_file.cpp

@@ -31,7 +31,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     if (opt_buffered_file && fuzzed_file != nullptr) {
         bool setpos_fail = false;
         while (fuzzed_data_provider.ConsumeBool()) {
-            switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 5)) {
+            switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 4)) {
             case 0: {
                 std::array<uint8_t, 4096> arr{};
                 try {
@@ -41,20 +41,16 @@ void test_one_input(const std::vector<uint8_t>& buffer)
                 break;
             }
             case 1: {
-                opt_buffered_file->Seek(fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096));
-                break;
-            }
-            case 2: {
                 opt_buffered_file->SetLimit(fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096));
                 break;
             }
-            case 3: {
+            case 2: {
                 if (!opt_buffered_file->SetPos(fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096))) {
                     setpos_fail = true;
                 }
                 break;
             }
-            case 4: {
+            case 3: {
                 if (setpos_fail) {
                     // Calling FindByte(...) after a failed SetPos(...) call may result in an infinite loop.
                     break;
@@ -65,7 +61,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
                 }
                 break;
             }
-            case 5: {
+            case 4: {
                 ReadFromStream(fuzzed_data_provider, *opt_buffered_file);
                 break;
             }

src/test/fuzz/http_request.cpp

@@ -7,6 +7,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <util/strencodings.h>
 
 #include <event2/buffer.h>
 #include <event2/http.h>
@@ -31,7 +32,14 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     assert(evbuf != nullptr);
     const std::vector<uint8_t> http_buffer = ConsumeRandomLengthByteVector(fuzzed_data_provider, 4096);
     evbuffer_add(evbuf, http_buffer.data(), http_buffer.size());
-    if (evhttp_parse_firstline_(evreq, evbuf) != 1 || evhttp_parse_headers_(evreq, evbuf) != 1) {
+    // Avoid constructing requests that will be interpreted by libevent as PROXY requests to avoid triggering
+    // a nullptr dereference. The dereference (req->evcon->http_server) takes place in evhttp_parse_request_line
+    // and is a consequence of our hacky but necessary use of the internal function evhttp_parse_firstline_ in
+    // this fuzzing harness. The workaround is not aesthetically pleasing, but it successfully avoids the troublesome
+    // code path. " http:// HTTP/1.1\n" was a crashing input prior to this workaround.
+    const std::string http_buffer_str = ToLower({http_buffer.begin(), http_buffer.end()});
+    if (http_buffer_str.find(" http://") != std::string::npos || http_buffer_str.find(" https://") != std::string::npos ||
+        evhttp_parse_firstline_(evreq, evbuf) != 1 || evhttp_parse_headers_(evreq, evbuf) != 1) {
         evbuffer_free(evbuf);
         evhttp_request_free(evreq);
         return;

src/test/fuzz/string.cpp

@@ -45,7 +45,8 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     (void)CopyrightHolders(random_string_1, fuzzed_data_provider.ConsumeIntegral<unsigned int>(), fuzzed_data_provider.ConsumeIntegral<unsigned int>());
     FeeEstimateMode fee_estimate_mode;
     (void)FeeModeFromString(random_string_1, fee_estimate_mode);
-    (void)FormatParagraph(random_string_1, fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 1000), fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 1000));
+    const auto width{fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 1000)};
+    (void)FormatParagraph(random_string_1, width, fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, width));
     (void)FormatSubVersion(random_string_1, fuzzed_data_provider.ConsumeIntegral<int>(), random_string_vector);
     (void)GetDescriptorChecksum(random_string_1);
     (void)HelpExampleCli(random_string_1, random_string_2);

src/util/settings.cpp

@@ -60,9 +60,15 @@ bool ReadSettings(const fs::path& path, std::map<std::string, SettingsValue>& va
     values.clear();
     errors.clear();
 
+    // Ok for file to not exist
+    if (!fs::exists(path)) return true;
+
     fsbridge::ifstream file;
     file.open(path);
-    if (!file.is_open()) return true; // Ok for file not to exist.
+    if (!file.is_open()) {
+      errors.emplace_back(strprintf("%s. Please check permissions.", path.string()));
+      return false;
+    }
 
     SettingsValue in;
     if (!in.read(std::string{std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()})) {
@@ -106,7 +112,7 @@ bool WriteSettings(const fs::path& path,
         errors.emplace_back(strprintf("Error: Unable to open settings file %s for writing", path.string()));
         return false;
     }
-    file << out.write(/* prettyIndent= */ 1, /* indentLevel= */ 4) << std::endl;
+    file << out.write(/* prettyIndent= */ 4, /* indentLevel= */ 1) << std::endl;
     file.close();
     return true;
 }

src/util/strencodings.cpp

@@ -384,6 +384,7 @@ bool ParseDouble(const std::string& str, double *out)
 
 std::string FormatParagraph(const std::string& in, size_t width, size_t indent)
 {
+    assert(width >= indent);
     std::stringstream out;
     size_t ptr = 0;
     size_t indented = 0;

src/util/system.cpp

@@ -468,6 +468,14 @@ bool ArgsManager::ReadSettingsFile(std::vector<std::string>* errors)
         SaveErrors(read_errors, errors);
         return false;
     }
+    for (const auto& setting : m_settings.rw_settings) {
+        std::string section;
+        std::string key = setting.first;
+        (void)InterpretOption(section, key, /* value */ {}); // Split setting key into section and argname
+        if (!GetArgFlags('-' + key)) {
+            LogPrintf("Ignoring unknown rw_settings value %s\n", setting.first);
+        }
+    }
     return true;
 }
 
@@ -1344,8 +1352,9 @@ void ScheduleBatchPriority()
 {
 #ifdef SCHED_BATCH
     const static sched_param param{};
-    if (pthread_setschedparam(pthread_self(), SCHED_BATCH, &param) != 0) {
+    const int rc = pthread_setschedparam(pthread_self(), SCHED_BATCH, &param);
-        LogPrintf("Failed to pthread_setschedparam: %s\n", strerror(errno));
+    if (rc != 0) {
+        LogPrintf("Failed to pthread_setschedparam: %s\n", strerror(rc));
     }
 #endif
 }

src/util/system.h

@@ -160,7 +160,7 @@ struct SectionInfo
 class ArgsManager
 {
 public:
-    enum Flags {
+    enum Flags : uint32_t {
         // Boolean options can accept negation syntax -noOPTION or -noOPTION=1
         ALLOW_BOOL = 0x01,
         ALLOW_INT = 0x02,

test/functional/feature_config_args.py

@@ -94,8 +94,8 @@ class ConfArgsTest(BitcoinTestFramework):
                     'Command-line arg: rpcpassword=****',
                     'Command-line arg: rpcuser=****',
                     'Command-line arg: torpassword=****',
-                    'Config file arg: regtest="1"',
+                    'Config file arg: %s="1"' % self.chain,
-                    'Config file arg: [regtest] server="1"',
+                    'Config file arg: [%s] server="1"' % self.chain,
                 ],
                 unexpected_msgs=[
                     'alice:f7efda5c189b999524f151318c0c86$d5b51b3beffbc0',

test/functional/feature_settings.py

@@ -30,19 +30,25 @@ class SettingsTest(BitcoinTestFramework):
 
         # Assert settings are parsed and logged
         with settings.open("w") as fp:
-            json.dump({"string": "string", "num": 5, "bool": True, "null": None, "list": [6,7]}, fp)
+            json.dump({"string": "string", "num": 5, "bool": True, "null": None, "list": [6, 7]}, fp)
         with node.assert_debug_log(expected_msgs=[
+                'Ignoring unknown rw_settings value bool',
+                'Ignoring unknown rw_settings value list',
+                'Ignoring unknown rw_settings value null',
+                'Ignoring unknown rw_settings value num',
+                'Ignoring unknown rw_settings value string',
                 'Setting file arg: string = "string"',
                 'Setting file arg: num = 5',
                 'Setting file arg: bool = true',
                 'Setting file arg: null = null',
-                'Setting file arg: list = [6,7]']):
+                'Setting file arg: list = [6,7]',
+        ]):
             self.start_node(0)
             self.stop_node(0)
 
         # Assert settings are unchanged after shutdown
         with settings.open() as fp:
-            assert_equal(json.load(fp), {"string": "string", "num": 5, "bool": True, "null": None, "list": [6,7]})
+            assert_equal(json.load(fp), {"string": "string", "num": 5, "bool": True, "null": None, "list": [6, 7]})
 
         # Test invalid json
         with settings.open("w") as fp: