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.

Sign up for GitHub

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

Jump to bottom

fix(wallet): correctly handle extra calldata bytes (uplift to 1.42.x) #14162

Merged
merged 1 commit into from
Jul 18, 2022
Merged

fix(wallet): correctly handle extra calldata bytes (uplift to 1.42.x) #14162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 8 additions & 10 deletions components/brave_wallet/browser/eth_abi_decoder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,14 @@ absl::optional<std::vector<std::string>> UniswapEncodedPathDecode(
return path;
}

absl::optional<std::tuple<std::vector<std::string>, // tx_params
std::vector<std::string>>> // tx_args
absl::optional<std::tuple<std::vector<std::string>, // params
std::vector<std::string>>> // args
ABIDecode(const std::vector<std::string>& types,
const std::vector<uint8_t>& data) {
size_t offset = 0;
size_t calldata_tail = 0;
std::vector<std::string> tx_params;
std::vector<std::string> tx_args;
std::vector<std::string> params;
std::vector<std::string> args;

for (const auto& type : types) {
absl::optional<std::string> value;
Expand Down Expand Up @@ -285,15 +285,13 @@ ABIDecode(const std::vector<std::string>& types,

offset += 32;

tx_args.push_back(*value);
tx_params.push_back(type);
args.push_back(*value);
params.push_back(type);
}

// Extraneous calldata, unless in the tail section, is considered invalid.
if (offset != calldata_tail && offset < data.size())
return absl::nullopt;
// Extra calldata bytes are ignored.

return std::make_tuple(tx_params, tx_args);
return std::make_tuple(params, args);
}

} // namespace brave_wallet
32 changes: 21 additions & 11 deletions components/brave_wallet/browser/eth_abi_decoder_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,68 +39,78 @@ TEST(EthABIDecoderTest, ABIDecodeAddress) {
TEST(EthABIDecoderTest, ABIDecodeUint256) {
std::vector<std::string> tx_params;
std::vector<std::string> tx_args;

std::vector<uint8_t> data;

// OK: 32-bytes well-formed uint256
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x00000000000000000000000000000000000000000000000000000000000000ff",
&data));
auto decoded = ABIDecode({"uint256"}, data);
ASSERT_NE(decoded, absl::nullopt);
std::tie(tx_params, tx_args) = *decoded;

ASSERT_EQ(tx_params.size(), 1UL);
EXPECT_EQ(tx_params[0], "uint256");
EXPECT_EQ(tx_args[0], "0xff");

// Insufficient uint256 length
// KO: insufficient uint256 length
ASSERT_TRUE(PrefixedHexStringToBytes("0xff", &data));
ASSERT_FALSE(ABIDecode({"uint256"}, data));

// Extra uint256 length
// OK: extra uint256 length
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x00000000000000000000000000000000000000000000000000000000000000ff"
"ff",
&data));
ASSERT_FALSE(ABIDecode({"uint256"}, data));
decoded = ABIDecode({"uint256"}, data);
ASSERT_NE(decoded, absl::nullopt);
std::tie(tx_params, tx_args) = *decoded;
ASSERT_EQ(tx_params.size(), 1UL);
EXPECT_EQ(tx_params[0], "uint256");
EXPECT_EQ(tx_args[0], "0xff");
}

TEST(EthABIDecoderTest, ABIDecodeBool) {
std::vector<std::string> tx_params;
std::vector<std::string> tx_args;

std::vector<uint8_t> data;

// OK: false
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x0000000000000000000000000000000000000000000000000000000000000000",
&data));
auto decoded = ABIDecode({"bool"}, data);
ASSERT_NE(decoded, absl::nullopt);
std::tie(tx_params, tx_args) = *decoded;

ASSERT_EQ(tx_params.size(), 1UL);
EXPECT_EQ(tx_params[0], "bool");
EXPECT_EQ(tx_args[0], "false");

// OK: true
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x0000000000000000000000000000000000000000000000000000000000000001",
&data));
decoded = ABIDecode({"bool"}, data);
ASSERT_NE(decoded, absl::nullopt);
std::tie(tx_params, tx_args) = *decoded;

ASSERT_EQ(tx_params.size(), 1UL);
EXPECT_EQ(tx_params[0], "bool");
EXPECT_EQ(tx_args[0], "true");

// Insufficient bool length
// KO: insufficient bool length
ASSERT_TRUE(PrefixedHexStringToBytes("0x0", &data));
ASSERT_FALSE(ABIDecode({"bool"}, data));

// Extra bool length
// OK: extra bool length
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x0000000000000000000000000000000000000000000000000000000000000000"
"00",
&data));
ASSERT_FALSE(ABIDecode({"bool"}, data));
decoded = ABIDecode({"bool"}, data);
ASSERT_NE(decoded, absl::nullopt);
std::tie(tx_params, tx_args) = *decoded;
ASSERT_EQ(tx_params.size(), 1UL);
EXPECT_EQ(tx_params[0], "bool");
EXPECT_EQ(tx_args[0], "false");
}

TEST(EthABIDecoderTest, ABIDecodeAddressArray) {
Expand Down
76 changes: 52 additions & 24 deletions components/brave_wallet/browser/eth_data_parser_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ TEST(EthDataParser, GetTransactionInfoFromDataTransfer) {
mojom::TransactionType tx_type;
std::vector<std::string> tx_params;
std::vector<std::string> tx_args;

std::vector<uint8_t> data;

// OK: well-formed ERC20Transfer
ASSERT_TRUE(PrefixedHexStringToBytes(
"0xa9059cbb"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f"
Expand All @@ -54,7 +55,6 @@ TEST(EthDataParser, GetTransactionInfoFromDataTransfer) {
auto tx_info = GetTransactionInfoFromData(data);
ASSERT_NE(tx_info, absl::nullopt);
std::tie(tx_type, tx_params, tx_args) = *tx_info;

ASSERT_EQ(tx_type, mojom::TransactionType::ERC20Transfer);
ASSERT_EQ(tx_params.size(), 2UL);
EXPECT_EQ(tx_params[0], "address");
Expand All @@ -63,41 +63,51 @@ TEST(EthDataParser, GetTransactionInfoFromDataTransfer) {
EXPECT_EQ(tx_args[0], "0xbfb30a082f650c2a15d0632f0e87be4f8e64460f");
EXPECT_EQ(tx_args[1], "0x3fffffffffffffff");

// Missing a byte for the last param
// KO: missing a byte for the last param
ASSERT_TRUE(PrefixedHexStringToBytes(
"0xa9059cbb"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f"
"0000000000000000000000000000000000000000000000003fffffffffffff",
&data));
EXPECT_FALSE(GetTransactionInfoFromData(data));

// Missing the entire last param
// KO: missing the entire last param
ASSERT_TRUE(PrefixedHexStringToBytes(
"0xa9059cbb"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f",
&data));
EXPECT_FALSE(GetTransactionInfoFromData(data));

// No params
// KO: no params
ASSERT_TRUE(PrefixedHexStringToBytes("0xa9059cbb", &data));
EXPECT_FALSE(GetTransactionInfoFromData(data));

// Extra data
// OK: extra data
ASSERT_TRUE(PrefixedHexStringToBytes(
"0xa9059cbb"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f"
"0000000000000000000000000000000000000000000000003fffffffffffffff"
"00",
&data));
EXPECT_FALSE(GetTransactionInfoFromData(data));
tx_info = GetTransactionInfoFromData(data);
ASSERT_NE(tx_info, absl::nullopt);
std::tie(tx_type, tx_params, tx_args) = *tx_info;
ASSERT_EQ(tx_type, mojom::TransactionType::ERC20Transfer);
ASSERT_EQ(tx_params.size(), 2UL);
EXPECT_EQ(tx_params[0], "address");
EXPECT_EQ(tx_params[1], "uint256");
ASSERT_EQ(tx_args.size(), 2UL);
EXPECT_EQ(tx_args[0], "0xbfb30a082f650c2a15d0632f0e87be4f8e64460f");
EXPECT_EQ(tx_args[1], "0x3fffffffffffffff");
}

TEST(EthDataParser, GetTransactionInfoFromDataApprove) {
mojom::TransactionType tx_type;
std::vector<std::string> tx_params;
std::vector<std::string> tx_args;

std::vector<uint8_t> data;

// OK: well-formed ERC20Approve
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x095ea7b3"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f"
Expand All @@ -106,7 +116,6 @@ TEST(EthDataParser, GetTransactionInfoFromDataApprove) {
auto tx_info = GetTransactionInfoFromData(data);
ASSERT_NE(tx_info, absl::nullopt);
std::tie(tx_type, tx_params, tx_args) = *tx_info;

ASSERT_EQ(tx_type, mojom::TransactionType::ERC20Approve);
ASSERT_EQ(tx_params.size(), 2UL);
EXPECT_EQ(tx_params[0], "address");
Expand All @@ -115,7 +124,7 @@ TEST(EthDataParser, GetTransactionInfoFromDataApprove) {
EXPECT_EQ(tx_args[0], "0xbfb30a082f650c2a15d0632f0e87be4f8e64460f");
EXPECT_EQ(tx_args[1], "0x3fffffffffffffff");

// Function case doesn't matter
// OK: function case doesn't matter
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x095EA7b3"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f"
Expand All @@ -124,7 +133,6 @@ TEST(EthDataParser, GetTransactionInfoFromDataApprove) {
tx_info = GetTransactionInfoFromData(data);
ASSERT_NE(tx_info, absl::nullopt);
std::tie(tx_type, tx_params, tx_args) = *tx_info;

ASSERT_EQ(tx_type, mojom::TransactionType::ERC20Approve);
ASSERT_EQ(tx_params.size(), 2UL);
EXPECT_EQ(tx_params[0], "address");
Expand All @@ -133,33 +141,42 @@ TEST(EthDataParser, GetTransactionInfoFromDataApprove) {
EXPECT_EQ(tx_args[0], "0xbfb30a082f650c2a15d0632f0e87be4f8e64460f");
EXPECT_EQ(tx_args[1], "0x3fffffffffffffff");

// Missing a byte for the last param
// KO: missing a byte for the last param
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x095ea7b3"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f"
"0000000000000000000000000000000000000000000000003fffffffffffff",
&data));
EXPECT_FALSE(GetTransactionInfoFromData(data));

// Missing the entire last param
// KO: missing the entire last param
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x095ea7b3"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f",
&data));
EXPECT_FALSE(GetTransactionInfoFromData(data));

// No params
// KO: no params
ASSERT_TRUE(PrefixedHexStringToBytes("0x095ea7b3", &data));
EXPECT_FALSE(GetTransactionInfoFromData(data));

// Extra data
// OK: extra data
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x095ea7b3"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f"
"0000000000000000000000000000000000000000000000003fffffffffffffff"
"00",
&data));
EXPECT_FALSE(GetTransactionInfoFromData(data));
tx_info = GetTransactionInfoFromData(data);
ASSERT_NE(tx_info, absl::nullopt);
std::tie(tx_type, tx_params, tx_args) = *tx_info;
ASSERT_EQ(tx_type, mojom::TransactionType::ERC20Approve);
ASSERT_EQ(tx_params.size(), 2UL);
EXPECT_EQ(tx_params[0], "address");
EXPECT_EQ(tx_params[1], "uint256");
ASSERT_EQ(tx_args.size(), 2UL);
EXPECT_EQ(tx_args[0], "0xbfb30a082f650c2a15d0632f0e87be4f8e64460f");
EXPECT_EQ(tx_args[1], "0x3fffffffffffffff");
}

TEST(EthDataParser, GetTransactionInfoFromDataETHSend) {
Expand Down Expand Up @@ -189,8 +206,9 @@ TEST(EthDataParser, GetTransactionInfoFromDataERC721TransferFrom) {
mojom::TransactionType tx_type;
std::vector<std::string> tx_params;
std::vector<std::string> tx_args;

std::vector<uint8_t> data;

// OK: well-formed ERC721TransferFrom
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x23b872dd"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f"
Expand All @@ -200,7 +218,6 @@ TEST(EthDataParser, GetTransactionInfoFromDataERC721TransferFrom) {
auto tx_info = GetTransactionInfoFromData(data);
ASSERT_NE(tx_info, absl::nullopt);
std::tie(tx_type, tx_params, tx_args) = *tx_info;

ASSERT_EQ(tx_type, mojom::TransactionType::ERC721TransferFrom);
ASSERT_EQ(tx_params.size(), 3UL);
EXPECT_EQ(tx_params[0], "address");
Expand All @@ -211,6 +228,7 @@ TEST(EthDataParser, GetTransactionInfoFromDataERC721TransferFrom) {
EXPECT_EQ(tx_args[1], "0xbfb30a082f650c2a15d0632f0e87be4f8e64460a");
EXPECT_EQ(tx_args[2], "0xf");

// OK: well-formed ERC721SafeTransferFrom
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x42842e0e"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f"
Expand All @@ -220,7 +238,6 @@ TEST(EthDataParser, GetTransactionInfoFromDataERC721TransferFrom) {
tx_info = GetTransactionInfoFromData(data);
ASSERT_NE(tx_info, absl::nullopt);
std::tie(tx_type, tx_params, tx_args) = *tx_info;

ASSERT_EQ(tx_type, mojom::TransactionType::ERC721SafeTransferFrom);
ASSERT_EQ(tx_params.size(), 3UL);
EXPECT_EQ(tx_params[0], "address");
Expand All @@ -231,7 +248,7 @@ TEST(EthDataParser, GetTransactionInfoFromDataERC721TransferFrom) {
EXPECT_EQ(tx_args[1], "0xbfb30a082f650c2a15d0632f0e87be4f8e64460a");
EXPECT_EQ(tx_args[2], "0xf");

// Missing a byte for the last param
// KO: missing a byte for the last param
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x23b872dd"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f"
Expand All @@ -240,27 +257,38 @@ TEST(EthDataParser, GetTransactionInfoFromDataERC721TransferFrom) {
&data));
EXPECT_FALSE(GetTransactionInfoFromData(data));

// Missing the entire last param
// KO: missing the entire last param
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x23b872dd"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460a",
&data));
EXPECT_FALSE(GetTransactionInfoFromData(data));

// No params
// KO: no params
ASSERT_TRUE(PrefixedHexStringToBytes("0x23b872dd", &data));
EXPECT_FALSE(GetTransactionInfoFromData(data));

// Extra data
// OK: extra data
ASSERT_TRUE(PrefixedHexStringToBytes(
"0x23b872dd"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f"
"000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460a"
"000000000000000000000000000000000000000000000000000000000000000f"
"00",
&data));
EXPECT_FALSE(GetTransactionInfoFromData(data));
tx_info = GetTransactionInfoFromData(data);
ASSERT_NE(tx_info, absl::nullopt);
std::tie(tx_type, tx_params, tx_args) = *tx_info;
ASSERT_EQ(tx_type, mojom::TransactionType::ERC721TransferFrom);
ASSERT_EQ(tx_params.size(), 3UL);
EXPECT_EQ(tx_params[0], "address");
EXPECT_EQ(tx_params[1], "address");
EXPECT_EQ(tx_params[2], "uint256");
ASSERT_EQ(tx_args.size(), 3UL);
EXPECT_EQ(tx_args[0], "0xbfb30a082f650c2a15d0632f0e87be4f8e64460f");
EXPECT_EQ(tx_args[1], "0xbfb30a082f650c2a15d0632f0e87be4f8e64460a");
EXPECT_EQ(tx_args[2], "0xf");
}

TEST(EthDataParser, GetTransactionInfoFromDataERC1155SafeTransferFrom) {
Expand Down
19 changes: 19 additions & 0 deletions components/brave_wallet/browser/eth_response_parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "base/strings/string_number_conversions.h"
#include "brave/components/brave_wallet/browser/brave_wallet_utils.h"
#include "brave/components/brave_wallet/browser/eth_abi_decoder.h"
#include "brave/components/brave_wallet/browser/json_rpc_response_parser.h"
#include "brave/components/brave_wallet/common/eth_address.h"
#include "brave/components/brave_wallet/common/hex_utils.h"
Expand Down Expand Up @@ -235,6 +236,24 @@ bool ParseEthCall(const std::string& json, std::string* result) {
return ParseSingleStringResult(json, result);
}

absl::optional<std::vector<std::string>> DecodeEthCallResponse(
const std::string& data,
const std::vector<std::string>& abi_types) {
std::vector<uint8_t> response_bytes;
if (!PrefixedHexStringToBytes(data, &response_bytes))
return absl::nullopt;

auto decoded = ABIDecode(abi_types, response_bytes);
if (decoded == absl::nullopt)
return absl::nullopt;

const auto& args = std::get<1>(*decoded);
if (args.size() != abi_types.size())
return absl::nullopt;

return args;
}

bool ParseEthEstimateGas(const std::string& json, std::string* result) {
return ParseSingleStringResult(json, result);
}
Expand Down
Loading