Skip to content

Commit

Permalink
Fix UNSAFE_TODO for wallet
Browse files Browse the repository at this point in the history
  • Loading branch information
supermassive committed Nov 11, 2024
1 parent 24529ee commit 4758598
Show file tree
Hide file tree
Showing 17 changed files with 163 additions and 179 deletions.
2 changes: 1 addition & 1 deletion components/brave_wallet/browser/eip1559_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ std::vector<uint8_t> Eip1559Transaction::GetMessageToSign(uint256_t chain_id,

const std::string rlp_msg = RLPEncode(base::Value(std::move(list)));
result.insert(result.end(), rlp_msg.begin(), rlp_msg.end());
return hash ? KeccakHash(result) : result;
return hash ? KeccakHashToVector(result) : result;
}

std::string Eip1559Transaction::GetSignedTransaction() const {
Expand Down
2 changes: 1 addition & 1 deletion components/brave_wallet/browser/eip2930_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ std::vector<uint8_t> Eip2930Transaction::GetMessageToSign(uint256_t chain_id,

const std::string rlp_msg = RLPEncode(base::Value(std::move(list)));
result.insert(result.end(), rlp_msg.begin(), rlp_msg.end());
return hash ? KeccakHash(result) : result;
return hash ? KeccakHashToVector(result) : result;
}

std::string Eip2930Transaction::GetSignedTransaction() const {
Expand Down
3 changes: 2 additions & 1 deletion components/brave_wallet/browser/eth_allowance_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ void EthAllowanceManager::OnGetCurrentBlock(
return;
}

const auto approval_topic_hash = KeccakHash(kApprovalTopicFunctionSignature);
const auto approval_topic_hash = ToHex(KeccakHash(
base::byte_span_from_cstring(kApprovalTopicFunctionSignature)));
for (const auto& account_address : account_addresses) {
std::string account_address_hex;
if (!PadHexEncodedParameter(account_address, &account_address_hex)) {
Expand Down
3 changes: 2 additions & 1 deletion components/brave_wallet/browser/eth_topics_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ bool MakeAssetDiscoveryTopics(
const std::vector<std::string>& to_account_addresses,
base::Value::List* topics) {
// First topic matches full keccak hash of the erc20::Transfer event signature
topics->Append(brave_wallet::KeccakHash("Transfer(address,address,uint256)"));
topics->Append(ToHex(KeccakHash(
base::byte_span_from_cstring("Transfer(address,address,uint256)"))));

// Second topic matches everything (any from_address)
topics->Append(base::Value());
Expand Down
4 changes: 2 additions & 2 deletions components/brave_wallet/browser/eth_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ std::vector<uint8_t> EthTransaction::GetMessageToSign(uint256_t chain_id,

const std::string message = RLPEncode(base::Value(std::move(list)));
auto result = std::vector<uint8_t>(message.begin(), message.end());
return hash ? KeccakHash(result) : result;
return hash ? KeccakHashToVector(result) : result;
}

std::string EthTransaction::GetSignedTransaction() const {
Expand All @@ -202,7 +202,7 @@ std::string EthTransaction::GetTransactionHash() const {
DCHECK(IsSigned());
DCHECK(nonce_);

return KeccakHash(RLPEncode(Serialize()));
return ToHex(KeccakHash(base::as_byte_span(RLPEncode(Serialize()))));
}

bool EthTransaction::ProcessVRS(const std::vector<uint8_t>& v,
Expand Down
11 changes: 6 additions & 5 deletions components/brave_wallet/browser/ethereum_keyring.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
#include <optional>

#include "base/base64.h"
#include "base/containers/extend.h"
#include "base/containers/span.h"
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
#include "brave/components/brave_wallet/browser/eth_transaction.h"
#include "brave/components/brave_wallet/common/eth_address.h"
Expand All @@ -20,12 +22,11 @@ namespace {

// Get the 32 byte message hash
std::vector<uint8_t> GetMessageHash(base::span<const uint8_t> message) {
std::string prefix("\x19");
prefix += std::string("Ethereum Signed Message:\n" +
base::NumberToString(message.size()));
std::string prefix = base::StrCat({"\x19", "Ethereum Signed Message:\n",
base::NumberToString(message.size())});
std::vector<uint8_t> hash_input(prefix.begin(), prefix.end());
hash_input.insert(hash_input.end(), message.begin(), message.end());
return brave_wallet::KeccakHash(hash_input);
base::Extend(hash_input, message);
return KeccakHashToVector(hash_input);
}

} // namespace
Expand Down
7 changes: 4 additions & 3 deletions components/brave_wallet/browser/ethereum_provider_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -684,9 +684,10 @@ void EthereumProviderImpl::SignTypedMessage(
mojom::SignDataUnionPtr sign_data =
mojom::SignDataUnion::NewEthSignTypedData(std::move(eth_sign_typed_data));

SignMessageInternal(account_id, std::move(sign_data),
std::move(message_to_sign), std::move(callback),
std::move(id));
SignMessageInternal(
account_id, std::move(sign_data),
std::vector<uint8_t>(message_to_sign.begin(), message_to_sign.end()),
std::move(callback), std::move(id));
}

void EthereumProviderImpl::SignMessageInternal(
Expand Down
10 changes: 6 additions & 4 deletions components/brave_wallet/browser/internal/hd_key.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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/hex_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"
Expand Down Expand Up @@ -78,8 +79,7 @@ bool UTCPasswordVerification(const std::string& derived_key,
mac_verification_input.insert(mac_verification_input.end(),
ciphertext.begin(), ciphertext.end());
// verify password
std::vector<uint8_t> mac_verification(KeccakHash(mac_verification_input));
if (base::ToLowerASCII(base::HexEncode(mac_verification)) != mac) {
if (HexEncodeLower(KeccakHash(mac_verification_input)) != mac) {
VLOG(0) << __func__ << ": password does not match";
return false;
}
Expand Down Expand Up @@ -397,7 +397,8 @@ void HDKey::SetPrivateKey(base::span<const uint8_t> value) {
}
private_key_.assign(value.begin(), value.end());
GeneratePublicKey();
identifier_ = Hash160(public_key_);
auto pubkey_hash = Hash160(public_key_);
identifier_.assign(pubkey_hash.begin(), pubkey_hash.end());

const uint8_t* ptr = identifier_.data();
fingerprint_ = ptr[0] << 24 | ptr[1] << 16 | ptr[2] << 8 | ptr[3] << 0;
Expand Down Expand Up @@ -426,7 +427,8 @@ void HDKey::SetPublicKey(
return;
}
public_key_.assign(value.begin(), value.end());
identifier_ = Hash160(public_key_);
auto pubkey_hash = Hash160(public_key_);
identifier_.assign(pubkey_hash.begin(), pubkey_hash.end());

const uint8_t* ptr = identifier_.data();
fingerprint_ = ptr[0] << 24 | ptr[1] << 16 | ptr[2] << 8 | ptr[3] << 0;
Expand Down
3 changes: 2 additions & 1 deletion components/brave_wallet/browser/zcash/zcash_keyring.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ std::optional<std::vector<uint8_t>> ZCashKeyring::GetPubkeyHash(
return std::nullopt;
}

return Hash160(hd_key_base->GetPublicKeyBytes());
auto hash = Hash160(hd_key_base->GetPublicKeyBytes());
return std::vector<uint8_t>{hash.begin(), hash.end()};
}

#if BUILDFLAG(ENABLE_ORCHARD)
Expand Down
32 changes: 10 additions & 22 deletions components/brave_wallet/common/eth_address.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +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/common/eth_address.h"

#include <utility>

#include "base/check_op.h"
#include "base/containers/span.h"
#include "base/logging.h"
#include "base/ranges/algorithm.h"
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
#include "brave/components/brave_wallet/common/hash_utils.h"
Expand Down Expand Up @@ -45,12 +40,8 @@ EthAddress EthAddress::FromPublicKey(const std::vector<uint8_t>& public_key) {
return EthAddress();
}

std::vector<uint8_t> hash = KeccakHash(public_key);
std::vector<uint8_t> result(hash.end() - kEthAddressLength, hash.end());

DCHECK_EQ(result.size(), kEthAddressLength);

return EthAddress(std::move(result));
return EthAddress(
base::as_byte_span(KeccakHash(public_key)).last(kEthAddressLength));
}

// static
Expand Down Expand Up @@ -95,8 +86,7 @@ bool EthAddress::IsValidAddress(const std::string& input) {
}

std::string EthAddress::ToHex() const {
const std::string input(bytes_.begin(), bytes_.end());
return ::brave_wallet::ToHex(input);
return ::brave_wallet::ToHex(bytes_);
}

// static
Expand All @@ -121,21 +111,19 @@ std::optional<std::string> EthAddress::ToEip1191ChecksumAddress(

std::string EthAddress::ToChecksumAddress(uint256_t eip1191_chaincode) const {
std::string result = "0x";
std::string input;
std::string prefix;

if (eip1191_chaincode == static_cast<uint256_t>(30) ||
eip1191_chaincode == static_cast<uint256_t>(31)) {
// TODO(jocelyn): We will need to revise this if there are supported chains
// with ID larger than uint64_t.
input +=
prefix =
base::NumberToString(static_cast<uint64_t>(eip1191_chaincode)) + "0x";
}

input += std::string(ToHex().data() + 2);

const std::string hash_str(KeccakHash(input).data() + 2);
const std::string address_str =
base::ToLowerASCII(base::HexEncode(bytes_.data(), bytes_.size()));
const std::string address_str = HexEncodeLower(bytes_.data(), bytes_.size());
const std::string hash_str = base::HexEncode(
KeccakHash(base::as_byte_span(base::StrCat({prefix, address_str}))));

for (size_t i = 0; i < address_str.length(); ++i) {
if (isdigit(address_str[i])) {
Expand Down
6 changes: 4 additions & 2 deletions components/brave_wallet/common/eth_request_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -528,12 +528,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;
}
Expand Down
Loading

0 comments on commit 4758598

Please sign in to comment.