Skip to content

Commit

Permalink
Fix UNSAFE_TODO for wallet [part 2 of N] (#26469)
Browse files Browse the repository at this point in the history
  • Loading branch information
supermassive authored and emerick committed Dec 2, 2024
1 parent 02dfffa commit 10b809c
Show file tree
Hide file tree
Showing 24 changed files with 205 additions and 236 deletions.
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
44 changes: 17 additions & 27 deletions components/brave_wallet/browser/eth_abi_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,9 @@
* 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>
Expand Down Expand Up @@ -98,8 +91,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 @@ -116,10 +108,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 @@ -208,7 +202,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 @@ -474,37 +468,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 @@ -516,9 +508,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

0 comments on commit 10b809c

Please sign in to comment.