From 0bac52d5cfaf1a3beb99b780ed7446e2faba877f Mon Sep 17 00:00:00 2001 From: Pttn <28868425+Pttn@users.noreply.github.com> Date: Sun, 16 Apr 2023 23:47:15 +0200 Subject: [PATCH 1/5] Don't return OutputType::UNKNOWN in ParseOutputType Fixes https://github.com/bitcoin/bitcoin/issues/27472 Signed-off-by: Pttn <28868425+Pttn@users.noreply.github.com> Github-Pull: #27473 Rebased-From: 0d6383fda04a99726654945a737bbb1369e0e44a --- src/outputtype.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/outputtype.cpp b/src/outputtype.cpp index 9ab29022562..e95ec7f4d36 100644 --- a/src/outputtype.cpp +++ b/src/outputtype.cpp @@ -32,8 +32,6 @@ std::optional ParseOutputType(const std::string& type) return OutputType::BECH32; } else if (type == OUTPUT_TYPE_STRING_BECH32M) { return OutputType::BECH32M; - } else if (type == OUTPUT_TYPE_STRING_UNKNOWN) { - return OutputType::UNKNOWN; } return std::nullopt; } From c40b1da2fd64bb10f120f85966b44f0d2bb315f8 Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 14 Apr 2023 10:45:34 +0100 Subject: [PATCH 2/5] depends: fix compiling bdb with clang-16 on aarch64 Compiling bdb with clang-16 on aarch64 (hardware) currently fails: ```bash make -C depends/ bdb CC=clang CXX=clang++ ... checking for mutexes... UNIX/fcntl configure: WARNING: NO SHARED LATCH IMPLEMENTATION FOUND FOR THIS PLATFORM. configure: error: Unable to find a mutex implementation ``` Looking at config.log we've got: ```bash configure:18704: checking for mutexes configure:18815: clang -o conftest -pipe -std=c11 -O2 -Wno-error=implicit-function-declaration -Wno-error=format-security -I/bitcoin/depends/aarch64-unknown-linux-gnu/include -D_GNU_SOURCE -D_REENTRANT -L/bitcoin/depends/aarch64-unknown-linux-gnu/lib conftest.c -lpthread >&5 conftest.c:45:1: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int] main() { ^ int conftest.c:50:2: warning: call to undeclared library function 'exit' with type 'void (int) __attribute__((noreturn))'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] exit ( ^ conftest.c:50:2: note: include the header or explicitly provide a declaration for 'exit' 1 warning and 1 error generated. ``` Clang-16 changed `-Wimplicit-function-declaration` and `-Wimplicit-int` warnings into errors, see: https://releases.llvm.org/16.0.0/tools/clang/docs/ReleaseNotes.html#potentially-breaking-changes. > The -Wimplicit-function-declaration and -Wimplicit-int warnings now > default to an error in C99, C11, and C17. As of C2x, support for implicit > function declarations and implicit int has been removed, and the > warning options will have no effect. Specifying -Wimplicit-int in > C89 mode will now issue warnings instead of being a noop. Github-Pull: #27462 Rebased-From: f8b8458276983f8fc1e2a47c4d00c1e30633067d --- depends/packages/bdb.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/depends/packages/bdb.mk b/depends/packages/bdb.mk index 2370c5b759a..c650c9bf44f 100644 --- a/depends/packages/bdb.mk +++ b/depends/packages/bdb.mk @@ -14,7 +14,7 @@ $(package)_config_opts_freebsd=--with-pic $(package)_config_opts_netbsd=--with-pic $(package)_config_opts_openbsd=--with-pic $(package)_config_opts_android=--with-pic -$(package)_cflags+=-Wno-error=implicit-function-declaration -Wno-error=format-security +$(package)_cflags+=-Wno-error=implicit-function-declaration -Wno-error=format-security -Wno-error=implicit-int $(package)_cppflags_mingw32=-DUNICODE -D_UNICODE endef From 3a26b19df25ca99a9a58ae5398f6f423ac074368 Mon Sep 17 00:00:00 2001 From: pablomartin4btc Date: Fri, 14 Apr 2023 19:03:08 -0300 Subject: [PATCH 3/5] bugfix: rest: avoid segfault for invalid URI `evhttp_uri_parse` can return a nullptr, for example when the URI contains invalid characters (e.g. "%"). `GetQueryParameterFromUri` passes the output of `evhttp_uri_parse` straight into `evhttp_uri_get_query`, which means that anyone calling a REST endpoint in which query parameters are used (e.g. `rest_headers`) can cause a segfault. This bugfix is designed to be minimal and without additional behaviour change. Github-Pull: #27468 Rebased-From: 11422cc5720c8d73a87600de8fe8abb156db80dc --- src/httpserver.cpp | 3 +++ src/rest.cpp | 12 ++++++++++-- src/test/httpserver_tests.cpp | 4 ++++ test/functional/interface_rest.py | 4 ++++ 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index e68436cc2c9..fce15bf4dfb 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -652,6 +652,9 @@ std::optional HTTPRequest::GetQueryParameter(const std::string& key std::optional GetQueryParameterFromUri(const char* uri, const std::string& key) { evhttp_uri* uri_parsed{evhttp_uri_parse(uri)}; + if (!uri_parsed) { + throw std::runtime_error("URI parsing failed, it likely contained RFC 3986 invalid characters"); + } const char* query{evhttp_uri_get_query(uri_parsed)}; std::optional result; diff --git a/src/rest.cpp b/src/rest.cpp index 7f00db2222c..56b6fbd175e 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -200,7 +200,11 @@ static bool rest_headers(const std::any& context, } else if (path.size() == 1) { // new path with query parameter: /rest/headers/?count= hashStr = path[0]; - raw_count = req->GetQueryParameter("count").value_or("5"); + try { + raw_count = req->GetQueryParameter("count").value_or("5"); + } catch (const std::runtime_error& e) { + return RESTERR(req, HTTP_BAD_REQUEST, e.what()); + } } else { return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/headers/.?count="); } @@ -369,7 +373,11 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const } else if (uri_parts.size() == 2) { // new path with query parameter: /rest/blockfilterheaders//?count= raw_blockhash = uri_parts[1]; - raw_count = req->GetQueryParameter("count").value_or("5"); + try { + raw_count = req->GetQueryParameter("count").value_or("5"); + } catch (const std::runtime_error& e) { + return RESTERR(req, HTTP_BAD_REQUEST, e.what()); + } } else { return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/blockfilterheaders//.?count="); } diff --git a/src/test/httpserver_tests.cpp b/src/test/httpserver_tests.cpp index ee59ec6967f..c95a777e80c 100644 --- a/src/test/httpserver_tests.cpp +++ b/src/test/httpserver_tests.cpp @@ -34,5 +34,9 @@ BOOST_AUTO_TEST_CASE(test_query_parameters) // Invalid query string syntax is the same as not having parameters uri = "/rest/endpoint/someresource.json&p1=v1&p2=v2"; BOOST_CHECK(!GetQueryParameterFromUri(uri.c_str(), "p1").has_value()); + + // URI with invalid characters (%) raises a runtime error regardless of which query parameter is queried + uri = "/rest/endpoint/someresource.json&p1=v1&p2=v2%"; + BOOST_CHECK_EXCEPTION(GetQueryParameterFromUri(uri.c_str(), "p1"), std::runtime_error, HasReason("URI parsing failed, it likely contained RFC 3986 invalid characters")); } BOOST_AUTO_TEST_SUITE_END() diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index f36bbda3afd..cb1fbdfb7a2 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -281,6 +281,10 @@ class RESTTest (BitcoinTestFramework): assert_equal(len(json_obj), 1) # ensure that there is one header in the json response assert_equal(json_obj[0]['hash'], bb_hash) # request/response hash should be the same + # Check invalid uri (% symbol at the end of the request) + resp = self.test_rest_request(f"/headers/{bb_hash}%", ret_type=RetType.OBJ, status=400) + assert_equal(resp.read().decode('utf-8').rstrip(), "URI parsing failed, it likely contained RFC 3986 invalid characters") + # Compare with normal RPC block response rpc_block_json = self.nodes[0].getblock(bb_hash) for key in ['hash', 'confirmations', 'height', 'version', 'merkleroot', 'time', 'nonce', 'bits', 'difficulty', 'chainwork', 'previousblockhash']: From fc8c1a8deb80913ff353c878f494c9eaf28061c0 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Sun, 19 Mar 2023 10:23:45 -0700 Subject: [PATCH 4/5] doc: fix/improve warning helps in {create,load,unload,restore}wallet - clarify that there can be multiple warning messages - specify the correct wallet action - describe the use of newlines as delimiters Github-Pull: #27279 Rebased-From: f73782a9032a462a71569e9424db9bf9eeababf3 --- src/wallet/rpc/backup.cpp | 2 +- src/wallet/rpc/wallet.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index a971331a706..bebd47356a3 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1886,7 +1886,7 @@ RPCHelpMan restorewallet() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "name", "The wallet name if restored successfully."}, - {RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."}, + {RPCResult::Type::STR, "warning", "Warning messages, if any, related to restoring the wallet. Multiple messages will be delimited by newlines."}, } }, RPCExamples{ diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index a2ae078343a..dfa136e4421 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -207,7 +207,7 @@ static RPCHelpMan loadwallet() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "name", "The wallet name if loaded successfully."}, - {RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."}, + {RPCResult::Type::STR, "warning", "Warning messages, if any, related to loading the wallet. Multiple messages will be delimited by newlines."}, } }, RPCExamples{ @@ -327,7 +327,7 @@ static RPCHelpMan createwallet() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "name", "The wallet name if created successfully. If the wallet was created using a full path, the wallet_name will be the full path."}, - {RPCResult::Type::STR, "warning", "Warning message if wallet was not loaded cleanly."}, + {RPCResult::Type::STR, "warning", "Warning messages, if any, related to creating the wallet. Multiple messages will be delimited by newlines."}, } }, RPCExamples{ @@ -414,7 +414,7 @@ static RPCHelpMan unloadwallet() {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::STR, "warning", "Warning message if wallet was not unloaded cleanly."}, + {RPCResult::Type::STR, "warning", "Warning messages, if any, related to unloading the wallet. Multiple messages will be delimited by newlines."}, }}, RPCExamples{ HelpExampleCli("unloadwallet", "wallet_name") From dc711fbd32653b09e196f72942106114a32353f4 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 17 Apr 2023 15:15:35 +0100 Subject: [PATCH 5/5] doc: update 24.1 release notes --- doc/release-notes.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/release-notes.md b/doc/release-notes.md index 19f604c66bf..970e2486187 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -45,10 +45,13 @@ unsupported systems. ### RPC and other APIs - #26515 rpc: Require NodeStateStats object in getpeerinfo +- #27279 doc: fix/improve warning helps in {create,load,unload,restore}wallet +- #27468 rest: avoid segfault for invalid URI ### Build System - #26944 depends: fix systemtap download URL +- #27462 depends: fix compiling bdb with clang-16 on aarch64 ### Wallet @@ -58,6 +61,7 @@ unsupported systems. - #26761 wallet: fully migrate address book entries for watchonly/solvable wallets - #27053 wallet: reuse change dest when re-creating TX with avoidpartialspends - #27080 wallet: Zero out wallet master key upon locking so it doesn't persist in memory +- #27473 wallet: Properly handle "unknown" Address Type ### GUI changes @@ -77,11 +81,14 @@ Thanks to everyone who directly contributed to this release: - Andrew Chow - Hennadii Stepanov - John Moffett +- Jon Atack - Marco Falke - Martin Zumsande - Matthew Zipkin - Michael Ford +- pablomartin4btc - Sebastian Falbesoner +- Thomas Nguyen - Vasil Dimov As well as to everyone that helped with translations on