Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UNSAFE_TODO for wallet [part 2 of N] #26469

Merged
merged 4 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion components/brave_wallet/browser/eip1559_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <optional>
#include <utility>

#include "base/containers/to_vector.h"
#include "base/values.h"
#include "brave/components/brave_wallet/browser/rlp_encode.h"
#include "brave/components/brave_wallet/common/hash_utils.h"
Expand Down Expand Up @@ -289,7 +290,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 ? base::ToVector(KeccakHash(result)) : result;
}

std::string Eip1559Transaction::GetSignedTransaction() const {
Expand Down
3 changes: 2 additions & 1 deletion components/brave_wallet/browser/eip2930_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <optional>
#include <utility>

#include "base/containers/to_vector.h"
#include "base/values.h"
#include "brave/components/brave_wallet/browser/rlp_encode.h"
#include "brave/components/brave_wallet/common/eth_address.h"
Expand Down Expand Up @@ -179,7 +180,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 ? base::ToVector(KeccakHash(result)) : result;
}

std::string Eip2930Transaction::GetSignedTransaction() const {
Expand Down
45 changes: 18 additions & 27 deletions components/brave_wallet/browser/eth_abi_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +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 <limits>
#include <map>
#include <memory>
#include <optional>
#include <utility>

#include "base/containers/span.h"
#include "base/containers/span_reader.h"
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
Expand Down Expand Up @@ -96,8 +90,7 @@ std::optional<DecoderResult<base::Value>> GetAddressFromData(ByteView input) {
}

return DecoderResult<base::Value>(
base::Value("0x" + HexEncodeLower(input.data() + kWordSize - kAddressSize,
kAddressSize)),
base::Value(ToHex(input.first(kWordSize).last(kAddressSize))),
GetSubByteView(input, kWordSize), kWordSize);
}

Expand All @@ -114,10 +107,12 @@ std::optional<DecoderResult<M>> GetUintFromData(ByteView input) {
return std::nullopt;
}

auto arg = HexEncodeLower(input.data(), kWordSize);

uint256_t value;
if (!HexValueToUint256("0x" + arg, &value)) {
// TODO(apaymyshev): we don't need string in this bytes->string->bytes
// conversion here.
if (!HexValueToUint256(
base::StrCat({"0x", HexEncodeLower(input.first(kWordSize))}),
&value)) {
return std::nullopt;
}

Expand Down Expand Up @@ -206,7 +201,7 @@ std::optional<DecoderResult<base::Value>> GetBytesHexFromData(

size_t parts_size = parts_count * kWordSize;
return DecoderResult<base::Value>(
base::Value("0x" + HexEncodeLower(remaining.data(), size)),
base::Value(base::StrCat({"0x", HexEncodeLower(remaining.first(size))})),
GetSubByteView(remaining, parts_size), consumed + parts_size);
}

Expand Down Expand Up @@ -472,37 +467,35 @@ std::optional<std::vector<std::string>> UniswapEncodedPathDecode(
if (!PrefixedHexStringToBytes(encoded_path, &data)) {
return std::nullopt;
}
size_t offset = 0;
std::vector<std::string> 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(base::StrCat({"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(base::StrCat({"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.
Expand All @@ -514,9 +507,7 @@ std::optional<std::vector<std::string>> UniswapEncodedPathDecode(
}

std::optional<base::Value::List> ABIDecode(const eth_abi::Type& type,
const ByteArray& data) {
ByteView input = base::make_span(data.data(), data.size());

base::span<const uint8_t> input) {
auto decoded = DecodeParam(type, input);
if (!decoded) {
return std::nullopt;
Expand Down
3 changes: 1 addition & 2 deletions components/brave_wallet/browser/eth_abi_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

#include <optional>
#include <string>
#include <tuple>
#include <vector>

#include "base/values.h"
Expand All @@ -20,7 +19,7 @@ std::optional<std::vector<std::string>> UniswapEncodedPathDecode(
const std::string& encoded_path);

std::optional<base::Value::List> ABIDecode(const eth_abi::Type& type,
const std::vector<uint8_t>& data);
base::span<const uint8_t> input);

} // namespace brave_wallet

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
11 changes: 6 additions & 5 deletions components/brave_wallet/browser/eth_data_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,10 @@ std::optional<SquidSwapData> SquidDecodeCall(const base::Value::List& call) {
return std::nullopt;
}

std::vector<uint8_t> calldata(calldata_with_selector->begin() + 4,
calldata_with_selector->end());
auto [selector_span, calldata] =
base::span(*calldata_with_selector).split_at(4);

auto selector = "0x" + HexEncodeLower(calldata_with_selector->data(), 4);
auto selector = ToHex(selector_span);

if (selector == kSquidExactInputSingle) {
// exactInputSingle((address tokenIn,
Expand Down Expand Up @@ -544,8 +544,9 @@ GetTransactionInfoFromData(const std::vector<uint8_t>& data) {
std::vector<std::string>(), nullptr);
}

std::string selector = "0x" + HexEncodeLower(data.data(), 4);
std::vector<uint8_t> calldata(data.begin() + 4, data.end());
auto [selector_span, calldata] = base::span(data).split_at(4);

std::string selector = ToHex(selector_span);
if (selector == kFilForwarderTransferSelector) {
auto type = eth_abi::Tuple().AddTupleType(eth_abi::Bytes()).build();
auto decoded = ABIDecode(type, calldata);
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
5 changes: 3 additions & 2 deletions components/brave_wallet/browser/eth_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <utility>

#include "base/base64.h"
#include "base/containers/to_vector.h"
#include "base/logging.h"
#include "base/strings/string_number_conversions.h"
#include "base/values.h"
Expand Down Expand Up @@ -189,7 +190,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 ? base::ToVector(KeccakHash(result)) : result;
}

std::string EthTransaction::GetSignedTransaction() const {
Expand All @@ -202,7 +203,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
12 changes: 7 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,10 @@
#include <optional>

#include "base/base64.h"
#include "base/containers/extend.h"
#include "base/containers/span.h"
#include "base/containers/to_vector.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 +23,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 base::ToVector(KeccakHash(hash_input));
}

} // namespace
Expand Down
3 changes: 2 additions & 1 deletion components/brave_wallet/browser/ethereum_provider_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <vector>

#include "base/containers/contains.h"
#include "base/containers/to_vector.h"
#include "base/json/json_reader.h"
#include "base/json/json_writer.h"
#include "base/strings/strcat.h"
Expand Down Expand Up @@ -685,7 +686,7 @@ void EthereumProviderImpl::SignTypedMessage(
mojom::SignDataUnion::NewEthSignTypedData(std::move(eth_sign_typed_data));

SignMessageInternal(account_id, std::move(sign_data),
std::move(message_to_sign), std::move(callback),
base::ToVector(message_to_sign), std::move(callback),
std::move(id));
}

Expand Down
11 changes: 6 additions & 5 deletions components/brave_wallet/browser/internal/hd_key.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include "base/check.h"
#include "base/containers/span.h"
#include "base/containers/to_vector.h"
#include "base/json/json_reader.h"
#include "base/logging.h"
#include "base/numerics/byte_conversions.h"
Expand All @@ -25,6 +26,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 +80,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 +398,7 @@ void HDKey::SetPrivateKey(base::span<const uint8_t> value) {
}
private_key_.assign(value.begin(), value.end());
GeneratePublicKey();
identifier_ = Hash160(public_key_);
identifier_ = base::ToVector(Hash160(public_key_));

const uint8_t* ptr = identifier_.data();
fingerprint_ = ptr[0] << 24 | ptr[1] << 16 | ptr[2] << 8 | ptr[3] << 0;
Expand Down Expand Up @@ -425,8 +426,8 @@ void HDKey::SetPublicKey(
LOG(ERROR) << __func__ << ": not a valid public key";
return;
}
public_key_.assign(value.begin(), value.end());
identifier_ = Hash160(public_key_);
public_key_ = base::ToVector(value);
identifier_ = base::ToVector(Hash160(public_key_));

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 @@ -9,6 +9,7 @@
#include <optional>

#include "base/check.h"
#include "base/containers/to_vector.h"
#include "brave/components/brave_wallet/common/common_utils.h"
#include "brave/components/brave_wallet/common/hash_utils.h"
#include "brave/components/brave_wallet/common/zcash_utils.h"
Expand Down Expand Up @@ -72,7 +73,7 @@ std::optional<std::vector<uint8_t>> ZCashKeyring::GetPubkeyHash(
return std::nullopt;
}

return Hash160(hd_key_base->GetPublicKeyBytes());
return base::ToVector(Hash160(hd_key_base->GetPublicKeyBytes()));
}

#if BUILDFLAG(ENABLE_ORCHARD)
Expand Down
Loading
Loading