diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 7b49bad3f52..95e1ba4dd95 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -43,9 +43,7 @@ RPCHelpMan getnewaddress() } // Parse the label first so we don't generate a key if there's an error - std::string label; - if (!request.params[0].isNull()) - label = LabelFromValue(request.params[0]); + const std::string label{LabelFromValue(request.params[0])}; OutputType output_type = pwallet->m_default_address_type; if (!request.params[1].isNull()) { @@ -140,7 +138,7 @@ RPCHelpMan setlabel() throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address"); } - std::string label = LabelFromValue(request.params[1]); + const std::string label{LabelFromValue(request.params[1])}; if (pwallet->IsMine(dest)) { pwallet->SetAddressBook(dest, label, "receive"); @@ -258,9 +256,7 @@ RPCHelpMan addmultisigaddress() LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); - std::string label; - if (!request.params[2].isNull()) - label = LabelFromValue(request.params[2]); + const std::string label{LabelFromValue(request.params[2])}; int required = request.params[0].getInt(); @@ -662,7 +658,7 @@ RPCHelpMan getaddressesbylabel() LOCK(pwallet->cs_wallet); - std::string label = LabelFromValue(request.params[0]); + const std::string label{LabelFromValue(request.params[0])}; // Find all addresses that have the given label UniValue ret(UniValue::VOBJ); diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 95117b6c19e..64253789eca 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -156,9 +156,7 @@ RPCHelpMan importprivkey() EnsureWalletIsUnlocked(*pwallet); std::string strSecret = request.params[0].get_str(); - std::string strLabel; - if (!request.params[1].isNull()) - strLabel = request.params[1].get_str(); + const std::string strLabel{LabelFromValue(request.params[1])}; // Whether to perform rescan after import if (!request.params[2].isNull()) @@ -249,9 +247,7 @@ RPCHelpMan importaddress() EnsureLegacyScriptPubKeyMan(*pwallet, true); - std::string strLabel; - if (!request.params[1].isNull()) - strLabel = request.params[1].get_str(); + const std::string strLabel{LabelFromValue(request.params[1])}; // Whether to perform rescan after import bool fRescan = true; @@ -442,9 +438,7 @@ RPCHelpMan importpubkey() EnsureLegacyScriptPubKeyMan(*pwallet, true); - std::string strLabel; - if (!request.params[1].isNull()) - strLabel = request.params[1].get_str(); + const std::string strLabel{LabelFromValue(request.params[1])}; // Whether to perform rescan after import bool fRescan = true; @@ -1170,7 +1164,7 @@ static UniValue ProcessImport(CWallet& wallet, const UniValue& data, const int64 if (internal && data.exists("label")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal addresses should not have a label"); } - const std::string& label = data.exists("label") ? data["label"].get_str() : ""; + const std::string label{LabelFromValue(data["label"])}; const bool add_keypool = data.exists("keypool") ? data["keypool"].get_bool() : false; // Add to keypool only works with privkeys disabled @@ -1464,7 +1458,7 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c const std::string& descriptor = data["desc"].get_str(); const bool active = data.exists("active") ? data["active"].get_bool() : false; const bool internal = data.exists("internal") ? data["internal"].get_bool() : false; - const std::string& label = data.exists("label") ? data["label"].get_str() : ""; + const std::string label{LabelFromValue(data["label"])}; // Parse descriptor string FlatSigningProvider keys; diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index c257af13c4f..f571f8bcb2a 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -486,7 +486,7 @@ RPCHelpMan listtransactions() std::optional filter_label; if (!request.params[0].isNull() && request.params[0].get_str() != "*") { - filter_label = request.params[0].get_str(); + filter_label.emplace(LabelFromValue(request.params[0])); if (filter_label.value().empty()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Label argument must be a valid label name or \"*\"."); } @@ -634,10 +634,9 @@ RPCHelpMan listsinceblock() bool include_removed = (request.params[3].isNull() || request.params[3].get_bool()); bool include_change = (!request.params[4].isNull() && request.params[4].get_bool()); + // Only set it if 'label' was provided. std::optional filter_label; - if (!request.params[5].isNull()) { - filter_label = request.params[5].get_str(); - } + if (!request.params[5].isNull()) filter_label.emplace(LabelFromValue(request.params[5])); int depth = height ? wallet.GetLastBlockHeight() + 1 - *height : -1; diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index e9607791a90..af3497e8a16 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -132,7 +132,10 @@ const LegacyScriptPubKeyMan& EnsureConstLegacyScriptPubKeyMan(const CWallet& wal std::string LabelFromValue(const UniValue& value) { - std::string label = value.get_str(); + static const std::string empty_string; + if (value.isNull()) return empty_string; + + const std::string& label{value.get_str()}; if (label == "*") throw JSONRPCError(RPC_WALLET_INVALID_LABEL_NAME, "Invalid label name"); return label; diff --git a/test/functional/wallet_labels.py b/test/functional/wallet_labels.py index 34acaee563b..f9b951d4436 100755 --- a/test/functional/wallet_labels.py +++ b/test/functional/wallet_labels.py @@ -28,6 +28,44 @@ class WalletLabelsTest(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() + def invalid_label_name_test(self): + node = self.nodes[0] + address = node.getnewaddress() + pubkey = node.getaddressinfo(address)['pubkey'] + rpc_calls = [ + [node.getnewaddress], + [node.setlabel, address], + [node.getaddressesbylabel], + [node.importpubkey, pubkey], + [node.addmultisigaddress, 1, [pubkey]], + [node.getreceivedbylabel], + [node.listsinceblock, node.getblockhash(0), 1, False, True, False], + ] + if self.options.descriptors: + response = node.importdescriptors([{ + 'desc': f'pkh({pubkey})', + 'label': '*', + 'timestamp': 'now', + }]) + else: + rpc_calls.extend([ + [node.importprivkey, node.dumpprivkey(address)], + [node.importaddress, address], + ]) + + response = node.importmulti([{ + 'scriptPubKey': {'address': address}, + 'label': '*', + 'timestamp': 'now', + }]) + + assert_equal(response[0]['success'], False) + assert_equal(response[0]['error']['code'], -11) + assert_equal(response[0]['error']['message'], "Invalid label name") + + for rpc_call in rpc_calls: + assert_raises_rpc_error(-11, "Invalid label name", *rpc_call, "*") + def run_test(self): # Check that there's no UTXO on the node node = self.nodes[0] @@ -138,6 +176,8 @@ class WalletLabelsTest(BitcoinTestFramework): # in the label. This is a no-op. change_label(node, labels[2].addresses[0], labels[2], labels[2]) + self.invalid_label_name_test() + if self.options.descriptors: # This is a descriptor wallet test because of segwit v1+ addresses self.log.info('Check watchonly labels')