diff --git a/components/brave_wallet/browser/eth_abi_decoder.cc b/components/brave_wallet/browser/eth_abi_decoder.cc index 3b1df890494d..95013d1da7b0 100644 --- a/components/brave_wallet/browser/eth_abi_decoder.cc +++ b/components/brave_wallet/browser/eth_abi_decoder.cc @@ -242,14 +242,14 @@ absl::optional> UniswapEncodedPathDecode( return path; } -absl::optional, // tx_params - std::vector>> // tx_args +absl::optional, // params + std::vector>> // args ABIDecode(const std::vector& types, const std::vector& data) { size_t offset = 0; size_t calldata_tail = 0; - std::vector tx_params; - std::vector tx_args; + std::vector params; + std::vector args; for (const auto& type : types) { absl::optional value; @@ -285,15 +285,13 @@ ABIDecode(const std::vector& 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 diff --git a/components/brave_wallet/browser/eth_abi_decoder_unittest.cc b/components/brave_wallet/browser/eth_abi_decoder_unittest.cc index 5f9740d97313..1086ea3e8730 100644 --- a/components/brave_wallet/browser/eth_abi_decoder_unittest.cc +++ b/components/brave_wallet/browser/eth_abi_decoder_unittest.cc @@ -39,68 +39,78 @@ TEST(EthABIDecoderTest, ABIDecodeAddress) { TEST(EthABIDecoderTest, ABIDecodeUint256) { std::vector tx_params; std::vector tx_args; - std::vector 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 tx_params; std::vector tx_args; - std::vector 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) { diff --git a/components/brave_wallet/browser/eth_data_parser_unittest.cc b/components/brave_wallet/browser/eth_data_parser_unittest.cc index 8909c291488c..b46d0b25a16c 100644 --- a/components/brave_wallet/browser/eth_data_parser_unittest.cc +++ b/components/brave_wallet/browser/eth_data_parser_unittest.cc @@ -44,8 +44,9 @@ TEST(EthDataParser, GetTransactionInfoFromDataTransfer) { mojom::TransactionType tx_type; std::vector tx_params; std::vector tx_args; - std::vector data; + + // OK: well-formed ERC20Transfer ASSERT_TRUE(PrefixedHexStringToBytes( "0xa9059cbb" "000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f" @@ -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"); @@ -63,7 +63,7 @@ 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" @@ -71,33 +71,43 @@ TEST(EthDataParser, GetTransactionInfoFromDataTransfer) { &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 tx_params; std::vector tx_args; - std::vector data; + + // OK: well-formed ERC20Approve ASSERT_TRUE(PrefixedHexStringToBytes( "0x095ea7b3" "000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f" @@ -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"); @@ -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" @@ -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"); @@ -133,7 +141,7 @@ 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" @@ -141,25 +149,34 @@ TEST(EthDataParser, GetTransactionInfoFromDataApprove) { &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) { @@ -189,8 +206,9 @@ TEST(EthDataParser, GetTransactionInfoFromDataERC721TransferFrom) { mojom::TransactionType tx_type; std::vector tx_params; std::vector tx_args; - std::vector data; + + // OK: well-formed ERC721TransferFrom ASSERT_TRUE(PrefixedHexStringToBytes( "0x23b872dd" "000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f" @@ -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"); @@ -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" @@ -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"); @@ -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" @@ -240,7 +257,7 @@ TEST(EthDataParser, GetTransactionInfoFromDataERC721TransferFrom) { &data)); EXPECT_FALSE(GetTransactionInfoFromData(data)); - // Missing the entire last param + // KO: missing the entire last param ASSERT_TRUE(PrefixedHexStringToBytes( "0x23b872dd" "000000000000000000000000BFb30a082f650C2A15D0632f0e87bE4F8e64460f" @@ -248,11 +265,11 @@ TEST(EthDataParser, GetTransactionInfoFromDataERC721TransferFrom) { &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" @@ -260,7 +277,18 @@ TEST(EthDataParser, GetTransactionInfoFromDataERC721TransferFrom) { "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) { diff --git a/components/brave_wallet/browser/eth_response_parser.cc b/components/brave_wallet/browser/eth_response_parser.cc index d92c443dfe07..f22984ec479f 100644 --- a/components/brave_wallet/browser/eth_response_parser.cc +++ b/components/brave_wallet/browser/eth_response_parser.cc @@ -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" @@ -235,6 +236,24 @@ bool ParseEthCall(const std::string& json, std::string* result) { return ParseSingleStringResult(json, result); } +absl::optional> DecodeEthCallResponse( + const std::string& data, + const std::vector& abi_types) { + std::vector 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); } diff --git a/components/brave_wallet/browser/eth_response_parser.h b/components/brave_wallet/browser/eth_response_parser.h index 161f97d688a2..066a5d070cc6 100644 --- a/components/brave_wallet/browser/eth_response_parser.h +++ b/components/brave_wallet/browser/eth_response_parser.h @@ -32,6 +32,9 @@ bool ParseEthGetTransactionReceipt(const std::string& json, TransactionReceipt* receipt); bool ParseEthSendRawTransaction(const std::string& json, std::string* tx_hash); bool ParseEthCall(const std::string& json, std::string* result); +absl::optional> DecodeEthCallResponse( + const std::string& data, + const std::vector& abi_types); bool ParseEthEstimateGas(const std::string& json, std::string* result); bool ParseEthGasPrice(const std::string& json, std::string* result); diff --git a/components/brave_wallet/browser/eth_response_parser_unittest.cc b/components/brave_wallet/browser/eth_response_parser_unittest.cc index d835f2743fd1..febf2dcb42a7 100644 --- a/components/brave_wallet/browser/eth_response_parser_unittest.cc +++ b/components/brave_wallet/browser/eth_response_parser_unittest.cc @@ -71,6 +71,32 @@ TEST(EthResponseParserUnitTest, ParseEthCall) { ASSERT_EQ(result, "0x0"); } +TEST(EthResponseParserUnitTest, DecodeEthCallResponse) { + // OK: 32-byte uint256 + std::string result = + "0x00000000000000000000000000000000000000000000000166e12cfce39a0000"; + auto args = DecodeEthCallResponse(result, {"uint256"}); + ASSERT_NE(args, absl::nullopt); + ASSERT_EQ(args->size(), 1UL); + ASSERT_EQ(args->at(0), "0x166e12cfce39a0000"); + + // OK: 32-byte uint256 with extra zero bytes + result = + "0x0000000000000000000000000000000000000000000000000000000000045d12000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "00000000000000000000000000000000000000000000000000"; + args = DecodeEthCallResponse(result, {"uint256"}); + ASSERT_NE(args, absl::nullopt); + ASSERT_EQ(args->size(), 1UL); + ASSERT_EQ(args->at(0), "0x45d12"); + + // KO: insufficient length of response + ASSERT_EQ(DecodeEthCallResponse("0x0", {"uint256"}), absl::nullopt); + + // KO: invalid response + ASSERT_EQ(DecodeEthCallResponse("foobarbaz", {"uint256"}), absl::nullopt); +} + TEST(EthResponseParserUnitTest, ParseEthGetTransactionReceipt) { std::string json( R"({ diff --git a/components/brave_wallet/browser/json_rpc_service.cc b/components/brave_wallet/browser/json_rpc_service.cc index 2276749f9297..99b883bb5e43 100644 --- a/components/brave_wallet/browser/json_rpc_service.cc +++ b/components/brave_wallet/browser/json_rpc_service.cc @@ -975,7 +975,16 @@ void JsonRpcService::OnGetERC20TokenBalance( std::move(callback).Run("", error, error_message); return; } - std::move(callback).Run(result, mojom::ProviderError::kSuccess, ""); + + const auto& args = eth::DecodeEthCallResponse(result, {"uint256"}); + if (args == absl::nullopt) { + std::move(callback).Run( + "", mojom::ProviderError::kInternalError, + l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR)); + return; + } + + std::move(callback).Run(args->at(0), mojom::ProviderError::kSuccess, ""); } void JsonRpcService::GetERC20TokenAllowance( @@ -1018,7 +1027,16 @@ void JsonRpcService::OnGetERC20TokenAllowance( std::move(callback).Run("", error, error_message); return; } - std::move(callback).Run(result, mojom::ProviderError::kSuccess, ""); + + const auto& args = eth::DecodeEthCallResponse(result, {"uint256"}); + if (args == absl::nullopt) { + std::move(callback).Run( + "", mojom::ProviderError::kInternalError, + l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR)); + return; + } + + std::move(callback).Run(args->at(0), mojom::ProviderError::kSuccess, ""); } void JsonRpcService::EnsRegistryGetResolver(const std::string& domain, diff --git a/components/brave_wallet/browser/json_rpc_service_unittest.cc b/components/brave_wallet/browser/json_rpc_service_unittest.cc index e156eeda5f72..e1b4aea1ab1d 100644 --- a/components/brave_wallet/browser/json_rpc_service_unittest.cc +++ b/components/brave_wallet/browser/json_rpc_service_unittest.cc @@ -1790,8 +1790,7 @@ TEST_F(JsonRpcServiceUnitTest, GetERC20TokenBalance) { "0x4e02f254184E904300e0775E4b8eeCB1", mojom::kMainnetChainId, base::BindOnce(&OnStringResponse, &callback_called, mojom::ProviderError::kSuccess, "", - "0x00000000000000000000000000000000000000000000000166e12cf" - "ce39a0000")); + "0x166e12cfce39a0000")); base::RunLoop().RunUntilIdle(); EXPECT_TRUE(callback_called); @@ -1866,8 +1865,7 @@ TEST_F(JsonRpcServiceUnitTest, GetERC20TokenAllowance) { "0xBFb30a082f650C2A15D0632f0e87bE4F8e64460a", base::BindOnce(&OnStringResponse, &callback_called, mojom::ProviderError::kSuccess, "", - "0x00000000000000000000000000000000000000000000000166e12cf" - "ce39a0000")); + "0x166e12cfce39a0000")); base::RunLoop().RunUntilIdle(); EXPECT_TRUE(callback_called);