Skip to content

Commit

Permalink
Merge pull request #14162 from brave/pr14138_f/wallet/extra-calldata-…
Browse files Browse the repository at this point in the history 
…bytes_1.42.x

fix(wallet): correctly handle extra calldata bytes (uplift to 1.42.x)
  • Loading branch information
@kjozwiak
kjozwiak authored Jul 18, 2022
2 parents c528344 + 1035672 commit fdbb4c8
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 51 deletions.
18 changes: 8 additions & 10 deletions components/brave_wallet/browser/eth_abi_decoder.cc
View file
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
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
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
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

0 comments on commit fdbb4c8

Please sign in to comment.