diff --git a/browser/ui/webui/brave_wallet/line_chart/line_chart_ui.cc b/browser/ui/webui/brave_wallet/line_chart/line_chart_ui.cc index 792941c73b39..c7ad69a0f542 100644 --- a/browser/ui/webui/brave_wallet/line_chart/line_chart_ui.cc +++ b/browser/ui/webui/brave_wallet/line_chart/line_chart_ui.cc @@ -3,12 +3,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/browser/ui/webui/brave_wallet/line_chart/line_chart_ui.h" #include @@ -40,13 +34,12 @@ UntrustedLineChartUI::UntrustedLineChartUI(content::WebUI* web_ui) untrusted_source->SetDefaultResource( IDR_BRAVE_WALLET_LINE_CHART_DISPLAY_HTML); - untrusted_source->AddResourcePaths(base::make_span( - kLineChartDisplayGenerated, kLineChartDisplayGeneratedSize)); + untrusted_source->AddResourcePaths( + base::make_span(kLineChartDisplayGenerated)); untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPageURL)); untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPanelURL)); webui::SetupWebUIDataSource(untrusted_source, - base::make_span(kLineChartDisplayGenerated, - kLineChartDisplayGeneratedSize), + base::make_span(kLineChartDisplayGenerated), IDR_BRAVE_WALLET_LINE_CHART_DISPLAY_HTML); untrusted_source->OverrideContentSecurityPolicy( network::mojom::CSPDirectiveName::ScriptSrc, diff --git a/browser/ui/webui/brave_wallet/market/market_ui.cc b/browser/ui/webui/brave_wallet/market/market_ui.cc index 3e7c588da4bf..b6f273a57b98 100644 --- a/browser/ui/webui/brave_wallet/market/market_ui.cc +++ b/browser/ui/webui/brave_wallet/market/market_ui.cc @@ -3,12 +3,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/browser/ui/webui/brave_wallet/market/market_ui.h" #include @@ -38,14 +32,12 @@ UntrustedMarketUI::UntrustedMarketUI(content::WebUI* web_ui) untrusted_source->AddString(str.name, l10n_str); } untrusted_source->SetDefaultResource(IDR_BRAVE_WALLET_MARKET_DISPLAY_HTML); - untrusted_source->AddResourcePaths( - base::make_span(kMarketDisplayGenerated, kMarketDisplayGeneratedSize)); + untrusted_source->AddResourcePaths(base::make_span(kMarketDisplayGenerated)); untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPageURL)); untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPanelURL)); - webui::SetupWebUIDataSource( - untrusted_source, - base::make_span(kMarketDisplayGenerated, kMarketDisplayGeneratedSize), - IDR_BRAVE_WALLET_MARKET_DISPLAY_HTML); + webui::SetupWebUIDataSource(untrusted_source, + base::make_span(kMarketDisplayGenerated), + IDR_BRAVE_WALLET_MARKET_DISPLAY_HTML); untrusted_source->OverrideContentSecurityPolicy( network::mojom::CSPDirectiveName::ScriptSrc, diff --git a/browser/ui/webui/brave_wallet/nft/nft_ui.cc b/browser/ui/webui/brave_wallet/nft/nft_ui.cc index ee357c0fe832..258c1edee275 100644 --- a/browser/ui/webui/brave_wallet/nft/nft_ui.cc +++ b/browser/ui/webui/brave_wallet/nft/nft_ui.cc @@ -3,12 +3,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/browser/ui/webui/brave_wallet/nft/nft_ui.h" #include @@ -39,14 +33,12 @@ UntrustedNftUI::UntrustedNftUI(content::WebUI* web_ui) } untrusted_source->SetDefaultResource(IDR_BRAVE_WALLET_NFT_DISPLAY_HTML); - untrusted_source->AddResourcePaths( - base::make_span(kNftDisplayGenerated, kNftDisplayGeneratedSize)); + untrusted_source->AddResourcePaths(base::make_span(kNftDisplayGenerated)); untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPageURL)); untrusted_source->AddFrameAncestor(GURL(kBraveUIWalletPanelURL)); - webui::SetupWebUIDataSource( - untrusted_source, - base::make_span(kNftDisplayGenerated, kNftDisplayGeneratedSize), - IDR_BRAVE_WALLET_NFT_DISPLAY_HTML); + webui::SetupWebUIDataSource(untrusted_source, + base::make_span(kNftDisplayGenerated), + IDR_BRAVE_WALLET_NFT_DISPLAY_HTML); untrusted_source->OverrideContentSecurityPolicy( network::mojom::CSPDirectiveName::ScriptSrc, std::string("script-src 'self' chrome-untrusted://resources;")); diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.cc b/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.cc index 00a22f120428..cef617391268 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.cc +++ b/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.cc @@ -3,12 +3,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.h" #include diff --git a/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.h b/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.h index 8055c361cdca..a7e7c70eec6d 100644 --- a/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.h +++ b/components/brave_wallet/browser/bitcoin/bitcoin_discover_account_task.h @@ -6,7 +6,9 @@ #ifndef BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_BITCOIN_BITCOIN_DISCOVER_ACCOUNT_TASK_H_ #define BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_BITCOIN_BITCOIN_DISCOVER_ACCOUNT_TASK_H_ +#include #include +#include #include "base/memory/raw_ptr.h" #include "base/memory/weak_ptr.h" @@ -71,7 +73,7 @@ class DiscoverAccountTaskBase { uint32_t active_requests_ = 0; // Indexed by 0 and 1 for receive and change addresses discovery states // respectively. - State states_[2]; + std::array states_; bool account_is_used_ = false; mojom::BitcoinBalancePtr balance_; diff --git a/components/brave_wallet/browser/eip1559_transaction_unittest.cc b/components/brave_wallet/browser/eip1559_transaction_unittest.cc index 1742cf7e57cd..586c2c5a8347 100644 --- a/components/brave_wallet/browser/eip1559_transaction_unittest.cc +++ b/components/brave_wallet/browser/eip1559_transaction_unittest.cc @@ -9,6 +9,7 @@ #include #include +#include "base/containers/span.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/values.h" @@ -125,10 +126,10 @@ TEST(Eip1559TransactionUnitTest, GetSignedTransactionAndHash) { "0x863c02549182b91f1764714b93d7e882f010539c0907adaf4de761f7b06a713c"}}; for (const auto& entry : cases) { SCOPED_TRACE(entry.signed_tx); - std::vector private_key; - EXPECT_TRUE(base::HexStringToBytes( + std::array private_key; + EXPECT_TRUE(base::HexStringToSpan( "8f2a55949038a9610f50fb23b5883af3b4ecb3c3bb792cbcefbd1542c692be63", - &private_key)); + private_key)); HDKey key; key.SetPrivateKey(private_key); @@ -142,9 +143,9 @@ TEST(Eip1559TransactionUnitTest, GetSignedTransactionAndHash) { nullptr)); int recid; - const std::vector signature = - key.SignCompact(tx.GetHashedMessageToSign(0), &recid); - tx.ProcessSignature(signature, recid); + auto signature = key.SignCompact(tx.GetHashedMessageToSign(0), &recid); + ASSERT_TRUE(signature); + tx.ProcessSignature(*signature, recid, 0); EXPECT_EQ(tx.GetSignedTransaction(), entry.signed_tx); EXPECT_EQ(tx.GetTransactionHash(), entry.hash); } diff --git a/components/brave_wallet/browser/eip2930_transaction.cc b/components/brave_wallet/browser/eip2930_transaction.cc index a39707e40a5a..16ce5ea90e9e 100644 --- a/components/brave_wallet/browser/eip2930_transaction.cc +++ b/components/brave_wallet/browser/eip2930_transaction.cc @@ -196,7 +196,7 @@ std::string Eip2930Transaction::GetTransactionHash() const { return ToHex(KeccakHash(Serialize())); } -void Eip2930Transaction::ProcessSignature(const std::vector signature, +void Eip2930Transaction::ProcessSignature(base::span signature, int recid, uint256_t chain_id) { EthTransaction::ProcessSignature(signature, recid, chain_id_); diff --git a/components/brave_wallet/browser/eip2930_transaction.h b/components/brave_wallet/browser/eip2930_transaction.h index 6c605bfa40e2..f197edb60458 100644 --- a/components/brave_wallet/browser/eip2930_transaction.h +++ b/components/brave_wallet/browser/eip2930_transaction.h @@ -62,9 +62,9 @@ class Eip2930Transaction : public EthTransaction { // accessList, signatureYParity, signatureR, signatureS])) std::string GetTransactionHash() const override; - void ProcessSignature(const std::vector signature, + void ProcessSignature(base::span signature, int recid, - uint256_t chain_id = 0) override; + uint256_t chain_id) override; bool IsSigned() const override; diff --git a/components/brave_wallet/browser/eip2930_transaction_unittest.cc b/components/brave_wallet/browser/eip2930_transaction_unittest.cc index d5a8e082b480..ddb23f5b7f4a 100644 --- a/components/brave_wallet/browser/eip2930_transaction_unittest.cc +++ b/components/brave_wallet/browser/eip2930_transaction_unittest.cc @@ -9,6 +9,7 @@ #include #include +#include "base/containers/span.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/values.h" @@ -117,19 +118,19 @@ TEST(Eip2930TransactionUnitTest, GetSignedTransactionAndHash) { access_list->push_back(item); - std::vector private_key; - EXPECT_TRUE(base::HexStringToBytes( + std::array private_key; + EXPECT_TRUE(base::HexStringToSpan( "fad9c8855b740a0b7ed4c221dbad0f33a83a49cad6b3fe8d5817ac83d38b6a19", - &private_key)); + private_key)); HDKey key; key.SetPrivateKey(private_key); int recid; - const std::vector signature = - key.SignCompact(tx.GetHashedMessageToSign(0), &recid); + auto signature = key.SignCompact(tx.GetHashedMessageToSign(0), &recid); + ASSERT_TRUE(signature); ASSERT_FALSE(tx.IsSigned()); - tx.ProcessSignature(signature, recid); + tx.ProcessSignature(*signature, recid, 0); ASSERT_TRUE(tx.IsSigned()); EXPECT_EQ( tx.GetSignedTransaction(), diff --git a/components/brave_wallet/browser/eth_abi_decoder.cc b/components/brave_wallet/browser/eth_abi_decoder.cc index af9041a3630c..62780ba0d23e 100644 --- a/components/brave_wallet/browser/eth_abi_decoder.cc +++ b/components/brave_wallet/browser/eth_abi_decoder.cc @@ -3,22 +3,15 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/components/brave_wallet/browser/eth_abi_decoder.h" #include -#include #include #include #include #include "base/containers/span.h" -#include "base/strings/strcat.h" +#include "base/containers/span_reader.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "brave/components/brave_wallet/common/eth_abi_utils.h" @@ -96,8 +89,8 @@ std::optional> GetAddressFromData(ByteView input) { } return DecoderResult( - base::Value("0x" + HexEncodeLower(input.data() + kWordSize - kAddressSize, - kAddressSize)), + base::Value("0x" + + HexEncodeLower(input.first(kWordSize).last(kAddressSize))), GetSubByteView(input, kWordSize), kWordSize); } @@ -472,37 +465,35 @@ std::optional> UniswapEncodedPathDecode( if (!PrefixedHexStringToBytes(encoded_path, &data)) { return std::nullopt; } - size_t offset = 0; std::vector path; + auto reader = base::SpanReader(base::as_byte_span(data)); + // The path should be long enough to encode a single-hop swap. // 43 = 20(address) + 3(fee) + 20(address) - if (data.size() < 43) { + if (reader.remaining() < 43) { return std::nullopt; } // Parse first hop address. - path.push_back("0x" + HexEncodeLower(data.data(), 20)); - offset += 20; + path.push_back("0x" + HexEncodeLower(*reader.Read(20u))); while (true) { - if (offset == data.size()) { + if (!reader.remaining()) { break; } // Parse the pool fee, and ignore. - if (data.size() - offset < 3) { + if (!reader.Skip(3u)) { return std::nullopt; } - offset += 3; - // Parse next hop. - if (data.size() - offset < 20) { + if (auto address = reader.Read(20u)) { + path.push_back("0x" + HexEncodeLower(*address)); + } else { return std::nullopt; } - path.push_back("0x" + HexEncodeLower(data.data() + offset, 20)); - offset += 20; } // Require a minimum of 2 addresses for a single-hop swap. @@ -515,9 +506,7 @@ std::optional> UniswapEncodedPathDecode( std::optional ABIDecode(const eth_abi::Type& type, const ByteArray& data) { - ByteView input = base::make_span(data.data(), data.size()); - - auto decoded = DecodeParam(type, input); + auto decoded = DecodeParam(type, data); if (!decoded) { return std::nullopt; } diff --git a/components/brave_wallet/browser/eth_gas_utils.cc b/components/brave_wallet/browser/eth_gas_utils.cc index ad9e7fdbfdc8..e0c68229bd18 100644 --- a/components/brave_wallet/browser/eth_gas_utils.cc +++ b/components/brave_wallet/browser/eth_gas_utils.cc @@ -3,12 +3,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/components/brave_wallet/browser/eth_gas_utils.h" #include @@ -62,9 +56,9 @@ std::optional ScaleBaseFeePerGas(const std::string& value) { // base_fee_per_gas (last element) * 33% // - avg_priority_fee will be the 0.4 * length's element of the sorted // reward array. -// - The same applies to low_prirority_fee, but if it was equal to avg +// - The same applies to low_priority_fee, but if it was equal to avg // then we walk it back to the next smallest element if possible. -// - The same applies to high_prirority_fee, but if it was equal to avg +// - The same applies to high_priority_fee, but if it was equal to avg // then we walk it forward to the next biggest element if possible. bool GetSuggested1559Fees(const std::vector& base_fee_per_gas, const std::vector& gas_used_ratio, @@ -104,40 +98,37 @@ bool GetSuggested1559Fees(const std::vector& base_fee_per_gas, return true; } - std::vector priority_fee_uints[3]; - uint256_t* priority_fees[3] = {low_priority_fee, avg_priority_fee, - high_priority_fee}; + std::array, 3> priority_fee_uints; + std::array priority_fees = {low_priority_fee, avg_priority_fee, + high_priority_fee}; for (size_t i = 0; i < 3; i++) { - uint256_t& current_priority_fee = *(priority_fees[i]); std::vector& current_priority_fee_uints = priority_fee_uints[i]; bool invalid_data = false; // Convert the string priority fees to uints - std::transform(reward.begin(), reward.end(), - std::back_inserter(current_priority_fee_uints), - [&](const std::vector& v) -> uint256_t { - uint256_t val = fallback_priority_fee; - if (v.size() != 3) { - invalid_data = true; - } else if (!HexValueToUint256(v[i], &val)) { - invalid_data = true; - } - return val; - }); + for (auto& v : reward) { + uint256_t val = fallback_priority_fee; + if (v.size() != 3) { + invalid_data = true; + } else if (!HexValueToUint256(v[i], &val)) { + invalid_data = true; + } + current_priority_fee_uints.push_back(val); + } // We allow no reward info but we don't allow invalid reward info if (invalid_data) { return false; } - // Sort the priroirty fee uints + // Sort the priority fee uints std::sort(current_priority_fee_uints.begin(), current_priority_fee_uints.end()); - // Calculate the avg priorty fee first to be the 40th percentile of the avg + // Calculate the avg priority fee first to be the 40th percentile of the avg // percentiles. We use this same method as the initial value for low // and high too. size_t percentile_index = current_priority_fee_uints.size() * 0.4; - current_priority_fee = current_priority_fee_uints[percentile_index]; + *(priority_fees[i]) = current_priority_fee_uints[percentile_index]; } // Re-adjust the percentiles for low down to the next non-equal value if diff --git a/components/brave_wallet/browser/eth_transaction.cc b/components/brave_wallet/browser/eth_transaction.cc index 46ad2953f9f4..1d266b4ecb3f 100644 --- a/components/brave_wallet/browser/eth_transaction.cc +++ b/components/brave_wallet/browser/eth_transaction.cc @@ -188,9 +188,7 @@ std::vector EthTransaction::GetMessageToSign( list.Append(RLPUint256ToBlob(0)); } - std::vector result; - base::Extend(result, RLPEncode(list)); - return result; + return RLPEncode(list); } KeccakHashArray EthTransaction::GetHashedMessageToSign( @@ -229,7 +227,7 @@ bool EthTransaction::ProcessVRS(const std::vector& v, } // signature and recid will be used to produce v, r, s -void EthTransaction::ProcessSignature(const std::vector signature, +void EthTransaction::ProcessSignature(base::span signature, int recid, uint256_t chain_id) { if (signature.size() != 64) { diff --git a/components/brave_wallet/browser/eth_transaction.h b/components/brave_wallet/browser/eth_transaction.h index 57961add0158..7bd231de4410 100644 --- a/components/brave_wallet/browser/eth_transaction.h +++ b/components/brave_wallet/browser/eth_transaction.h @@ -69,8 +69,7 @@ class EthTransaction { const std::vector& s); bool IsToCreationAddress() const { return to_.IsEmpty(); } - // return - // rlp([nonce, gasPrice, gasLimit, to, value, data, chainID, 0, 0]) + // return rlp([nonce, gasPrice, gasLimit, to, value, data, chainID, 0, 0]) // Support EIP-155 chain id virtual std::vector GetMessageToSign(uint256_t chain_id) const; @@ -85,7 +84,7 @@ class EthTransaction { // signature and recid will be used to produce v, r, s // Support EIP-155 chain id - virtual void ProcessSignature(const std::vector signature, + virtual void ProcessSignature(base::span signature, int recid, uint256_t chain_id); diff --git a/components/brave_wallet/browser/eth_transaction_unittest.cc b/components/brave_wallet/browser/eth_transaction_unittest.cc index aa0413b81c04..d30900818258 100644 --- a/components/brave_wallet/browser/eth_transaction_unittest.cc +++ b/components/brave_wallet/browser/eth_transaction_unittest.cc @@ -114,10 +114,10 @@ TEST(EthTransactionUnitTest, GetMessageToSign) { } TEST(EthTransactionUnitTest, GetSignedTransactionAndHash) { - std::vector private_key; - EXPECT_TRUE(base::HexStringToBytes( + std::array private_key; + EXPECT_TRUE(base::HexStringToSpan( "4646464646464646464646464646464646464646464646464646464646464646", - &private_key)); + private_key)); HDKey key; key.SetPrivateKey(private_key); @@ -131,7 +131,7 @@ TEST(EthTransactionUnitTest, GetSignedTransactionAndHash) { "daf5a779ae972f972197303d7b574746c7ef83eadac0f2791ad23db92e4c8e53"); int recid; - const std::vector signature = key.SignCompact(message, &recid); + auto signature = *key.SignCompact(message, &recid); // invalid tx.ProcessSignature(std::vector(63), recid, 1); @@ -170,8 +170,7 @@ TEST(EthTransactionUnitTest, GetSignedTransactionAndHash) { EXPECT_EQ(base::ToLowerASCII(base::HexEncode(message1337)), "9df81edc908cd622cbbab86525a4588fdcbaf6c88757f39b42b1f8f58fd617c2"); recid = 0; - const std::vector signature1337 = - key.SignCompact(message1337, &recid); + auto signature1337 = *key.SignCompact(message1337, &recid); tx.ProcessSignature(signature1337, recid, 1337); EXPECT_EQ(tx.GetSignedTransaction(), "0xf86e098504a817c8008252089435353535353535353535353535353535353535" diff --git a/components/brave_wallet/browser/eth_tx_manager.cc b/components/brave_wallet/browser/eth_tx_manager.cc index b166c42dd8f1..c4ac9149c0f6 100644 --- a/components/brave_wallet/browser/eth_tx_manager.cc +++ b/components/brave_wallet/browser/eth_tx_manager.cc @@ -453,8 +453,8 @@ void EthTxManager::GetEthTransactionMessageToSign( std::move(callback).Run(std::nullopt); return; } - std::move(callback).Run(base::ToLowerASCII( - base::HexEncode(meta->tx()->GetMessageToSign(chain_id)))); + std::move(callback).Run( + HexEncodeLower(meta->tx()->GetMessageToSign(chain_id))); } mojom::CoinType EthTxManager::GetCoinType() const { diff --git a/components/brave_wallet/browser/ethereum_keyring.cc b/components/brave_wallet/browser/ethereum_keyring.cc index c4c004c0e91a..0aa0980676cb 100644 --- a/components/brave_wallet/browser/ethereum_keyring.cc +++ b/components/brave_wallet/browser/ethereum_keyring.cc @@ -38,14 +38,8 @@ EthereumKeyring::EthereumKeyring(base::span seed) // static std::optional EthereumKeyring::RecoverAddress( base::span message, - base::span signature) { - // A compact ECDSA signature (recovery id byte + 64 bytes). - if (signature.size() != kCompactSignatureSize + 1) { - return std::nullopt; - } - - std::vector signature_only(signature.begin(), signature.end()); - uint8_t v = signature_only.back(); + base::span signature) { + uint8_t v = signature.back(); if (v < 27) { VLOG(1) << "v should be >= 27"; return std::nullopt; @@ -54,7 +48,6 @@ std::optional EthereumKeyring::RecoverAddress( // v = chain_id ? recid + chain_id * 2 + 35 : recid + 27; // So recid = v - 27 when chain_id is 0 uint8_t recid = v - 27; - signature_only.pop_back(); auto hash = GetMessageHash(message); // Public keys (in scripts) are given as 04 where x and y are 32 @@ -62,8 +55,8 @@ std::optional EthereumKeyring::RecoverAddress( // curve or in compressed form given as where is 0x02 if // y is even and 0x03 if y is odd. HDKey key; - std::vector public_key = - key.RecoverCompact(false, hash, signature_only, recid); + std::vector public_key = key.RecoverCompact( + false, hash, signature.first(), recid); if (public_key.size() != 65) { VLOG(1) << "public key should be 65 bytes"; return std::nullopt; @@ -90,7 +83,7 @@ std::vector EthereumKeyring::SignMessage( return std::vector(); } - std::array hashed_message; + std::array hashed_message; if (!is_eip712) { hashed_message = GetMessageHash(message); } else { @@ -103,12 +96,17 @@ std::vector EthereumKeyring::SignMessage( } int recid; - std::vector signature = hd_key->SignCompact(hashed_message, &recid); + auto signature = hd_key->SignCompact(hashed_message, &recid); + if (!signature) { + return std::vector(); + } + + std::vector eth_signature(signature->begin(), signature->end()); uint8_t v = static_cast(chain_id ? recid + chain_id * 2 + 35 : recid + 27); - signature.push_back(v); + eth_signature.push_back(v); - return signature; + return eth_signature; } void EthereumKeyring::SignTransaction(const std::string& address, @@ -120,9 +118,12 @@ void EthereumKeyring::SignTransaction(const std::string& address, } int recid = 0; - const std::vector signature = + auto signature = hd_key->SignCompact(tx->GetHashedMessageToSign(chain_id), &recid); - tx->ProcessSignature(signature, recid, chain_id); + if (!signature) { + return; + } + tx->ProcessSignature(*signature, recid, chain_id); } std::string EthereumKeyring::GetAddressInternal(const HDKey& hd_key) const { diff --git a/components/brave_wallet/browser/ethereum_keyring.h b/components/brave_wallet/browser/ethereum_keyring.h index a094a44b1a4b..569627de5b58 100644 --- a/components/brave_wallet/browser/ethereum_keyring.h +++ b/components/brave_wallet/browser/ethereum_keyring.h @@ -32,7 +32,7 @@ class EthereumKeyring : public Secp256k1HDKeyring { // signature: The 64 byte signature + v parameter (0 chain id assumed) static std::optional RecoverAddress( base::span message, - base::span signature); + base::span signature); std::vector SignMessage(const std::string& address, base::span message, diff --git a/components/brave_wallet/browser/ethereum_keyring_unittest.cc b/components/brave_wallet/browser/ethereum_keyring_unittest.cc index 808dee64c810..c2b04b0416ca 100644 --- a/components/brave_wallet/browser/ethereum_keyring_unittest.cc +++ b/components/brave_wallet/browser/ethereum_keyring_unittest.cc @@ -3,12 +3,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/components/brave_wallet/browser/ethereum_keyring.h" #include @@ -115,10 +109,10 @@ TEST(EthereumKeyringUnitTest, SignTransaction) { } TEST(EthereumKeyringUnitTest, SignMessage) { - std::vector private_key; - EXPECT_TRUE(base::HexStringToBytes( + std::array private_key; + EXPECT_TRUE(base::HexStringToSpan( "6969696969696969696969696969696969696969696969696969696969696969", - &private_key)); + private_key)); std::unique_ptr key = std::make_unique(); key->SetPrivateKey(private_key); @@ -189,10 +183,10 @@ TEST(EthereumKeyringUnitTest, ImportedAccounts) { &seed)); EthereumKeyring keyring(seed); size_t private_keys_size = sizeof(private_keys) / sizeof(private_keys[0]); - for (size_t i = 0; i < private_keys_size; ++i) { + for (auto& test_case : private_keys) { std::vector private_key; - EXPECT_TRUE(base::HexStringToBytes(private_keys[i].key, &private_key)); - EXPECT_EQ(keyring.ImportAccount(private_key), private_keys[i].address); + EXPECT_TRUE(base::HexStringToBytes(test_case.key, &private_key)); + EXPECT_EQ(keyring.ImportAccount(private_key), test_case.address); } EXPECT_EQ(keyring.GetImportedAccountsForTesting().size(), private_keys_size); // Trying to add a duplicate account diff --git a/components/brave_wallet/browser/ethereum_provider_impl.cc b/components/brave_wallet/browser/ethereum_provider_impl.cc index c151be9d3076..7c6a9961aa65 100644 --- a/components/brave_wallet/browser/ethereum_provider_impl.cc +++ b/components/brave_wallet/browser/ethereum_provider_impl.cc @@ -461,10 +461,6 @@ void EthereumProviderImpl::RecoverAddress(const std::string& message, RequestCallback callback, base::Value id) { bool reject = false; - // 65 * 2 hex chars per byte + 2 chars for 0x - if (signature.length() != 132) { - return RejectInvalidParams(std::move(id), std::move(callback)); - } auto message_bytes = PrefixedHexStringToBytes(message); if (!message_bytes) { @@ -476,8 +472,15 @@ void EthereumProviderImpl::RecoverAddress(const std::string& message, return RejectInvalidParams(std::move(id), std::move(callback)); } + auto signature_bytes_span = + base::make_span(*signature_bytes) + .to_fixed_extent<65>(); /*kRecoverableSignatureSize*/ + if (!signature_bytes_span) { + return RejectInvalidParams(std::move(id), std::move(callback)); + } + auto address = keyring_service_->RecoverAddressByDefaultKeyring( - *message_bytes, *signature_bytes); + *message_bytes, *signature_bytes_span); if (!address) { base::Value formed_response = GetProviderErrorDictionary( mojom::ProviderError::kInternalError, @@ -686,7 +689,7 @@ void EthereumProviderImpl::SignTypedMessage( SignMessageInternal( account_id, std::move(sign_data), - std::vector(message_to_sign->begin(), message_to_sign->end()), + std::vector(message_to_sign.begin(), message_to_sign.end()), std::move(callback), std::move(id)); } diff --git a/components/brave_wallet/browser/internal/hd_key.cc b/components/brave_wallet/browser/internal/hd_key.cc index df400c8651b8..e2d0cb95ca2f 100644 --- a/components/brave_wallet/browser/internal/hd_key.cc +++ b/components/brave_wallet/browser/internal/hd_key.cc @@ -3,19 +3,15 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/components/brave_wallet/browser/internal/hd_key.h" +#include #include #include #include "base/check.h" #include "base/containers/span.h" +#include "base/containers/span_reader.h" #include "base/json/json_reader.h" #include "base/logging.h" #include "base/numerics/byte_conversions.h" @@ -25,6 +21,7 @@ #include "base/strings/string_util.h" #include "brave/components/brave_wallet/common/bitcoin_utils.h" #include "brave/components/brave_wallet/common/hash_utils.h" +#include "brave/components/brave_wallet/common/mem_utils.h" #include "brave/components/brave_wallet/common/zcash_utils.h" #include "brave/third_party/bitcoin-core/src/src/base58.h" #include "brave/vendor/bat-native-tweetnacl/tweetnacl.h" @@ -68,17 +65,35 @@ const secp256k1_context* GetSecp256k1Ctx() { return kSecp256k1Ctx; } +std::vector GeneratePublicKey(Secp256k1PrivateKeySpan private_key) { + secp256k1_pubkey public_key; + if (!secp256k1_ec_pubkey_create(GetSecp256k1Ctx(), &public_key, + private_key.data())) { + LOG(ERROR) << "secp256k1_ec_pubkey_create failed"; + return {}; + } + + size_t public_key_len = kSecp256k1PubkeySize; + std::vector public_key_bytes(kSecp256k1PubkeySize); + if (!secp256k1_ec_pubkey_serialize(GetSecp256k1Ctx(), public_key_bytes.data(), + &public_key_len, &public_key, + SECP256K1_EC_COMPRESSED)) { + LOG(ERROR) << "secp256k1_ec_pubkey_serialize failed"; + } + + return public_key_bytes; +} + bool UTCPasswordVerification(const std::string& derived_key, base::span ciphertext, - const std::string& mac, + base::span mac, size_t dklen) { std::vector mac_verification_input(derived_key.end() - dklen / 2, derived_key.end()); mac_verification_input.insert(mac_verification_input.end(), ciphertext.begin(), ciphertext.end()); // verify password - if (base::ToLowerASCII( - base::HexEncode(KeccakHash(mac_verification_input))) != mac) { + if (KeccakHash(mac_verification_input) != mac) { VLOG(0) << __func__ << ": password does not match"; return false; } @@ -120,7 +135,7 @@ bool UTCDecryptPrivateKey(const std::string& derived_key, } // namespace -HDKey::HDKey() : identifier_(20), public_key_(33), chain_code_(32) {} +HDKey::HDKey() = default; HDKey::~HDKey() = default; HDKey::ParsedExtendedKey::ParsedExtendedKey() = default; @@ -134,7 +149,7 @@ std::unique_ptr HDKey::GenerateFromSeed(base::span seed) { return nullptr; } - SecureVector hmac(kSHA512Length); + SecureByteArray hmac; unsigned int out_len; if (!HMAC(EVP_sha512(), kMasterSecret, sizeof(kMasterSecret), seed.data(), seed.size(), hmac.data(), &out_len)) { @@ -144,9 +159,7 @@ std::unique_ptr HDKey::GenerateFromSeed(base::span seed) { DCHECK(out_len == kSHA512Length); std::unique_ptr hdkey = std::make_unique(); - auto hmac_span = base::make_span(hmac); - auto IL = hmac_span.first(kSHA512Length / 2); - auto IR = hmac_span.last(kSHA512Length / 2); + auto [IL, IR] = base::make_span(hmac).split_at(); hdkey->SetPrivateKey(IL); hdkey->SetChainCode(IR); hdkey->path_ = kMasterNode; @@ -164,51 +177,56 @@ std::unique_ptr HDKey::GenerateFromExtendedKey( SecureVector buf(decoded_key.begin(), decoded_key.end()); SecureZeroData(base::as_writable_byte_span(decoded_key)); + + auto reader = base::SpanReader(base::as_byte_span(buf)); + // version(4) || depth(1) || parent_fingerprint(4) || index(4) || chain(32) || // key(33) - const uint8_t* ptr = buf.data(); - auto version = static_cast(ptr[0] << 24 | ptr[1] << 16 | - ptr[2] << 8 | ptr[3] << 0); - ptr += sizeof(version); - uint8_t depth = *ptr; - ptr += sizeof(depth); + auto result = std::make_unique(); + result->hdkey = std::make_unique(); - int32_t parent_fingerprint = - ptr[0] << 24 | ptr[1] << 16 | ptr[2] << 8 | ptr[3] << 0; - ptr += sizeof(parent_fingerprint); + uint32_t version = 0; + if (!reader.ReadU32BigEndian(version)) { + return nullptr; + } + result->version = static_cast(version); - int32_t index = ptr[0] << 24 | ptr[1] << 16 | ptr[2] << 8 | ptr[3] << 0; - ptr += sizeof(index); + if (!reader.ReadU8BigEndian(result->hdkey->depth_)) { + return nullptr; + } - std::unique_ptr hdkey = std::make_unique(); - hdkey->depth_ = depth; - hdkey->parent_fingerprint_ = parent_fingerprint; - hdkey->index_ = index; + if (!reader.ReadCopy(result->hdkey->parent_fingerprint_)) { + return nullptr; + } - std::vector chain_code(ptr, ptr + 32); - ptr += chain_code.size(); - hdkey->SetChainCode(chain_code); + if (!reader.ReadU32BigEndian(result->hdkey->index_)) { + return nullptr; + } - if (*ptr == 0x00) { + auto chain_code = reader.Read(); + if (!chain_code) { + return nullptr; + } + result->hdkey->SetChainCode(*chain_code); + + auto key_bytes = reader.Read(); + if (!key_bytes) { + return nullptr; + } + if (key_bytes->front() == 0x00) { // Skip first zero byte which is not part of private key. - hdkey->SetPrivateKey(base::make_span(ptr + 1, ptr + 33)); + result->hdkey->SetPrivateKey(key_bytes->subspan<1>()); } else { - hdkey->SetPublicKey(*base::span(ptr, ptr + kSecp256k1PubkeySize) - .to_fixed_extent()); + result->hdkey->SetPublicKey(*key_bytes); } - auto result = std::make_unique(); - result->hdkey = std::move(hdkey); - result->version = version; + return result; } // static std::unique_ptr HDKey::GenerateFromPrivateKey( - base::span private_key) { - if (private_key.size() != 32) { - return nullptr; - } + Secp256k1PrivateKeySpan private_key) { std::unique_ptr hd_key = std::make_unique(); hd_key->SetPrivateKey(private_key); return hd_key; @@ -332,11 +350,17 @@ std::unique_ptr HDKey::GenerateFromV3UTC(const std::string& password, return nullptr; } - const auto* mac = crypto->FindString("mac"); - if (!mac) { + const auto* mac_hex = crypto->FindString("mac"); + if (!mac_hex) { VLOG(0) << __func__ << ": missing mac"; return nullptr; } + std::array mac; + if (!base::HexStringToSpan(*mac_hex, mac)) { + VLOG(0) << __func__ << ": invalid mac"; + return nullptr; + } + const auto* ciphertext = crypto->FindString("ciphertext"); if (!ciphertext) { VLOG(0) << __func__ << ": missing ciphertext"; @@ -349,7 +373,7 @@ std::unique_ptr HDKey::GenerateFromV3UTC(const std::string& password, } auto derived_key = std::make_unique(key); - if (!UTCPasswordVerification(derived_key->key(), ciphertext_bytes, *mac, + if (!UTCPasswordVerification(derived_key->key(), ciphertext_bytes, mac, *dklen)) { return nullptr; } @@ -382,32 +406,37 @@ std::unique_ptr HDKey::GenerateFromV3UTC(const std::string& password, return nullptr; } - return GenerateFromPrivateKey(private_key); + auto private_key_span = base::as_byte_span(private_key) + .to_fixed_extent(); + if (!private_key_span) { + return nullptr; + } + + return GenerateFromPrivateKey(*private_key_span); } std::string HDKey::GetPath() const { return path_; } -void HDKey::SetPrivateKey(base::span value) { - if (value.size() != 32) { - LOG(ERROR) << __func__ << ": pivate key must be 32 bytes"; - return; - } - private_key_.assign(value.begin(), value.end()); - GeneratePublicKey(); - identifier_ = Hash160(public_key_); - - const uint8_t* ptr = identifier_.data(); - fingerprint_ = ptr[0] << 24 | ptr[1] << 16 | ptr[2] << 8 | ptr[3] << 0; +void HDKey::SetPrivateKey(Secp256k1PrivateKeySpan value) { + private_key_.emplace(); + base::span(*private_key_).copy_from(value); + public_key_ = GeneratePublicKey(*private_key_); } std::string HDKey::GetPrivateExtendedKey(ExtendedKeyVersion version) const { - return Serialize(version, private_key_); + if (!private_key_) { + return {}; + } + return Serialize(version, *private_key_); } std::vector HDKey::GetPrivateKeyBytes() const { - return std::vector(private_key_.begin(), private_key_.end()); + if (!private_key_) { + return {}; + } + return std::vector(private_key_->begin(), private_key_->end()); } std::vector HDKey::GetPublicKeyBytes() const { @@ -415,8 +444,7 @@ std::vector HDKey::GetPublicKeyBytes() const { return public_key_; } -void HDKey::SetPublicKey( - base::span value) { +void HDKey::SetPublicKey(Secp256k1PubkeyKeySpan value) { // Verify public key secp256k1_pubkey pubkey; if (!secp256k1_ec_pubkey_parse(GetSecp256k1Ctx(), &pubkey, value.data(), @@ -425,10 +453,6 @@ void HDKey::SetPublicKey( return; } public_key_.assign(value.begin(), value.end()); - identifier_ = Hash160(public_key_); - - const uint8_t* ptr = identifier_.data(); - fingerprint_ = ptr[0] << 24 | ptr[1] << 16 | ptr[2] << 8 | ptr[3] << 0; } std::string HDKey::GetPublicExtendedKey(ExtendedKeyVersion version) const { @@ -461,9 +485,8 @@ std::string HDKey::GetZCashTransparentAddress(bool testnet) const { std::vector HDKey::GetPublicKeyFromX25519_XSalsa20_Poly1305() const { size_t public_key_len = crypto_scalarmult_curve25519_tweet_BYTES; std::vector public_key(public_key_len); - const uint8_t* private_key_ptr = private_key_.data(); - if (crypto_scalarmult_curve25519_tweet_base(public_key.data(), - private_key_ptr) != 0) { + if (!private_key_ || crypto_scalarmult_curve25519_tweet_base( + public_key.data(), private_key_->data()) != 0) { return std::vector(); } return public_key; @@ -486,19 +509,19 @@ HDKey::DecryptCipherFromX25519_XSalsa20_Poly1305( crypto_box_curve25519xsalsa20poly1305_tweet_PUBLICKEYBYTES) { return std::nullopt; } - if (private_key_.size() != - crypto_box_curve25519xsalsa20poly1305_tweet_SECRETKEYBYTES) { + if (!private_key_) { return std::nullopt; } + static_assert(kSecp256k1PrivateKeySize == + crypto_box_curve25519xsalsa20poly1305_tweet_SECRETKEYBYTES); std::vector padded_ciphertext(ciphertext.begin(), ciphertext.end()); padded_ciphertext.insert(padded_ciphertext.begin(), crypto_box_BOXZEROBYTES, 0); std::vector padded_plaintext(padded_ciphertext.size()); - const uint8_t* private_key_ptr = private_key_.data(); if (crypto_box_open(padded_plaintext.data(), padded_ciphertext.data(), padded_ciphertext.size(), nonce.data(), - ephemeral_public_key.data(), private_key_ptr) != 0) { + ephemeral_public_key.data(), private_key_->data()) != 0) { return std::nullopt; } std::vector plaintext( @@ -507,8 +530,8 @@ HDKey::DecryptCipherFromX25519_XSalsa20_Poly1305( return plaintext; } -void HDKey::SetChainCode(base::span value) { - chain_code_.assign(value.begin(), value.end()); +void HDKey::SetChainCode(Bip32ChainCodeSpan value) { + base::span(chain_code_).copy_from(value); } std::unique_ptr HDKey::DeriveNormalChild(uint32_t index) { @@ -533,9 +556,9 @@ std::unique_ptr HDKey::DeriveChild(uint32_t index) { if (is_hardened) { // Hardened: data = 0x00 || ser256(kpar) || ser32(index) - DCHECK(!private_key_.empty()); + CHECK(private_key_); data.push_back(0x00); - data.insert(data.end(), private_key_.begin(), private_key_.end()); + data.insert(data.end(), private_key_->begin(), private_key_->end()); } else { // Normal private: data = serP(point(kpar)) || ser32(index) // Normal pubic : data = serP(Kpar) || ser32(index) @@ -547,7 +570,7 @@ std::unique_ptr HDKey::DeriveChild(uint32_t index) { data.push_back((index >> 8) & 0xFF); data.push_back(index & 0xFF); - SecureVector hmac(kSHA512Length); + SecureByteArray hmac; unsigned int out_len; if (!HMAC(EVP_sha512(), chain_code_.data(), chain_code_.size(), data.data(), data.size(), hmac.data(), &out_len)) { @@ -556,18 +579,16 @@ std::unique_ptr HDKey::DeriveChild(uint32_t index) { } DCHECK(out_len == kSHA512Length); - auto hmac_span = base::make_span(hmac); - auto IL = hmac_span.first(kSHA512Length / 2); - auto IR = hmac_span.last(kSHA512Length / 2); + auto [IL, IR] = base::make_span(hmac).split_at(); std::unique_ptr hdkey = std::make_unique(); hdkey->SetChainCode(IR); - if (!private_key_.empty()) { + if (private_key_) { // Private parent key -> private child key // Also Private parent key -> public child key because we always create // public key. - SecureVector private_key = private_key_; + SecureByteArray private_key = *private_key_; if (!secp256k1_ec_seckey_tweak_add(GetSecp256k1Ctx(), private_key.data(), IL.data())) { LOG(ERROR) << __func__ << ": secp256k1_ec_seckey_tweak_add failed"; @@ -607,7 +628,7 @@ std::unique_ptr HDKey::DeriveChild(uint32_t index) { hdkey->path_ = base::StrCat({path_, "/", node}); } hdkey->depth_ = depth_ + 1; - hdkey->parent_fingerprint_ = fingerprint_; + hdkey->parent_fingerprint_ = GetFingerprint(); hdkey->index_ = index; return hdkey; @@ -618,7 +639,7 @@ std::unique_ptr HDKey::DeriveChildFromPath(const std::string& path) { LOG(ERROR) << __func__ << ": must derive only from master key"; return nullptr; } - if (private_key_.empty()) { + if (!private_key_) { LOG(ERROR) << __func__ << ": master key must have private key"; return nullptr; } @@ -636,7 +657,8 @@ std::unique_ptr HDKey::DeriveChildFromPath(const std::string& path) { LOG(ERROR) << __func__ << ": path must start with \"m\""; return nullptr; } - hd_key->SetPrivateKey(private_key_); + + hd_key->SetPrivateKey(*private_key_); hd_key->SetChainCode(chain_code_); hd_key->path_ = path_; @@ -671,17 +693,19 @@ std::unique_ptr HDKey::DeriveChildFromPath(const std::string& path) { return hd_key; } -std::vector HDKey::SignCompact( - base::span msg, +std::optional> HDKey::SignCompact( + Secp256k1SignMsgSpan msg, int* recid) { - std::vector sig(kCompactSignatureSize); + CHECK(private_key_); + + std::array sig = {}; if (!recid) { secp256k1_ecdsa_signature ecdsa_sig; if (!secp256k1_ecdsa_sign(GetSecp256k1Ctx(), &ecdsa_sig, msg.data(), - private_key_.data(), + private_key_->data(), secp256k1_nonce_function_rfc6979, nullptr)) { LOG(ERROR) << __func__ << ": secp256k1_ecdsa_sign failed"; - return sig; + return std::nullopt; } if (!secp256k1_ecdsa_signature_serialize_compact(GetSecp256k1Ctx(), @@ -692,10 +716,10 @@ std::vector HDKey::SignCompact( } else { secp256k1_ecdsa_recoverable_signature ecdsa_sig; if (!secp256k1_ecdsa_sign_recoverable( - GetSecp256k1Ctx(), &ecdsa_sig, msg.data(), private_key_.data(), + GetSecp256k1Ctx(), &ecdsa_sig, msg.data(), private_key_->data(), secp256k1_nonce_function_rfc6979, nullptr)) { LOG(ERROR) << __func__ << ": secp256k1_ecdsa_sign_recoverable failed"; - return sig; + return std::nullopt; } if (!secp256k1_ecdsa_recoverable_signature_serialize_compact( GetSecp256k1Ctx(), sig.data(), recid, &ecdsa_sig)) { @@ -708,12 +732,13 @@ std::vector HDKey::SignCompact( return sig; } -std::optional> HDKey::SignDer( - base::span msg) { +std::optional> HDKey::SignDer(Secp256k1SignMsgSpan msg) { + CHECK(private_key_); + unsigned char extra_entropy[32] = {0}; secp256k1_ecdsa_signature ecdsa_sig; if (!secp256k1_ecdsa_sign(GetSecp256k1Ctx(), &ecdsa_sig, msg.data(), - private_key_.data(), + private_key_->data(), secp256k1_nonce_function_rfc6979, nullptr)) { LOG(ERROR) << __func__ << ": secp256k1_ecdsa_sign failed"; return std::nullopt; @@ -736,7 +761,7 @@ std::optional> HDKey::SignDer( base::byte_span_from_ref(++extra_entropy_counter)))); if (!secp256k1_ecdsa_sign( - GetSecp256k1Ctx(), &ecdsa_sig, msg.data(), private_key_.data(), + GetSecp256k1Ctx(), &ecdsa_sig, msg.data(), private_key_->data(), secp256k1_nonce_function_rfc6979, extra_entropy)) { LOG(ERROR) << __func__ << ": secp256k1_ecdsa_sign failed"; return std::nullopt; @@ -756,13 +781,8 @@ std::optional> HDKey::SignDer( return sig_der; } -bool HDKey::VerifyForTesting(base::span msg, - base::span sig) { - if (msg.size() != 32 || sig.size() != kCompactSignatureSize) { - LOG(ERROR) << __func__ << ": message or signature length is invalid"; - return false; - } - +bool HDKey::VerifyForTesting(Secp256k1SignMsgSpan msg, + CompactSignatureSpan sig) { secp256k1_ecdsa_signature ecdsa_sig; if (!secp256k1_ecdsa_signature_parse_compact(GetSecp256k1Ctx(), &ecdsa_sig, sig.data())) { @@ -784,17 +804,13 @@ bool HDKey::VerifyForTesting(base::span msg, return true; } -std::vector HDKey::RecoverCompact( - bool compressed, - base::span msg, - base::span sig, - int recid) { +std::vector HDKey::RecoverCompact(bool compressed, + Secp256k1SignMsgSpan msg, + CompactSignatureSpan sig, + int recid) { size_t public_key_len = compressed ? 33 : 65; std::vector public_key(public_key_len); - if (sig.size() != kCompactSignatureSize) { - LOG(ERROR) << __func__ << ": message or signature length is invalid"; - return public_key; - } + if (recid < 0 || recid > 3) { LOG(ERROR) << __func__ << ": recovery id must be 0, 1, 2 or 3"; return public_key; @@ -825,19 +841,17 @@ std::vector HDKey::RecoverCompact( return public_key; } -void HDKey::GeneratePublicKey() { - secp256k1_pubkey public_key; - if (!secp256k1_ec_pubkey_create(GetSecp256k1Ctx(), &public_key, - private_key_.data())) { - LOG(ERROR) << "secp256k1_ec_pubkey_create failed"; - return; - } - size_t public_key_len = 33; - if (!secp256k1_ec_pubkey_serialize(GetSecp256k1Ctx(), public_key_.data(), - &public_key_len, &public_key, - SECP256K1_EC_COMPRESSED)) { - LOG(ERROR) << "secp256k1_ec_pubkey_serialize failed"; - } +// https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#key-identifiers +std::array HDKey::GetIdentifier() { + return Hash160(public_key_); +} + +// https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#key-identifiers +std::array HDKey::GetFingerprint() { + std::array fingerprint; + CHECK(base::SpanReader(base::as_byte_span(GetIdentifier())) + .ReadCopy(fingerprint)); + return fingerprint; } // https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#serialization-format @@ -858,10 +872,7 @@ std::string HDKey::Serialize(ExtendedKeyVersion version, buf.push_back(depth_); - buf.push_back((parent_fingerprint_ >> 24) & 0xFF); - buf.push_back((parent_fingerprint_ >> 16) & 0xFF); - buf.push_back((parent_fingerprint_ >> 8) & 0xFF); - buf.push_back(parent_fingerprint_ & 0xFF); + buf.insert(buf.end(), parent_fingerprint_.begin(), parent_fingerprint_.end()); buf.push_back((index_ >> 24) & 0xFF); buf.push_back((index_ >> 16) & 0xFF); diff --git a/components/brave_wallet/browser/internal/hd_key.h b/components/brave_wallet/browser/internal/hd_key.h index 7f2d78f275dd..e12123716ee1 100644 --- a/components/brave_wallet/browser/internal/hd_key.h +++ b/components/brave_wallet/browser/internal/hd_key.h @@ -13,18 +13,28 @@ #include "base/containers/span.h" #include "base/gtest_prod_util.h" -#include "brave/components/brave_wallet/common/hash_utils.h" #include "brave/components/brave_wallet/common/mem_utils.h" namespace brave_wallet { inline constexpr size_t kCompactSignatureSize = 64; +inline constexpr size_t kRecoverableSignatureSize = 65; inline constexpr size_t kSecp256k1PubkeySize = 33; -inline constexpr size_t kSecp256k1MsgSize = 32; - +inline constexpr size_t kSecp256k1PrivateKeySize = 32; +inline constexpr size_t kSecp256k1SignMsgSize = 32; +inline constexpr size_t kBip32ChainCodeSize = 32; +inline constexpr size_t kBip32IdentifierSize = 20; +inline constexpr size_t kBip32FingerprintSize = 4; + +using Secp256k1PubkeyKeySpan = base::span; +using Secp256k1PrivateKeySpan = + base::span; using SecureVector = std::vector>; +using Secp256k1SignMsgSpan = base::span; +using Bip32ChainCodeSpan = base::span; +using CompactSignatureSpan = base::span; -enum class ExtendedKeyVersion { +enum class ExtendedKeyVersion : uint32_t { // https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#serialization-format kXprv = 0x0488ade4, kXpub = 0x0488b21e, @@ -61,7 +71,7 @@ class HDKey { static std::unique_ptr GenerateFromExtendedKey( const std::string& key); static std::unique_ptr GenerateFromPrivateKey( - base::span private_key); + Secp256k1PrivateKeySpan private_key); // https://github.com/ethereum/wiki/wiki/Web3-Secret-Storage-Definition static std::unique_ptr GenerateFromV3UTC(const std::string& password, const std::string& json); @@ -101,28 +111,25 @@ class HDKey { // Sign the message using private key. The msg has to be exactly 32 bytes // Return 64 bytes ECDSA signature when succeed, otherwise empty vector // if recid is not null, recovery id will be filled. - std::vector SignCompact( - base::span msg, + std::optional> SignCompact( + Secp256k1SignMsgSpan msg, int* recid); // Sign the message using private key and return it in DER format. - std::optional> SignDer( - base::span msg); + std::optional> SignDer(Secp256k1SignMsgSpan msg); // Verify the ECDSA signature using public key. The msg has to be exactly 32 // bytes and the sig has to be 64 bytes. // Return true when successfully verified, false otherwise. - bool VerifyForTesting(base::span msg, - base::span sig); + bool VerifyForTesting(Secp256k1SignMsgSpan msg, CompactSignatureSpan sig); // Recover public key from signature and message. The msg has to be exactly 32 // bytes and the sig has to be 64 bytes. // Return valid public key when succeed, all zero vector otherwise - std::vector RecoverCompact( - bool compressed, - base::span msg, - base::span sig, - int recid); + std::vector RecoverCompact(bool compressed, + Secp256k1SignMsgSpan msg, + CompactSignatureSpan sig, + int recid); private: FRIEND_TEST_ALL_PREFIXES(Eip1559TransactionUnitTest, @@ -136,30 +143,28 @@ class HDKey { FRIEND_TEST_ALL_PREFIXES(HDKeyUnitTest, SetPublicKey); FRIEND_TEST_ALL_PREFIXES(HDKeyUnitTest, SignAndVerifyAndRecover); - // value must be 32 bytes - void SetPrivateKey(base::span value); - void SetChainCode(base::span value); - // value must be 33 bytes valid public key (compressed) - void SetPublicKey(base::span value); + void SetPrivateKey(Secp256k1PrivateKeySpan value); + void SetPublicKey(Secp256k1PubkeyKeySpan value); + void SetChainCode(Bip32ChainCodeSpan value); // index should be 0 to 2^32 // 0 to 2^31-1 is normal derivation and 2^31 to 2^32-1 is harden derivation // If anything failed, nullptr will be returned std::unique_ptr DeriveChild(uint32_t index); - void GeneratePublicKey(); + std::array GetIdentifier(); + std::array GetFingerprint(); + std::string Serialize(ExtendedKeyVersion version, base::span key) const; std::string path_; uint8_t depth_ = 0; - uint32_t fingerprint_ = 0; - uint32_t parent_fingerprint_ = 0; + std::array parent_fingerprint_ = {}; uint32_t index_ = 0; - std::vector identifier_; - SecureVector private_key_; - std::vector public_key_; - SecureVector chain_code_; + std::optional> private_key_; + std::vector public_key_{33}; + SecureByteArray chain_code_ = {}; HDKey(const HDKey&) = delete; HDKey& operator=(const HDKey&) = delete; diff --git a/components/brave_wallet/browser/internal/hd_key_unittest.cc b/components/brave_wallet/browser/internal/hd_key_unittest.cc index 951780344362..5bb103865460 100644 --- a/components/brave_wallet/browser/internal/hd_key_unittest.cc +++ b/components/brave_wallet/browser/internal/hd_key_unittest.cc @@ -41,7 +41,10 @@ std::string GetHexAddr(const HDKey* key) { return addr.ToHex(); } -std::string GetWifPrivateKey(std::vector private_key, bool testnet) { +std::string GetWifPrivateKey(base::span private_key_bytes, + bool testnet) { + std::vector private_key(private_key_bytes.begin(), + private_key_bytes.end()); private_key.insert(private_key.begin(), testnet ? 0xef : 0x80); // Version Byte. auto sha256hash = DoubleSHA256Hash(private_key); @@ -284,7 +287,7 @@ TEST(HDKeyUnitTest, GenerateFromExtendedKey) { EXPECT_EQ(parsed_xprv->version, ExtendedKeyVersion::kXprv); auto* hdkey_from_pri = parsed_xprv->hdkey.get(); EXPECT_EQ(hdkey_from_pri->depth_, 5u); - EXPECT_EQ(hdkey_from_pri->parent_fingerprint_, 0x31a507b8u); + EXPECT_EQ(HexEncodeLower(hdkey_from_pri->parent_fingerprint_), "31a507b8"); EXPECT_EQ(hdkey_from_pri->index_, 2u); EXPECT_EQ(base::ToLowerASCII(base::HexEncode(hdkey_from_pri->chain_code_)), "9452b549be8cea3ecb7a84bec10dcfd94afe4d129ebfd3b3cb58eedf394ed271"); @@ -294,8 +297,9 @@ TEST(HDKeyUnitTest, GenerateFromExtendedKey) { EXPECT_EQ( base::ToLowerASCII(base::HexEncode(hdkey_from_pri->public_key_)), "024d902e1a2fc7a8755ab5b694c575fce742c48d9ff192e63df5193e4c7afe1f9c"); - EXPECT_EQ(base::ToLowerASCII(base::HexEncode(hdkey_from_pri->identifier_)), + EXPECT_EQ(HexEncodeLower(hdkey_from_pri->GetIdentifier()), "26132fdbe7bf89cbc64cf8dafa3f9f88b8666220"); + EXPECT_EQ(HexEncodeLower(hdkey_from_pri->GetFingerprint()), "26132fdb"); EXPECT_EQ(hdkey_from_pri->GetPath(), ""); // m/0/2147483647'/1/2147483646'/2 @@ -305,7 +309,7 @@ TEST(HDKeyUnitTest, GenerateFromExtendedKey) { EXPECT_EQ(parsed_xpub->version, ExtendedKeyVersion::kXpub); auto* hdkey_from_pub = parsed_xpub->hdkey.get(); EXPECT_EQ(hdkey_from_pub->depth_, 5u); - EXPECT_EQ(hdkey_from_pub->parent_fingerprint_, 0x31a507b8u); + EXPECT_EQ(HexEncodeLower(hdkey_from_pub->parent_fingerprint_), "31a507b8"); EXPECT_EQ(hdkey_from_pub->index_, 2u); EXPECT_EQ(base::ToLowerASCII(base::HexEncode(hdkey_from_pub->chain_code_)), "9452b549be8cea3ecb7a84bec10dcfd94afe4d129ebfd3b3cb58eedf394ed271"); @@ -313,15 +317,16 @@ TEST(HDKeyUnitTest, GenerateFromExtendedKey) { EXPECT_EQ( base::ToLowerASCII(base::HexEncode(hdkey_from_pub->public_key_)), "024d902e1a2fc7a8755ab5b694c575fce742c48d9ff192e63df5193e4c7afe1f9c"); - EXPECT_EQ(base::ToLowerASCII(base::HexEncode(hdkey_from_pub->identifier_)), + EXPECT_EQ(HexEncodeLower(hdkey_from_pub->GetIdentifier()), "26132fdbe7bf89cbc64cf8dafa3f9f88b8666220"); + EXPECT_EQ(HexEncodeLower(hdkey_from_pub->GetFingerprint()), "26132fdb"); EXPECT_EQ(hdkey_from_pub->GetPath(), ""); auto parsed_zprv = HDKey::GenerateFromExtendedKey(kBtcMainnetImportAccount0); EXPECT_EQ(parsed_zprv->version, ExtendedKeyVersion::kZprv); auto* hdkey_from_zprv = parsed_zprv->hdkey.get(); EXPECT_EQ(hdkey_from_zprv->depth_, 3u); - EXPECT_EQ(hdkey_from_zprv->parent_fingerprint_, 0x7ef32bdbu); + EXPECT_EQ(HexEncodeLower(hdkey_from_zprv->parent_fingerprint_), "7ef32bdb"); EXPECT_EQ(hdkey_from_zprv->index_, 2147483648u); EXPECT_EQ(base::ToLowerASCII(base::HexEncode(hdkey_from_zprv->chain_code_)), "4a53a0ab21b9dc95869c4e92a161194e03c0ef3ff5014ac692f433c4765490fc"); @@ -331,15 +336,16 @@ TEST(HDKeyUnitTest, GenerateFromExtendedKey) { EXPECT_EQ( base::ToLowerASCII(base::HexEncode(hdkey_from_zprv->public_key_)), "02707a62fdacc26ea9b63b1c197906f56ee0180d0bcf1966e1a2da34f5f3a09a9b"); - EXPECT_EQ(base::ToLowerASCII(base::HexEncode(hdkey_from_zprv->identifier_)), + EXPECT_EQ(HexEncodeLower(hdkey_from_zprv->GetIdentifier()), "fd13aac9a294188cdfe1331a8d94880bccbef8c1"); + EXPECT_EQ(HexEncodeLower(hdkey_from_zprv->GetFingerprint()), "fd13aac9"); EXPECT_EQ(hdkey_from_zprv->GetPath(), ""); auto parsed_vprv = HDKey::GenerateFromExtendedKey(kBtcTestnetImportAccount0); EXPECT_EQ(parsed_vprv->version, ExtendedKeyVersion::kVprv); auto* hdkey_from_vprv = parsed_vprv->hdkey.get(); EXPECT_EQ(hdkey_from_vprv->depth_, 3u); - EXPECT_EQ(hdkey_from_vprv->parent_fingerprint_, 0x0ef4b1afu); + EXPECT_EQ(HexEncodeLower(hdkey_from_vprv->parent_fingerprint_), "0ef4b1af"); EXPECT_EQ(hdkey_from_vprv->index_, 2147483648u); EXPECT_EQ(base::ToLowerASCII(base::HexEncode(hdkey_from_vprv->chain_code_)), "3c8c2037ee4c1621da0d348db51163709a622d0d2838dde6d8419c51f6301c62"); @@ -349,16 +355,17 @@ TEST(HDKeyUnitTest, GenerateFromExtendedKey) { EXPECT_EQ( base::ToLowerASCII(base::HexEncode(hdkey_from_vprv->public_key_)), "03b88e0fbe3f646337ed93bc0c0f3b843fcf7d2589e5ec884754e6402027a890b4"); - EXPECT_EQ(base::ToLowerASCII(base::HexEncode(hdkey_from_vprv->identifier_)), + EXPECT_EQ(HexEncodeLower(hdkey_from_vprv->GetIdentifier()), "e99b862826a40a32c24c79785d06b19de3fb076f"); + EXPECT_EQ(HexEncodeLower(hdkey_from_vprv->GetFingerprint()), "e99b8628"); EXPECT_EQ(hdkey_from_vprv->GetPath(), ""); } TEST(HDKeyUnitTest, GenerateFromPrivateKey) { - std::vector private_key; - ASSERT_TRUE(base::HexStringToBytes( + std::array private_key; + ASSERT_TRUE(base::HexStringToSpan( "bb7d39bdb83ecf58f2fd82b6d918341cbef428661ef01ab97c28a4842125ac23", - &private_key)); + private_key)); std::unique_ptr key = HDKey::GenerateFromPrivateKey(private_key); EXPECT_NE(key, nullptr); EXPECT_EQ(key->GetPath(), ""); @@ -367,8 +374,8 @@ TEST(HDKeyUnitTest, GenerateFromPrivateKey) { msg_b.fill(0x08); int recid_a = -1; int recid_b = -1; - const std::vector sig_a = key->SignCompact(msg_a, &recid_a); - const std::vector sig_b = key->SignCompact(msg_b, &recid_b); + auto sig_a = *key->SignCompact(msg_a, &recid_a); + auto sig_b = *key->SignCompact(msg_b, &recid_b); EXPECT_NE(recid_a, -1); EXPECT_NE(recid_b, -1); EXPECT_EQ(base::ToLowerASCII(base::HexEncode(sig_a)), @@ -379,9 +386,6 @@ TEST(HDKeyUnitTest, GenerateFromPrivateKey) { "532e5c0ae2a25392d97f5e55ab1288ef1e08d5c034bad3b0956fbbab73b381"); EXPECT_TRUE(key->VerifyForTesting(msg_a, sig_a)); EXPECT_TRUE(key->VerifyForTesting(msg_b, sig_b)); - - EXPECT_EQ(HDKey::GenerateFromPrivateKey(std::vector(33)), nullptr); - EXPECT_EQ(HDKey::GenerateFromPrivateKey(std::vector(31)), nullptr); } TEST(HDKeyUnitTest, SignAndVerifyAndRecover) { @@ -395,8 +399,8 @@ TEST(HDKeyUnitTest, SignAndVerifyAndRecover) { msg_b.fill(0x08); int recid_a = -1; int recid_b = -1; - const std::vector sig_a = key->SignCompact(msg_a, &recid_a); - const std::vector sig_b = key->SignCompact(msg_b, &recid_b); + auto sig_a = *key->SignCompact(msg_a, &recid_a); + auto sig_b = *key->SignCompact(msg_b, &recid_b); EXPECT_NE(recid_a, -1); EXPECT_NE(recid_b, -1); EXPECT_EQ(base::ToLowerASCII(base::HexEncode(sig_a)), @@ -422,21 +426,11 @@ TEST(HDKeyUnitTest, SignAndVerifyAndRecover) { EXPECT_EQ(base::HexEncode(uncompressed_public_key_b), base::HexEncode(key->GetUncompressedPublicKey())); - EXPECT_FALSE(key->VerifyForTesting(std::vector(32), - std::vector(64))); + EXPECT_FALSE(key->VerifyForTesting(std::array{}, + std::array{})); EXPECT_FALSE(key->VerifyForTesting(msg_a, sig_b)); EXPECT_FALSE(key->VerifyForTesting(msg_b, sig_a)); - EXPECT_FALSE(key->VerifyForTesting(std::vector(31), sig_a)); - EXPECT_FALSE(key->VerifyForTesting(std::vector(33), sig_a)); - - EXPECT_FALSE(key->VerifyForTesting(msg_a, std::vector(63))); - EXPECT_FALSE(key->VerifyForTesting(msg_a, std::vector(65))); - - EXPECT_TRUE(IsPublicKeyEmpty( - key->RecoverCompact(true, msg_a, std::vector(31), recid_a))); - EXPECT_TRUE(IsPublicKeyEmpty( - key->RecoverCompact(true, msg_a, std::vector(33), recid_a))); EXPECT_TRUE(IsPublicKeyEmpty(key->RecoverCompact(true, msg_a, sig_a, -1))); EXPECT_TRUE(IsPublicKeyEmpty(key->RecoverCompact(true, msg_a, sig_a, 4))); EXPECT_TRUE(IsPublicKeyEmpty(key->RecoverCompact(false, msg_a, sig_a, -1))); @@ -445,11 +439,7 @@ TEST(HDKeyUnitTest, SignAndVerifyAndRecover) { TEST(HDKeyUnitTest, SetPrivateKey) { HDKey key; - key.SetPrivateKey(std::vector(31)); - ASSERT_TRUE(key.GetPrivateKeyBytes().empty()); - key.SetPrivateKey(std::vector(33)); - ASSERT_TRUE(key.GetPrivateKeyBytes().empty()); - key.SetPrivateKey(std::vector(32, 0x1)); + key.SetPrivateKey(std::array({1, 2, 3})); EXPECT_FALSE(key.GetPrivateKeyBytes().empty()); EXPECT_TRUE(!IsPublicKeyEmpty(key.public_key_)); } @@ -634,7 +624,7 @@ TEST(HDKeyUnitTest, GenerateFromV3UTC) { // https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#examples TEST(HDKeyUnitTest, GetSegwitAddress) { - std::vector private_key_bytes(32, 0); + std::array private_key_bytes = {}; private_key_bytes.back() = 1; std::unique_ptr hdkey = HDKey::GenerateFromPrivateKey(private_key_bytes); @@ -650,10 +640,10 @@ TEST(HDKeyUnitTest, GetSegwitAddress) { // TODO(apaymyshev): Consider more tests. Also test R grinding. TEST(HDKeyUnitTest, SignDer) { - std::vector private_key_bytes; - ASSERT_TRUE(base::HexStringToBytes( + std::array private_key_bytes; + ASSERT_TRUE(base::HexStringToSpan( "12b004fff7f4b69ef8650e767f18f11ede158148b425660723b9f9a66e61f747", - &private_key_bytes)); + private_key_bytes)); // https://github.com/bitcoin/bitcoin/blob/v24.0/src/test/key_tests.cpp#L20 ASSERT_EQ(GetWifPrivateKey(private_key_bytes, false), "5HxWvvfubhXpYYpS3tJkw6fq9jE9j18THftkZjHHfmFiWtmAbrj"); diff --git a/components/brave_wallet/browser/keyring_service.cc b/components/brave_wallet/browser/keyring_service.cc index 13b143ca4688..d2cf2226f4a8 100644 --- a/components/brave_wallet/browser/keyring_service.cc +++ b/components/brave_wallet/browser/keyring_service.cc @@ -1636,7 +1636,7 @@ KeyringService::SignatureWithError KeyringService::SignMessageByDefaultKeyring( std::optional KeyringService::RecoverAddressByDefaultKeyring( base::span message, - base::span signature) { + base::span signature) { return EthereumKeyring::RecoverAddress(message, signature); } diff --git a/components/brave_wallet/browser/keyring_service.h b/components/brave_wallet/browser/keyring_service.h index e2bccb83ee8e..98e26166c36d 100644 --- a/components/brave_wallet/browser/keyring_service.h +++ b/components/brave_wallet/browser/keyring_service.h @@ -158,7 +158,7 @@ class KeyringService : public mojom::KeyringService { base::span message); std::optional RecoverAddressByDefaultKeyring( base::span message, - base::span signature); + base::span signature); bool GetPublicKeyFromX25519_XSalsa20_Poly1305ByDefaultKeyring( const mojom::AccountIdPtr& account_id, std::string* key); diff --git a/components/brave_wallet/browser/meld_integration_response_parser_unittest.cc b/components/brave_wallet/browser/meld_integration_response_parser_unittest.cc index f32683eaf292..fb2162f29701 100644 --- a/components/brave_wallet/browser/meld_integration_response_parser_unittest.cc +++ b/components/brave_wallet/browser/meld_integration_response_parser_unittest.cc @@ -3,12 +3,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/components/brave_wallet/browser/meld_integration_response_parser.h" #include @@ -50,33 +44,30 @@ TEST(MeldIntegrationResponseParserUnitTest, Parse_ServiceProvider) { auto service_providers = ParseServiceProviders(ParseJson(json)); EXPECT_TRUE(service_providers); - EXPECT_EQ(base::ranges::count_if( - *service_providers, - [](const auto& item) { - return item->name == "Banxa" && - item->service_provider == "BANXA" && - item->status == "LIVE" && - item->web_site_url == "http://www.banxa.com" && - !item->categories.empty() && - item->categories[0] == "CRYPTO_ONRAMP" && - !item->category_statuses.empty() && - item->category_statuses.contains("CRYPTO_ONRAMP") && - item->category_statuses["CRYPTO_ONRAMP"] == "LIVE" && - item->logo_images && - item->logo_images->dark_url == - "https://images-serviceprovider.meld.io/BANXA/" - "logo_dark.png" && - item->logo_images->dark_short_url == - "https://images-serviceprovider.meld.io/BANXA/" - "short_logo_dark.png" && - item->logo_images->light_url == - "https://images-serviceprovider.meld.io/BANXA/" - "logo_light.png" && - item->logo_images->light_short_url == - "https://images-serviceprovider.meld.io/BANXA/" - "short_logo_light.png"; - }), - 1); + EXPECT_EQ( + base::ranges::count_if( + *service_providers, + [](const auto& item) { + return item->name == "Banxa" && item->service_provider == "BANXA" && + item->status == "LIVE" && + item->web_site_url == "http://www.banxa.com" && + item->categories[0] == "CRYPTO_ONRAMP" && + item->category_statuses.at("CRYPTO_ONRAMP") == "LIVE" && + item->logo_images && + item->logo_images->dark_url == + "https://images-serviceprovider.meld.io/BANXA/" + "logo_dark.png" && + item->logo_images->dark_short_url == + "https://images-serviceprovider.meld.io/BANXA/" + "short_logo_dark.png" && + item->logo_images->light_url == + "https://images-serviceprovider.meld.io/BANXA/" + "logo_light.png" && + item->logo_images->light_short_url == + "https://images-serviceprovider.meld.io/BANXA/" + "short_logo_light.png"; + }), + 1); std::string json_null_logos(R"([ { "serviceProvider": "BANXA", @@ -90,20 +81,18 @@ TEST(MeldIntegrationResponseParserUnitTest, Parse_ServiceProvider) { }])"); service_providers = ParseServiceProviders(ParseJson(json_null_logos)); EXPECT_TRUE(service_providers); - EXPECT_EQ(base::ranges::count_if( - *service_providers, - [](const auto& item) { - return item->name == "Banxa" && - item->service_provider == "BANXA" && - item->status == "LIVE" && - item->web_site_url == "http://www.banxa.com" && - item->categories.empty() && - !item->category_statuses.empty() && - item->category_statuses.contains("CRYPTO_ONRAMP") && - item->category_statuses["CRYPTO_ONRAMP"] == "LIVE" && - !item->logo_images; - }), - 1); + EXPECT_EQ( + base::ranges::count_if( + *service_providers, + [](const auto& item) { + return item->name == "Banxa" && item->service_provider == "BANXA" && + item->status == "LIVE" && + item->web_site_url == "http://www.banxa.com" && + item->categories.empty() && + item->category_statuses.at("CRYPTO_ONRAMP") == "LIVE" && + !item->logo_images; + }), + 1); EXPECT_FALSE(ParseServiceProviders(ParseJson(R"([ { diff --git a/components/brave_wallet/browser/permission_utils_unittest.cc b/components/brave_wallet/browser/permission_utils_unittest.cc index 482aee69d4a3..12b260ac971c 100644 --- a/components/brave_wallet/browser/permission_utils_unittest.cc +++ b/components/brave_wallet/browser/permission_utils_unittest.cc @@ -3,12 +3,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/components/brave_wallet/browser/permission_utils.h" #include @@ -51,30 +45,26 @@ TEST(PermissionUtilsUnitTest, GetConcatOriginFromWalletAddresses) { }}; url::Origin origin = url::Origin::Create(GURL("https://test.com")); - for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); ++i) { + for (auto& test_case : cases) { + SCOPED_TRACE(testing::Message() << test_case.expected_out_origin); + url::Origin out_origin; - EXPECT_TRUE( - GetConcatOriginFromWalletAddresses(origin, cases[i].addrs, &out_origin)) - << "case: " << i; + EXPECT_TRUE(GetConcatOriginFromWalletAddresses(origin, test_case.addrs, + &out_origin)); EXPECT_EQ(out_origin, - url::Origin::Create(GURL(cases[i].expected_out_origin))) - << "case: " << i; + url::Origin::Create(GURL(test_case.expected_out_origin))); EXPECT_FALSE(GetConcatOriginFromWalletAddresses( - url::Origin(), cases[i].addrs, &out_origin)) - << "case: " << i; + url::Origin(), test_case.addrs, &out_origin)); EXPECT_FALSE(GetConcatOriginFromWalletAddresses( - origin, std::vector(), &out_origin)) - << "case: " << i; + origin, std::vector(), &out_origin)); // Origin with port case: EXPECT_TRUE(GetConcatOriginFromWalletAddresses( - url::Origin::Create(GURL("https://test.com:123")), cases[i].addrs, - &out_origin)) - << "case: " << i; - EXPECT_EQ(out_origin, - url::Origin::Create(GURL(cases[i].expected_out_origin_with_port))) - << "case: " << i; + url::Origin::Create(GURL("https://test.com:123")), test_case.addrs, + &out_origin)); + EXPECT_EQ(out_origin, url::Origin::Create( + GURL(test_case.expected_out_origin_with_port))); } } @@ -101,41 +91,35 @@ TEST(PermissionUtilsUnitTest, ParseRequestingOriginFromSubRequest) { url::Origin requesting_origin; std::string account; - for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); ++i) { + for (auto& test_case : cases) { + SCOPED_TRACE(testing::Message() << test_case.account); + // Invalid requesting_origin format: EXPECT_FALSE(ParseRequestingOriginFromSubRequest( - cases[i].type, url::Origin::Create(GURL(cases[i].invalid_origin)), - nullptr, nullptr)) - << "case: " << i; + test_case.type, url::Origin::Create(GURL(test_case.invalid_origin)), + nullptr, nullptr)); EXPECT_FALSE(ParseRequestingOriginFromSubRequest( - cases[i].type, - url::Origin::Create(GURL(cases[i].invalid_origin_with_path)), nullptr, - nullptr)) - << "case: " << i; + test_case.type, + url::Origin::Create(GURL(test_case.invalid_origin_with_path)), nullptr, + nullptr)); EXPECT_FALSE(ParseRequestingOriginFromSubRequest( - cases[i].type, url::Origin(), nullptr, nullptr)) - << "case: " << i; + test_case.type, url::Origin(), nullptr, nullptr)); // invalid type EXPECT_FALSE(ParseRequestingOriginFromSubRequest( permissions::RequestType::kGeolocation, - url::Origin::Create(GURL(cases[i].valid_origin)), nullptr, nullptr)) - << "case: " << i; + url::Origin::Create(GURL(test_case.valid_origin)), nullptr, nullptr)); EXPECT_TRUE(ParseRequestingOriginFromSubRequest( - cases[i].type, url::Origin::Create(GURL(cases[i].valid_origin)), - &requesting_origin, &account)) - << "case: " << i; + test_case.type, url::Origin::Create(GURL(test_case.valid_origin)), + &requesting_origin, &account)); EXPECT_EQ(requesting_origin.GetURL(), GURL("https://test.com")); - EXPECT_TRUE(base::EqualsCaseInsensitiveASCII(account, cases[i].account)) - << "case: " << i; + EXPECT_TRUE(base::EqualsCaseInsensitiveASCII(account, test_case.account)); EXPECT_TRUE(ParseRequestingOriginFromSubRequest( - cases[i].type, - url::Origin::Create(GURL(cases[i].valid_origin_with_port)), - &requesting_origin, &account)) - << "case: " << i; + test_case.type, + url::Origin::Create(GURL(test_case.valid_origin_with_port)), + &requesting_origin, &account)); EXPECT_EQ(requesting_origin.GetURL(), GURL("https://test.com:123")); - EXPECT_TRUE(base::EqualsCaseInsensitiveASCII(account, cases[i].account)) - << "case: " << i; + EXPECT_TRUE(base::EqualsCaseInsensitiveASCII(account, test_case.account)); } // separator in domain would still work @@ -198,79 +182,69 @@ TEST(PermissionUtilsUnitTest, ParseRequestingOrigin) { "JDqrvDz8d8tFCADashbUKQDKfJZFobNy13ugN65t1wvV"}}; std::string account; - for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); ++i) { + for (auto& test_case : cases) { + SCOPED_TRACE(testing::Message() << test_case.account1); + // Invalid requesting_origin format: EXPECT_FALSE(ParseRequestingOrigin( - cases[i].type, url::Origin::Create(GURL(cases[i].invalid_origin)), - nullptr, nullptr)) - << "case: " << i; + test_case.type, url::Origin::Create(GURL(test_case.invalid_origin)), + nullptr, nullptr)); EXPECT_FALSE(ParseRequestingOrigin( - cases[i].type, - url::Origin::Create(GURL(cases[i].invalid_origin_with_path)), nullptr, - nullptr)) - << "case: " << i; + test_case.type, + url::Origin::Create(GURL(test_case.invalid_origin_with_path)), nullptr, + nullptr)); EXPECT_FALSE( - ParseRequestingOrigin(cases[i].type, url::Origin(), nullptr, nullptr)) - << "case: " << i; + ParseRequestingOrigin(test_case.type, url::Origin(), nullptr, nullptr)); // invalid type EXPECT_FALSE(ParseRequestingOrigin( permissions::RequestType::kGeolocation, - url::Origin::Create(GURL(cases[i].valid_origin)), nullptr, nullptr)) - << "case: " << i; + url::Origin::Create(GURL(test_case.valid_origin)), nullptr, nullptr)); std::queue address_queue; // Origin without port: url::Origin requesting_origin; EXPECT_TRUE(ParseRequestingOrigin( - cases[i].type, url::Origin::Create(GURL(cases[i].valid_origin)), + test_case.type, url::Origin::Create(GURL(test_case.valid_origin)), &requesting_origin, &address_queue)); - EXPECT_EQ(requesting_origin.GetURL(), GURL("https://test.com")) - << "case: " << i; - EXPECT_EQ(address_queue.size(), 1u) << "case: " << i; + EXPECT_EQ(requesting_origin.GetURL(), GURL("https://test.com")); + EXPECT_EQ(address_queue.size(), 1u); EXPECT_TRUE(base::EqualsCaseInsensitiveASCII(address_queue.front(), - cases[i].account1)) - << "case: " << i; + test_case.account1)); EXPECT_TRUE(ParseRequestingOrigin( - cases[i].type, - url::Origin::Create(GURL(cases[i].valid_origin_two_accounts)), - &requesting_origin, nullptr)) - << "case: " << i; - EXPECT_EQ(requesting_origin.GetURL(), GURL("https://test.com")) - << "case: " << i; + test_case.type, + url::Origin::Create(GURL(test_case.valid_origin_two_accounts)), + &requesting_origin, nullptr)); + EXPECT_EQ(requesting_origin.GetURL(), GURL("https://test.com")); // Origin with port: EXPECT_TRUE(ParseRequestingOrigin( - cases[i].type, - url::Origin::Create(GURL(cases[i].valid_origin_with_port)), - &requesting_origin, nullptr)) - << "case: " << i; - EXPECT_EQ(requesting_origin.GetURL(), GURL("https://test.com:123")) - << "case: " << i; + test_case.type, + url::Origin::Create(GURL(test_case.valid_origin_with_port)), + &requesting_origin, nullptr)); + EXPECT_EQ(requesting_origin.GetURL(), GURL("https://test.com:123")); EXPECT_FALSE(ParseRequestingOrigin( - cases[i].type, - url::Origin::Create(GURL(cases[i].valid_origin_two_accounts_with_port)), + test_case.type, + url::Origin::Create( + GURL(test_case.valid_origin_two_accounts_with_port)), &requesting_origin, &address_queue)) - << "Non-empty address_queue param should return false. case: " << i; + << "Non-empty address_queue param should return false."; address_queue = std::queue(); EXPECT_TRUE(ParseRequestingOrigin( - cases[i].type, - url::Origin::Create(GURL(cases[i].valid_origin_two_accounts_with_port)), - &requesting_origin, &address_queue)) - << "case: " << i; - EXPECT_EQ(requesting_origin.GetURL(), GURL("https://test.com:123")) - << "case: " << i; - EXPECT_EQ(address_queue.size(), 2u) << "case: " << i; + test_case.type, + url::Origin::Create( + GURL(test_case.valid_origin_two_accounts_with_port)), + &requesting_origin, &address_queue)); + EXPECT_EQ(requesting_origin.GetURL(), GURL("https://test.com:123")); + EXPECT_EQ(address_queue.size(), 2u); EXPECT_TRUE(base::EqualsCaseInsensitiveASCII(address_queue.front(), - cases[i].account1)) - << "case: " << i; + test_case.account1)); address_queue.pop(); EXPECT_TRUE(base::EqualsCaseInsensitiveASCII(address_queue.front(), - cases[i].account2)) - << "case: " << i; + test_case.account2)); } } @@ -295,32 +269,28 @@ TEST(PermissionUtilsUnitTest, GetSubRequestOrigin) { "https://test.com__BrG44HdsEhzapvs8bEqzvkq4egwevS3fRE6ze2ENo6S8", "https://" "test.com__BrG44HdsEhzapvs8bEqzvkq4egwevS3fRE6ze2ENo6S8:123"}}; - for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); ++i) { - EXPECT_FALSE(GetSubRequestOrigin(cases[i].type, url::Origin(), - cases[i].account, &new_origin)) - << "case: " << i; + for (auto& test_case : cases) { + SCOPED_TRACE(testing::Message() << test_case.account); + + EXPECT_FALSE(GetSubRequestOrigin(test_case.type, url::Origin(), + test_case.account, &new_origin)); // invalid type EXPECT_FALSE(GetSubRequestOrigin(permissions::RequestType::kGeolocation, - old_origin, cases[i].account, &new_origin)) - << "case: " << i; + old_origin, test_case.account, + &new_origin)); EXPECT_FALSE( - GetSubRequestOrigin(cases[i].type, old_origin, "", &new_origin)) - << "case: " << i; - EXPECT_FALSE(GetSubRequestOrigin(cases[i].type, old_origin, - cases[i].account, nullptr)) - << "case: " << i; - - EXPECT_TRUE(GetSubRequestOrigin(cases[i].type, old_origin, cases[i].account, - &new_origin)) - << "case: " << i; - EXPECT_EQ(new_origin, - url::Origin::Create(GURL(cases[i].expected_new_origin))); - EXPECT_TRUE(GetSubRequestOrigin(cases[i].type, old_origin_with_port, - cases[i].account, &new_origin)) - << "case: " << i; + GetSubRequestOrigin(test_case.type, old_origin, "", &new_origin)); + EXPECT_FALSE(GetSubRequestOrigin(test_case.type, old_origin, + test_case.account, nullptr)); + + EXPECT_TRUE(GetSubRequestOrigin(test_case.type, old_origin, + test_case.account, &new_origin)); EXPECT_EQ(new_origin, - url::Origin::Create(GURL(cases[i].expected_new_origin_with_port))) - << "case: " << i; + url::Origin::Create(GURL(test_case.expected_new_origin))); + EXPECT_TRUE(GetSubRequestOrigin(test_case.type, old_origin_with_port, + test_case.account, &new_origin)); + EXPECT_EQ(new_origin, url::Origin::Create( + GURL(test_case.expected_new_origin_with_port))); } } @@ -343,9 +313,12 @@ TEST(PermissionUtilsUnitTest, GetConnectWithSiteWebUIURL) { "?addr=BrG44HdsEhzapvs8bEqzvkq4egwevS3fRE6ze2ENo6S8&addr=" "JDqrvDz8d8tFCADashbUKQDKfJZFobNy13ugN65t1wvV&origin-spec=https://" "a.test.com:123&etld-plus-one=test.com#connectWithSite"}}; - for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); ++i) { - GURL url_out = GetConnectWithSiteWebUIURL(base_url, cases[i].addrs, origin); - EXPECT_EQ(url_out.spec(), cases[i].expected_out_url) << "case: " << i; + for (auto& test_case : cases) { + SCOPED_TRACE(testing::Message() << test_case.addrs[0]); + + GURL url_out = + GetConnectWithSiteWebUIURL(base_url, test_case.addrs, origin); + EXPECT_EQ(url_out.spec(), test_case.expected_out_url); } } diff --git a/components/brave_wallet/browser/rlp_encode.h b/components/brave_wallet/browser/rlp_encode.h index 9a956695d488..3a0cc930d763 100644 --- a/components/brave_wallet/browser/rlp_encode.h +++ b/components/brave_wallet/browser/rlp_encode.h @@ -6,7 +6,7 @@ #ifndef BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_RLP_ENCODE_H_ #define BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_RLP_ENCODE_H_ -#include +#include #include "base/values.h" #include "brave/components/brave_wallet/common/brave_wallet_types.h" diff --git a/components/brave_wallet/browser/rlp_encode_unittest.cc b/components/brave_wallet/browser/rlp_encode_unittest.cc index aa35530606c2..7bd0cd4a8e81 100644 --- a/components/brave_wallet/browser/rlp_encode_unittest.cc +++ b/components/brave_wallet/browser/rlp_encode_unittest.cc @@ -3,12 +3,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/components/brave_wallet/browser/rlp_encode.h" #include @@ -16,74 +10,16 @@ #include #include +#include "base/strings/string_util.h" +#include "base/test/values_test_util.h" #include "brave/components/brave_wallet/common/hex_utils.h" #include "testing/gtest/include/gtest/gtest.h" namespace { -base::Value RLPTestStringToValue(const std::string& s, std::string* remaining) { - const char* start = s.c_str(); - const char* p = start; - enum State { Start, InString, InList }; - size_t list_depth = 0; - State state = Start; - while (*p != '\0') { - if (isdigit(*p) && state == Start) { - start = p; - while (isdigit(*(p + 1))) { - ++p; - } - size_t len = p - start + 1; - if (start - s.c_str() + len + 1 < s.length()) { - *remaining = s.substr(start - s.c_str() + len + 1, std::string::npos); - } else { - *remaining = ""; - } - return base::Value(std::stoi(s.substr(start - s.c_str(), len))); - } else if (*p == '\'' && state == Start) { - state = InString; - start = p + 1; - } else if (*p == '\'' && state == InString) { - size_t len = p - start; - if (start - s.c_str() + len + 1 < s.length()) { - *remaining = s.substr(start - s.c_str() + len + 1, std::string::npos); - } else { - *remaining = ""; - } - return base::Value(s.substr(start - s.c_str(), len)); - } else if (*p == '[' && state == Start) { - state = InList; - // + 1 to not include the [ - start = p + 1; - list_depth++; - } else if (*p == '[' && state == InList) { - list_depth++; - } else if (*p == ']' && list_depth != 1) { - list_depth--; - } else if (*p == ']' && state == InList && list_depth == 1) { - base::Value::List list; - size_t len = p - start; - std::string list_contents = s.substr(start - s.c_str(), len); - while (list_contents.length() > 0) { - base::Value v = RLPTestStringToValue(list_contents, remaining); - list_contents = *remaining; - list.Append(std::move(v)); - } - if (start - s.c_str() + len + 1 < s.length()) { - *remaining = s.substr(start - s.c_str() + len + 1, std::string::npos); - } else { - *remaining = ""; - } - return base::Value(std::move(list)); - } - ++p; - } - return base::Value(); -} - -base::Value RLPTestStringToValue(const std::string& s) { - std::string left_over; - return RLPTestStringToValue(s, &left_over); +base::Value RLPTestStringToValue(std::string s) { + base::ReplaceChars(s, "'", "\"", &s); + return base::test::ParseJson(s); } } // namespace diff --git a/components/brave_wallet/browser/secp256k1_hd_keyring.cc b/components/brave_wallet/browser/secp256k1_hd_keyring.cc index 2d79ad6aff59..ed3dc311aa51 100644 --- a/components/brave_wallet/browser/secp256k1_hd_keyring.cc +++ b/components/brave_wallet/browser/secp256k1_hd_keyring.cc @@ -78,7 +78,12 @@ void Secp256k1HDKeyring::RemoveLastHDAccount() { std::string Secp256k1HDKeyring::ImportAccount( base::span private_key) { - std::unique_ptr hd_key = HDKey::GenerateFromPrivateKey(private_key); + auto private_key_32 = private_key.to_fixed_extent(); + if (!private_key_32) { + return std::string(); + } + std::unique_ptr hd_key = + HDKey::GenerateFromPrivateKey(*private_key_32); if (!hd_key) { return std::string(); } diff --git a/components/brave_wallet/browser/solana_instruction_builder.cc b/components/brave_wallet/browser/solana_instruction_builder.cc index 72aad492afc1..5b3e8f7b119c 100644 --- a/components/brave_wallet/browser/solana_instruction_builder.cc +++ b/components/brave_wallet/browser/solana_instruction_builder.cc @@ -3,18 +3,14 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/components/brave_wallet/browser/solana_instruction_builder.h" #include #include #include +#include "base/containers/span.h" +#include "base/containers/span_writer.h" #include "brave/components/brave_wallet/browser/simple_hash_client.h" #include "brave/components/brave_wallet/browser/solana_account_meta.h" #include "brave/components/brave_wallet/browser/solana_instruction.h" @@ -28,23 +24,12 @@ namespace { // Solana uses bincode::serialize when encoding instruction data, which encodes // unsigned numbers in little endian byte order. template -void UintToLEBytes(T val, std::vector* bytes) { - static_assert( - std::is_same::value || std::is_same::value, - "Incorrect type passed to function UintToLEBytes."); - - DCHECK(bytes); - size_t vec_size = sizeof(T) / sizeof(uint8_t); - *bytes = std::vector(vec_size); - - uint8_t* ptr = reinterpret_cast(&val); - for (size_t i = 0; i < vec_size; i++) { -#if defined(ARCH_CPU_LITTLE_ENDIAN) - bytes->at(i) = *ptr++; -#else - bytes->at(vec_size - 1 - i) = *ptr++; -#endif - } + requires(std::same_as || std::same_as) +void UintToLEBytes(std::vector& bytes, T val) { + auto val_span = base::byte_span_from_ref(val); + bytes.resize(bytes.size() + val_span.size()); + + base::as_writable_byte_span(bytes).last(val_span.size()).copy_from(val_span); } } // namespace @@ -70,13 +55,9 @@ std::optional Transfer(const std::string& from_pubkey, // Instruction data is consisted of u32 instruction index and u64 lamport. std::vector instruction_data; UintToLEBytes( - static_cast(mojom::SolanaSystemInstruction::kTransfer), - &instruction_data); - - std::vector lamport_bytes; - UintToLEBytes(lamport, &lamport_bytes); - instruction_data.insert(instruction_data.end(), lamport_bytes.begin(), - lamport_bytes.end()); + instruction_data, + static_cast(mojom::SolanaSystemInstruction::kTransfer)); + UintToLEBytes(instruction_data, lamport); return SolanaInstruction( mojom::kSolanaSystemProgramId, @@ -124,11 +105,7 @@ std::optional TransferChecked( std::vector instruction_data = { static_cast(mojom::SolanaTokenInstruction::kTransferChecked)}; - std::vector amount_bytes; - UintToLEBytes(amount, &amount_bytes); - instruction_data.insert(instruction_data.end(), amount_bytes.begin(), - amount_bytes.end()); - + UintToLEBytes(instruction_data, amount); instruction_data.emplace_back(decimals); std::vector account_metas = { @@ -198,10 +175,8 @@ SolanaInstruction SetComputeUnitLimit(uint32_t units) { std::vector instruction_data = {static_cast( mojom::SolanaComputeBudgetInstruction::kSetComputeUnitLimit)}; - std::vector units_bytes; - UintToLEBytes(units, &units_bytes); - instruction_data.insert(instruction_data.end(), units_bytes.begin(), - units_bytes.end()); + UintToLEBytes(instruction_data, units); + return SolanaInstruction(mojom::kSolanaComputeBudgetProgramId, {}, instruction_data); } @@ -212,10 +187,7 @@ SolanaInstruction SetComputeUnitPrice(uint64_t price) { std::vector instruction_data = {static_cast( mojom::SolanaComputeBudgetInstruction::kSetComputeUnitPrice)}; - std::vector price_bytes; - UintToLEBytes(price, &price_bytes); - instruction_data.insert(instruction_data.end(), price_bytes.begin(), - price_bytes.end()); + UintToLEBytes(instruction_data, price); return SolanaInstruction(mojom::kSolanaComputeBudgetProgramId, {}, instruction_data); @@ -256,21 +228,13 @@ std::optional Transfer( instruction_data.insert(instruction_data.end(), creator_hash_bytes.begin(), creator_hash_bytes.end()); - std::vector tempVec; - // Nonce - tempVec.clear(); // Use leaf.index for nonce like the example // https://solana.com/developers/guides/javascript/compressed-nfts#build-the-transfer-instruction - UintToLEBytes(static_cast(proof.leaf_index), &tempVec); - instruction_data.insert(instruction_data.end(), tempVec.begin(), - tempVec.end()); + UintToLEBytes(instruction_data, static_cast(proof.leaf_index)); // Index - tempVec.clear(); - UintToLEBytes(proof.leaf_index, &tempVec); - instruction_data.insert(instruction_data.end(), tempVec.begin(), - tempVec.end()); + UintToLEBytes(instruction_data, proof.leaf_index); // Create account metas. std::vector account_metas({ @@ -292,8 +256,7 @@ std::optional Transfer( } size_t end = proof.proof.size() - proof.canopy_depth; for (size_t i = 0; i < end; ++i) { - account_metas.push_back( - SolanaAccountMeta(proof.proof[i], std::nullopt, false, false)); + account_metas.emplace_back(proof.proof[i], std::nullopt, false, false); } return SolanaInstruction(mojom::kSolanaBubbleGumProgramId, diff --git a/components/brave_wallet/browser/zcash/zcash_keyring.cc b/components/brave_wallet/browser/zcash/zcash_keyring.cc index 1c8ab747eaa8..573f94c37aa1 100644 --- a/components/brave_wallet/browser/zcash/zcash_keyring.cc +++ b/components/brave_wallet/browser/zcash/zcash_keyring.cc @@ -72,7 +72,8 @@ std::optional> ZCashKeyring::GetPubkeyHash( return std::nullopt; } - return Hash160(hd_key_base->GetPublicKeyBytes()); + auto hashed = Hash160(hd_key_base->GetPublicKeyBytes()); + return std::vector(hashed.begin(), hashed.end()); } #if BUILDFLAG(ENABLE_ORCHARD) diff --git a/components/brave_wallet/common/eth_abi_utils.cc b/components/brave_wallet/common/eth_abi_utils.cc index 31e72af5effe..976cafdf2591 100644 --- a/components/brave_wallet/common/eth_abi_utils.cc +++ b/components/brave_wallet/common/eth_abi_utils.cc @@ -120,7 +120,7 @@ std::optional ExtractFixedBytesRowFromTuple(Span data, return std::nullopt; } - if (!CheckPadding(head->subspan(0), fixed_size)) { + if (!CheckPadding(*head, fixed_size)) { return std::nullopt; } diff --git a/components/brave_wallet/common/eth_abi_utils_unittest.cc b/components/brave_wallet/common/eth_abi_utils_unittest.cc index e3b21aa764b7..64e8b7779ddb 100644 --- a/components/brave_wallet/common/eth_abi_utils_unittest.cc +++ b/components/brave_wallet/common/eth_abi_utils_unittest.cc @@ -3,12 +3,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/components/brave_wallet/common/eth_abi_utils.h" #include @@ -16,6 +10,7 @@ #include #include +#include "base/containers/span.h" #include "base/ranges/algorithm.h" #include "brave/components/brave_wallet/common/hex_utils.h" #include "testing/gmock/include/gmock/gmock.h" @@ -714,8 +709,7 @@ TEST(EthAbiUtilsTest, ExtractFixedBytesFromTuple) { TEST(EthAbiTupleEncoderTest, EncodeCall) { std::vector data(33, 0xbb); - auto selector_bytes = ToBytes("f400d2f8"); - Span4 selector(selector_bytes.begin(), 4u); + Span4 selector({0xf4, 0x00, 0xd2, 0xf8}); // f(bytes,bytes) EXPECT_EQ( "f400d2f8" @@ -746,7 +740,7 @@ TEST(EthAbiTupleEncoderTest, EncodeCall) { "f400d2f8" "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", ToHex(TupleEncoder() - .AddFixedBytes(Span32(data.begin(), 32u)) + .AddFixedBytes(base::span(data).first(32u)) .EncodeWithSelector(selector)) .substr(2)); } diff --git a/components/brave_wallet/common/eth_address.cc b/components/brave_wallet/common/eth_address.cc index 2c7e515cdfe3..c6dd02580923 100644 --- a/components/brave_wallet/common/eth_address.cc +++ b/components/brave_wallet/common/eth_address.cc @@ -3,17 +3,10 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/components/brave_wallet/common/eth_address.h" #include -#include "base/check_op.h" #include "base/containers/span.h" #include "base/logging.h" #include "base/ranges/algorithm.h" diff --git a/components/brave_wallet/common/eth_request_helper.cc b/components/brave_wallet/common/eth_request_helper.cc index 514e36e8f4c5..afce42c29800 100644 --- a/components/brave_wallet/common/eth_request_helper.cc +++ b/components/brave_wallet/common/eth_request_helper.cc @@ -5,14 +5,12 @@ #include "brave/components/brave_wallet/common/eth_request_helper.h" -#include #include #include #include #include #include "base/base64.h" -#include "base/compiler_specific.h" #include "base/containers/span.h" #include "base/json/json_reader.h" #include "base/json/json_writer.h" @@ -531,12 +529,14 @@ mojom::EthSignTypedDataPtr ParseEthSignTypedDataParams( result->meta = nullptr; } - result->domain_hash = domain_hash->first; + result->domain_hash.assign(domain_hash->first.begin(), + domain_hash->first.end()); if (!base::JSONWriter::Write(domain_hash->second, &result->domain_json)) { return nullptr; } - result->primary_hash = primary_hash->first; + result->primary_hash.assign(primary_hash->first.begin(), + primary_hash->first.end()); if (!base::JSONWriter::Write(primary_hash->second, &result->message_json)) { return nullptr; } diff --git a/components/brave_wallet/common/eth_sign_typed_data_helper.cc b/components/brave_wallet/common/eth_sign_typed_data_helper.cc index 1a4747c8ab6c..a304022fe4ce 100644 --- a/components/brave_wallet/common/eth_sign_typed_data_helper.cc +++ b/components/brave_wallet/common/eth_sign_typed_data_helper.cc @@ -3,16 +3,11 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at https://mozilla.org/MPL/2.0/. */ -#ifdef UNSAFE_BUFFERS_BUILD -// TODO(https://github.com/brave/brave-browser/issues/41661): Remove this and -// convert code to safer constructs. -#pragma allow_unsafe_buffers -#endif - #include "brave/components/brave_wallet/common/eth_sign_typed_data_helper.h" #include #include +#include #include #include "base/containers/extend.h" @@ -190,8 +185,9 @@ EthSignTypedDataHelper::EncodeData(const std::string& primary_type_name, // Encode each field of a custom type, if a field is also a custom type it // will call EncodeData recursively until it reaches an atomic type std::optional EthSignTypedDataHelper::EncodeField( - const std::string& type, + const std::string& type_string, const base::Value& value) const { + auto type = std::string_view(type_string); // ES6 section 20.1.2.6 Number.MAX_SAFE_INTEGER constexpr double kMaxSafeInteger = static_cast(kMaxSafeIntegerUint64); @@ -269,7 +265,7 @@ std::optional EthSignTypedDataHelper::EncodeField( if (base::StartsWith(type, "bytes", base::CompareCase::SENSITIVE)) { unsigned num_bits; - if (!base::StringToUint(type.data() + 5, &num_bits) || num_bits > 32) { + if (!base::StringToUint(type.substr(5), &num_bits) || num_bits > 32) { return std::nullopt; } const std::string* value_str = value.GetIfString(); @@ -289,7 +285,7 @@ std::optional EthSignTypedDataHelper::EncodeField( if (base::StartsWith(type, "uint", base::CompareCase::SENSITIVE)) { // uint8 to uint256 in steps of 8 unsigned num_bits; - if (!base::StringToUint(type.data() + 4, &num_bits) || + if (!base::StringToUint(type.substr(4), &num_bits) || !ValidSolidityBits(num_bits)) { return std::nullopt; } @@ -322,11 +318,8 @@ std::optional EthSignTypedDataHelper::EncodeField( } KeccakHashArray result = {}; - auto encoded_value_span = base::byte_span_from_ref(encoded_value); - CHECK(result.size() == encoded_value_span.size()); - for (size_t i = 0; i < 32; ++i) { - result[i] = encoded_value_span[32 - i - 1]; - } + base::span(result).copy_from(base::byte_span_from_ref(encoded_value)); + base::ranges::reverse(result); return result; } @@ -334,7 +327,7 @@ std::optional EthSignTypedDataHelper::EncodeField( if (base::StartsWith(type, "int", base::CompareCase::SENSITIVE)) { // int8 to int256 in steps of 8 unsigned num_bits; - if (!base::StringToUint(type.data() + 3, &num_bits) || + if (!base::StringToUint(type.substr(3), &num_bits) || !ValidSolidityBits(num_bits)) { return std::nullopt; } @@ -368,11 +361,8 @@ std::optional EthSignTypedDataHelper::EncodeField( } KeccakHashArray result = {}; - auto encoded_value_span = base::byte_span_from_ref(encoded_value); - CHECK(result.size() == encoded_value_span.size()); - for (size_t i = 0; i < 32; ++i) { - result[i] = encoded_value_span[32 - i - 1]; - } + base::span(result).copy_from(base::byte_span_from_ref(encoded_value)); + base::ranges::reverse(result); return result; } @@ -380,7 +370,7 @@ std::optional EthSignTypedDataHelper::EncodeField( if (!value.is_dict()) { return std::nullopt; } - auto encoded_data = EncodeData(type, value.GetDict()); + auto encoded_data = EncodeData(type_string, value.GetDict()); if (!encoded_data) { return std::nullopt; } @@ -401,8 +391,7 @@ EthSignTypedDataHelper::GetTypedDataPrimaryHash( } // static -std::optional -EthSignTypedDataHelper::GetTypedDataMessageToSign( +KeccakHashArray EthSignTypedDataHelper::GetTypedDataMessageToSign( base::span domain_hash, base::span primary_hash) { DCHECK(!domain_hash.empty()); diff --git a/components/brave_wallet/common/hash_utils.cc b/components/brave_wallet/common/hash_utils.cc index 90582a473d5d..955eaa0691ec 100644 --- a/components/brave_wallet/common/hash_utils.cc +++ b/components/brave_wallet/common/hash_utils.cc @@ -8,8 +8,6 @@ #include #include -#include "base/check.h" -#include "base/compiler_specific.h" #include "base/containers/adapters.h" #include "base/containers/span.h" #include "base/ranges/algorithm.h" @@ -55,8 +53,8 @@ eth_abi::Bytes4 GetFunctionHashBytes4(const std::string& input) { eth_abi::Bytes32 Namehash(const std::string& name) { eth_abi::Bytes32 hash = {}; - std::vector labels = - SplitString(name, ".", base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY); + auto labels = SplitStringPiece(name, ".", base::KEEP_WHITESPACE, + base::SPLIT_WANT_NONEMPTY); for (const auto& label : base::Reversed(labels)) { auto label_hash = KeccakHash(base::as_byte_span(label)); @@ -69,15 +67,11 @@ SHA256HashArray DoubleSHA256Hash(base::span input) { return crypto::SHA256Hash(crypto::SHA256Hash(input)); } -std::vector Hash160(base::span input) { - std::vector result(CRIPEMD160::OUTPUT_SIZE); - - std::array sha256hash = - crypto::SHA256Hash(input); - DCHECK(!sha256hash.empty()); +Ripemd160HashArray Hash160(base::span input) { + Ripemd160HashArray result = {}; CRIPEMD160() - .Write(sha256hash.data(), sha256hash.size()) + .Write(crypto::SHA256Hash(input).data(), crypto::kSHA256Length) .Finalize(result.data()); return result; diff --git a/components/brave_wallet/common/hash_utils.h b/components/brave_wallet/common/hash_utils.h index 34919e4baf7e..2af8e272f15e 100644 --- a/components/brave_wallet/common/hash_utils.h +++ b/components/brave_wallet/common/hash_utils.h @@ -8,15 +8,18 @@ #include #include -#include #include "brave/components/brave_wallet/common/eth_abi_utils.h" #include "crypto/sha2.h" namespace brave_wallet { -static const size_t kKeccakHashLength = 32; +inline constexpr size_t kKeccakHashLength = 32; +inline constexpr size_t kRipemd160HashLength = 20; + using KeccakHashArray = std::array; +using SHA256HashArray = std::array; +using Ripemd160HashArray = std::array; KeccakHashArray KeccakHash(base::span input); @@ -31,11 +34,10 @@ eth_abi::Bytes4 GetFunctionHashBytes4(const std::string& input); eth_abi::Bytes32 Namehash(const std::string& name); // sha256(sha256(input)) -using SHA256HashArray = std::array; SHA256HashArray DoubleSHA256Hash(base::span input); // ripemd160(sha256(input)) -std::vector Hash160(base::span input); +Ripemd160HashArray Hash160(base::span input); } // namespace brave_wallet diff --git a/components/brave_wallet/common/hex_utils.cc b/components/brave_wallet/common/hex_utils.cc index dc7a8fbd8fe4..090aff8b2ff7 100644 --- a/components/brave_wallet/common/hex_utils.cc +++ b/components/brave_wallet/common/hex_utils.cc @@ -10,10 +10,8 @@ #include "base/containers/adapters.h" #include "base/containers/span.h" -#include "base/debug/crash_logging.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" -#include "base/strings/stringprintf.h" namespace brave_wallet { diff --git a/components/brave_wallet/common/mem_utils.h b/components/brave_wallet/common/mem_utils.h index a708335d1a02..ada9d699e352 100644 --- a/components/brave_wallet/common/mem_utils.h +++ b/components/brave_wallet/common/mem_utils.h @@ -39,6 +39,18 @@ struct SecureZeroAllocator { } }; +template +struct SecureByteArray : public std::array { + ~SecureByteArray() { SecureZeroData(base::span(*this)); } +}; + } // namespace brave_wallet +namespace base { +namespace internal { +template +struct ExtentImpl> : size_constant {}; +} // namespace internal +} // namespace base + #endif // BRAVE_COMPONENTS_BRAVE_WALLET_COMMON_MEM_UTILS_H_ diff --git a/components/brave_wallet/common/zcash_utils.cc b/components/brave_wallet/common/zcash_utils.cc index 997ec72b76d9..1ce30a850fa2 100644 --- a/components/brave_wallet/common/zcash_utils.cc +++ b/components/brave_wallet/common/zcash_utils.cc @@ -10,6 +10,7 @@ #include #include +#include "base/containers/extend.h" #include "base/containers/span.h" #include "base/numerics/byte_conversions.h" #include "base/types/expected.h" @@ -189,8 +190,8 @@ std::string PubkeyToTransparentAddress(base::span pubkey, bool testnet) { std::vector result = GetNetworkPrefix(testnet); - std::vector data_part = Hash160(pubkey); - result.insert(result.end(), data_part.begin(), data_part.end()); + base::Extend(result, Hash160(pubkey)); + return Base58EncodeWithCheck(result); } diff --git a/components/brave_wallet/renderer/js_solana_provider.cc b/components/brave_wallet/renderer/js_solana_provider.cc index 1a5882f37c09..ba9f18ffa0d2 100644 --- a/components/brave_wallet/renderer/js_solana_provider.cc +++ b/components/brave_wallet/renderer/js_solana_provider.cc @@ -720,7 +720,7 @@ void JSSolanaProvider::OnSignMessage( const base::Value signature_value(signature_bytes); v8::Local v8_signature = v8_value_converter_->ToV8Value(signature_value, context); - // From ArraryBuffer to Uint8Array + // From ArrayBuffer to Uint8Array v8_signature = v8::Uint8Array::New(v8::Local::Cast(v8_signature), 0, (kSolanaSignatureSize)); diff --git a/components/brave_wallet/renderer/js_solana_provider.h b/components/brave_wallet/renderer/js_solana_provider.h index 76cffb138b4f..db738d1a93df 100644 --- a/components/brave_wallet/renderer/js_solana_provider.h +++ b/components/brave_wallet/renderer/js_solana_provider.h @@ -73,7 +73,7 @@ class JSSolanaProvider final : public gin::Wrappable, // signature: }> // display encoding is optional v8::Local SignMessage(gin::Arguments* arguments); - // It takes { method: , pararms: {...} and return promise accroding to + // It takes { method: , params: {...} and return promise according to // the method: // - connect => { publicKey: solanaWeb3.PublicKey} // - disconnect => {} @@ -185,7 +185,6 @@ class JSSolanaProvider final : public gin::Wrappable, bool wallet_standard_loaded_ = false; v8::Global solana_web3_module_; std::unique_ptr v8_value_converter_; - // V8ConverterStrategy strategy_; mojo::Remote solana_provider_; mojo::Receiver receiver_{this}; };