From e37f4bec41f64ff78e1807a151902fc2da6fd159 Mon Sep 17 00:00:00 2001 From: Oleg Isupov Date: Mon, 27 Feb 2023 19:21:02 +0400 Subject: [PATCH 1/2] Verify NFT content media type before pinning Resolves https://github.com/brave/brave-browser/issues/28775 --- .../brave_wallet_pin_service_factory.cc | 7 +- .../browser/brave_wallet_pin_service.cc | 145 ++++++++++- .../browser/brave_wallet_pin_service.h | 50 +++- .../brave_wallet_pin_service_unittest.cc | 229 +++++++++++++++++- .../brave_wallet/common/brave_wallet.mojom | 3 +- 5 files changed, 413 insertions(+), 21 deletions(-) diff --git a/browser/brave_wallet/brave_wallet_pin_service_factory.cc b/browser/brave_wallet/brave_wallet_pin_service_factory.cc index 3f05074de32c..790cd0bbf366 100644 --- a/browser/brave_wallet/brave_wallet_pin_service_factory.cc +++ b/browser/brave_wallet/brave_wallet_pin_service_factory.cc @@ -20,6 +20,7 @@ #include "chrome/browser/profiles/incognito_helpers.h" #include "components/keyed_service/content/browser_context_dependency_manager.h" #include "components/user_prefs/user_prefs.h" +#include "content/public/browser/storage_partition.h" namespace brave_wallet { @@ -86,7 +87,11 @@ KeyedService* BraveWalletPinServiceFactory::BuildServiceInstanceFor( user_prefs::UserPrefs::Get(context), JsonRpcServiceFactory::GetServiceForContext(context), ipfs::IpfsLocalPinServiceFactory::GetServiceForContext(context), - ipfs::IpfsServiceFactory::GetForContext(context)); + ipfs::IpfsServiceFactory::GetForContext(context), + std::make_unique( + user_prefs::UserPrefs::Get(context), + context->GetDefaultStoragePartition() + ->GetURLLoaderFactoryForBrowserProcess())); } content::BrowserContext* BraveWalletPinServiceFactory::GetBrowserContextToUse( diff --git a/components/brave_wallet/browser/brave_wallet_pin_service.cc b/components/brave_wallet/browser/brave_wallet_pin_service.cc index 50963a3b5eba..4b070dc6b548 100644 --- a/components/brave_wallet/browser/brave_wallet_pin_service.cc +++ b/components/brave_wallet/browser/brave_wallet_pin_service.cc @@ -20,6 +20,7 @@ #include "brave/components/ipfs/ipfs_constants.h" #include "brave/components/ipfs/ipfs_utils.h" #include "components/prefs/scoped_user_pref_update.h" +#include "services/network/public/cpp/resource_request.h" namespace brave_wallet { @@ -78,6 +79,8 @@ absl::optional StringToErrorCode( return mojom::WalletPinServiceErrorCode::ERR_NOT_PINNED; } else if (error == "ERR_PINNING_FAILED") { return mojom::WalletPinServiceErrorCode::ERR_PINNING_FAILED; + } else if (error == "ERR_MEDIA_TYPE_UNSUPPORTED") { + return mojom::WalletPinServiceErrorCode::ERR_MEDIA_TYPE_UNSUPPORTED; } return absl::nullopt; } @@ -149,11 +152,97 @@ std::string BraveWalletPinService::ErrorCodeToString( return "ERR_NOT_PINNED"; case mojom::WalletPinServiceErrorCode::ERR_PINNING_FAILED: return "ERR_PINNING_FAILED"; + case mojom::WalletPinServiceErrorCode::ERR_MEDIA_TYPE_UNSUPPORTED: + return "ERR_MEDIA_TYPE_UNSUPPORTED"; } NOTREACHED(); return ""; } +ContentTypeChecker::ContentTypeChecker( + PrefService* pref_service, + scoped_refptr url_loader_factory) + : pref_service_(pref_service), + url_loader_factory_(std::move(url_loader_factory)) {} + +ContentTypeChecker::ContentTypeChecker() = default; + +ContentTypeChecker::~ContentTypeChecker() = default; + +void ContentTypeChecker::CheckContentTypeSupported( + const std::string& ipfs_url, + base::OnceCallback)> callback) { + // Create a request with no data or cookies. + auto resource_request = std::make_unique(); + GURL translated_url; + if (!ipfs::TranslateIPFSURI(GURL(ipfs_url), &translated_url, + ipfs::GetDefaultNFTIPFSGateway(pref_service_), + false)) { + std::move(callback).Run(false); + return; + } + resource_request->url = translated_url; + resource_request->credentials_mode = network::mojom::CredentialsMode::kOmit; + resource_request->redirect_mode = ::network::mojom::RedirectMode::kFollow; + + auto annotation = + net::DefineNetworkTrafficAnnotation("brave_wallet_pin_service", R"( + semantics { + sender: "Brave wallet pin service" + description: + "This service is used to pin NFTs" + "which are added to the wallet." + trigger: + "Triggered by enable auto-pinning mode" + "from the Brave Wallet page or settings." + data: + "Options of the commands." + destination: WEBSITE + } + policy { + cookies_allowed: NO + setting: + "You can enable or disable this feature in brave://settings." + policy_exception_justification: + "Not implemented." + } + )"); + + auto url_loader = + network::SimpleURLLoader::Create(std::move(resource_request), annotation); + auto* url_loader_ptr = url_loader.get(); + auto it = loaders_in_progress_.insert(loaders_in_progress_.end(), + std::move(url_loader)); + + url_loader_ptr->SetTimeoutDuration(base::Seconds(60)); + url_loader_ptr->SetRetryOptions( + 5, network::SimpleURLLoader::RetryMode::RETRY_ON_5XX | + network::SimpleURLLoader::RetryMode::RETRY_ON_NETWORK_CHANGE); + url_loader_ptr->DownloadHeadersOnly( + url_loader_factory_.get(), + base::BindOnce(&ContentTypeChecker::OnHeadersFetched, + weak_ptr_factory_.GetWeakPtr(), std::move(it), + std::move(callback))); +} + +void ContentTypeChecker::OnHeadersFetched( + UrlLoaderList::iterator loader_it, + base::OnceCallback)> callback, + scoped_refptr headers) { + loaders_in_progress_.erase(loader_it); + if (!headers) { + std::move(callback).Run(absl::nullopt); + return; + } + std::string content_type_value; + headers->GetMimeType(&content_type_value); + if (base::StartsWith(content_type_value, "image/")) { + std::move(callback).Run(true); + } else { + std::move(callback).Run(false); + } +} + // static bool BraveWalletPinService::IsTokenSupportedForPinning( const mojom::BlockchainTokenPtr& token) { @@ -204,11 +293,13 @@ BraveWalletPinService::BraveWalletPinService( PrefService* prefs, JsonRpcService* service, ipfs::IpfsLocalPinService* local_pin_service, - IpfsService* ipfs_service) + IpfsService* ipfs_service, + std::unique_ptr content_type_checker) : prefs_(prefs), json_rpc_service_(service), local_pin_service_(local_pin_service), - ipfs_service_(ipfs_service) { + ipfs_service_(ipfs_service), + content_type_checker_(std::move(content_type_checker)) { ipfs_service_->AddObserver(this); } @@ -507,20 +598,56 @@ void BraveWalletPinService::OnTokenMetaDataReceived( std::vector cids; cids.push_back(ExtractCID(token_url).value()); - auto* image = parsed_result->FindStringKey("image"); - if (image) { - auto image_cid = ExtractCID(*image); + auto* image_url = parsed_result->FindStringKey("image"); + if (image_url) { + auto image_cid = ExtractCID(*image_url); if (image_cid) { - cids.push_back(image_cid.value()); + cids.push_back(*image_cid); + content_type_checker_->CheckContentTypeSupported( + *image_url, + base::BindOnce(&BraveWalletPinService::OnContentTypeChecked, + weak_ptr_factory_.GetWeakPtr(), std::move(service), + std::move(token), std::move(cids), + std::move(callback))); + return; } } + OnContentTypeChecked(std::move(service), std::move(token), std::move(cids), + std::move(callback), true); +} + +void BraveWalletPinService::OnContentTypeChecked( + absl::optional service, + mojom::BlockchainTokenPtr token, + std::vector cids, + AddPinCallback callback, + absl::optional result) { + if (!result.has_value()) { + SetTokenStatus(service, token, + mojom::TokenPinStatusCode::STATUS_PINNING_FAILED, nullptr); + std::move(callback).Run( + false, mojom::PinError::New( + mojom::WalletPinServiceErrorCode::ERR_FETCH_METADATA_FAILED, + "Failed to verify media type")); + return; + } + + if (!result.value()) { + SetTokenStatus(service, token, + mojom::TokenPinStatusCode::STATUS_PINNING_FAILED, nullptr); + std::move(callback).Run( + false, mojom::PinError::New( + mojom::WalletPinServiceErrorCode::ERR_MEDIA_TYPE_UNSUPPORTED, + "Media type not supported")); + return; + } auto path = GetTokenPrefPath(service, token); if (!path) { std::move(callback).Run( - false, - mojom::PinError::New(mojom::WalletPinServiceErrorCode::ERR_WRONG_TOKEN, - "Wrong token data")); + false, mojom::PinError::New( + mojom::WalletPinServiceErrorCode::ERR_WRONG_METADATA_FORMAT, + "Wrong token data")); return; } diff --git a/components/brave_wallet/browser/brave_wallet_pin_service.h b/components/brave_wallet/browser/brave_wallet_pin_service.h index 58e38e8a3107..a7d27ff4f0ff 100644 --- a/components/brave_wallet/browser/brave_wallet_pin_service.h +++ b/components/brave_wallet/browser/brave_wallet_pin_service.h @@ -6,6 +6,8 @@ #ifndef BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_BRAVE_WALLET_PIN_SERVICE_H_ #define BRAVE_COMPONENTS_BRAVE_WALLET_BROWSER_BRAVE_WALLET_PIN_SERVICE_H_ +#include +#include #include #include #include @@ -23,10 +25,42 @@ #include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/receiver_set.h" #include "mojo/public/cpp/bindings/remote_set.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" +#include "services/network/public/cpp/simple_url_loader.h" #include "third_party/abseil-cpp/absl/types/optional.h" namespace brave_wallet { +class ContentTypeChecker { + public: + ContentTypeChecker( + PrefService* pref_service, + scoped_refptr url_loader_factory); + virtual ~ContentTypeChecker(); + + virtual void CheckContentTypeSupported( + const std::string& cid, + base::OnceCallback)> callback); + + protected: + // For tests + ContentTypeChecker(); + + private: + using UrlLoaderList = std::list>; + + void OnHeadersFetched(UrlLoaderList::iterator iterator, + base::OnceCallback)> callback, + scoped_refptr headers); + + raw_ptr pref_service_; + scoped_refptr url_loader_factory_; + // List of requests are actively being sent. + UrlLoaderList loaders_in_progress_; + + base::WeakPtrFactory weak_ptr_factory_{this}; +}; + /** * At the moment only local pinning is supported so use absl::nullopt * for optional service argument. @@ -35,10 +69,12 @@ class BraveWalletPinService : public KeyedService, public brave_wallet::mojom::WalletPinService, public ipfs::IpfsServiceObserver { public: - BraveWalletPinService(PrefService* prefs, - JsonRpcService* service, - ipfs::IpfsLocalPinService* local_pin_service, - IpfsService* ipfs_service); + BraveWalletPinService( + PrefService* prefs, + JsonRpcService* service, + ipfs::IpfsLocalPinService* local_pin_service, + IpfsService* ipfs_service, + std::unique_ptr content_type_checker); ~BraveWalletPinService() override; virtual void Restore(); @@ -134,6 +170,11 @@ class BraveWalletPinService : public KeyedService, const std::string& result, mojom::ProviderError error, const std::string& error_message); + void OnContentTypeChecked(absl::optional service, + mojom::BlockchainTokenPtr token, + std::vector cids, + AddPinCallback callback, + absl::optional result); // ipfs::IpfsServiceObserver void OnIpfsLaunched(bool result, int64_t pid) override; @@ -149,6 +190,7 @@ class BraveWalletPinService : public KeyedService, raw_ptr json_rpc_service_ = nullptr; raw_ptr local_pin_service_ = nullptr; raw_ptr ipfs_service_ = nullptr; + std::unique_ptr content_type_checker_; base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc b/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc index 9998748535d5..cb9f2c623e47 100644 --- a/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc +++ b/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc @@ -41,8 +41,31 @@ const char kMonkey3Path[] = const char kMonkey1Url[] = "ipfs://QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq/2413"; +const char kMonkey2Url[] = + "ipfs://QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq/2555"; +const char kMonkey3Url[] = + "ipfs://QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq/2777"; + const char kMonkey1[] = - R"({"image":"ipfs://Qmcyc7tm9sZB9JnvLgejPTwdzjjNjDMiRWCUvaZAfp6cUg", + R"({"image":"ipfs://bafy1", + "attributes":[ + {"trait_type":"Mouth","value":"Bored Cigarette"}, + {"trait_type":"Fur","value":"Zombie"}, + {"trait_type":"Background","value":"Purple"}, + {"trait_type":"Eyes","value":"Closed"}, + {"trait_type":"Clothes","value":"Toga"}, + {"trait_type":"Hat","value":"Cowboy Hat"}]})"; +const char kMonkey2[] = + R"({"image":"ipfs://bafy2", + "attributes":[ + {"trait_type":"Mouth","value":"Bored Cigarette"}, + {"trait_type":"Fur","value":"Zombie"}, + {"trait_type":"Background","value":"Purple"}, + {"trait_type":"Eyes","value":"Closed"}, + {"trait_type":"Clothes","value":"Toga"}, + {"trait_type":"Hat","value":"Cowboy Hat"}]})"; +const char kMonkey3[] = + R"({"image":"ipfs://bafy3", "attributes":[ {"trait_type":"Mouth","value":"Bored Cigarette"}, {"trait_type":"Fur","value":"Zombie"}, @@ -100,6 +123,13 @@ class MockIpfsService : public IpfsService { MOCK_METHOD0(IsDaemonLaunched, bool()); }; +class MockContentTypeChecker : public ContentTypeChecker { + public: + MOCK_METHOD2(CheckContentTypeSupported, + void(const std::string&, + base::OnceCallback)>)); +}; + } // namespace class BraveWalletPinServiceTest : public testing::Test { @@ -112,9 +142,12 @@ class BraveWalletPinServiceTest : public testing::Test { void SetUp() override { auto* registry = pref_service_.registry(); registry->RegisterDictionaryPref(kPinnedNFTAssets); + auto content_type_checker = + std::make_unique>(); + content_type_checker_ = content_type_checker.get(); brave_wallet_pin_service_ = std::make_unique( GetPrefs(), GetJsonRpcService(), GetIpfsLocalPinService(), - GetIpfsService()); + GetIpfsService(), std::move(content_type_checker)); } PrefService* GetPrefs() { return &pref_service_; } @@ -131,9 +164,14 @@ class BraveWalletPinServiceTest : public testing::Test { return &ipfs_service_; } + testing::NiceMock* GetContentTypeChecker() { + return content_type_checker_; + } + testing::NiceMock ipfs_local_pin_service_; testing::NiceMock json_rpc_service_; testing::NiceMock ipfs_service_; + testing::NiceMock* content_type_checker_; std::unique_ptr brave_wallet_pin_service_; TestingPrefServiceSimple pref_service_; @@ -141,6 +179,14 @@ class BraveWalletPinServiceTest : public testing::Test { }; TEST_F(BraveWalletPinServiceTest, AddPin) { + { + ON_CALL(*GetContentTypeChecker(), CheckContentTypeSupported(_, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& cid, + base::OnceCallback)> callback) { + std::move(callback).Run(true); + })); + } { ON_CALL(*GetJsonRpcService(), GetERC721Metadata(_, _, _, _)) .WillByDefault(::testing::Invoke( @@ -161,8 +207,7 @@ TEST_F(BraveWalletPinServiceTest, AddPin) { EXPECT_EQ(kMonkey1Path, prefix); EXPECT_EQ("QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq", cids.at(0)); - EXPECT_EQ("Qmcyc7tm9sZB9JnvLgejPTwdzjjNjDMiRWCUvaZAfp6cUg", - cids.at(1)); + EXPECT_EQ("bafy1", cids.at(1)); std::move(callback).Run(true); })); @@ -188,7 +233,7 @@ TEST_F(BraveWalletPinServiceTest, AddPin) { base::Value::List expected_cids; expected_cids.Append("QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq"); - expected_cids.Append("Qmcyc7tm9sZB9JnvLgejPTwdzjjNjDMiRWCUvaZAfp6cUg"); + expected_cids.Append("bafy1"); EXPECT_EQ(BraveWalletPinService::StatusToString( mojom::TokenPinStatusCode::STATUS_PINNED), @@ -241,6 +286,178 @@ TEST_F(BraveWalletPinServiceTest, AddPin) { } } +TEST_F(BraveWalletPinServiceTest, AddPin_ContentVerification) { + { + ON_CALL(*GetContentTypeChecker(), CheckContentTypeSupported(_, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& cid, + base::OnceCallback)> callback) { + EXPECT_EQ(cid, "ipfs://bafy1"); + std::move(callback).Run(true); + })); + ON_CALL(*GetJsonRpcService(), GetERC721Metadata(_, _, _, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& contract_address, const std::string& token_id, + const std::string& chain_id, + MockJsonRpcService::GetERC721MetadataCallback callback) { + EXPECT_EQ("0x1", chain_id); + EXPECT_EQ("0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d", + contract_address); + EXPECT_EQ("0x1", token_id); + std::move(callback).Run(kMonkey1Url, kMonkey1, + mojom::ProviderError::kSuccess, ""); + })); + ON_CALL(*GetIpfsLocalPinService(), AddPins(_, _, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& prefix, const std::vector& cids, + ipfs::AddPinCallback callback) { + EXPECT_EQ(kMonkey1Path, prefix); + EXPECT_EQ("QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq", + cids.at(0)); + EXPECT_EQ("bafy1", cids.at(1)); + std::move(callback).Run(true); + })); + + auto scoped_override = OverrideWithTimeNow(base::Time::FromTimeT(123u)); + + mojom::BlockchainTokenPtr token = + BraveWalletPinService::TokenFromPrefPath(kMonkey1Path); + token->is_erc721 = true; + absl::optional call_status; + service()->AddPin( + std::move(token), absl::nullopt, + base::BindLambdaForTesting( + [&call_status](bool result, mojom::PinErrorPtr error) { + call_status = result; + EXPECT_FALSE(error); + })); + EXPECT_TRUE(call_status.value()); + + const base::Value::Dict* token_record = + GetPrefs() + ->GetDict(kPinnedNFTAssets) + .FindDictByDottedPath(kMonkey1Path); + + base::Value::List expected_cids; + expected_cids.Append("QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq"); + expected_cids.Append("bafy1"); + + EXPECT_EQ(BraveWalletPinService::StatusToString( + mojom::TokenPinStatusCode::STATUS_PINNED), + *(token_record->FindString("status"))); + EXPECT_EQ(nullptr, token_record->FindDict("error")); + EXPECT_EQ(expected_cids, *(token_record->FindList("cids"))); + EXPECT_EQ(base::Time::FromTimeT(123u), + base::ValueToTime(token_record->Find("validate_timestamp"))); + } + + { + ON_CALL(*GetContentTypeChecker(), CheckContentTypeSupported(_, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& cid, + base::OnceCallback)> callback) { + EXPECT_EQ(cid, "ipfs://bafy2"); + std::move(callback).Run(false); + })); + ON_CALL(*GetJsonRpcService(), GetERC721Metadata(_, _, _, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& contract_address, const std::string& token_id, + const std::string& chain_id, + MockJsonRpcService::GetERC721MetadataCallback callback) { + EXPECT_EQ("0x1", chain_id); + EXPECT_EQ("0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d", + contract_address); + EXPECT_EQ("0x1", token_id); + std::move(callback).Run(kMonkey2Url, kMonkey2, + mojom::ProviderError::kSuccess, ""); + })); + ON_CALL(*GetIpfsLocalPinService(), AddPins(_, _, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& prefix, const std::vector& cids, + ipfs::AddPinCallback callback) { + EXPECT_EQ(kMonkey1Path, prefix); + EXPECT_EQ("QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq", + cids.at(0)); + EXPECT_EQ("bafy2", cids.at(1)); + std::move(callback).Run(true); + })); + + auto scoped_override = OverrideWithTimeNow(base::Time::FromTimeT(123u)); + + mojom::BlockchainTokenPtr token = + BraveWalletPinService::TokenFromPrefPath(kMonkey1Path); + token->is_erc721 = true; + absl::optional call_status; + service()->AddPin( + std::move(token), absl::nullopt, + base::BindLambdaForTesting( + [&call_status](bool result, mojom::PinErrorPtr error) { + call_status = result; + EXPECT_TRUE(error); + })); + EXPECT_TRUE(call_status.value()); + + const base::Value::Dict* token_record = + GetPrefs() + ->GetDict(kPinnedNFTAssets) + .FindDictByDottedPath(kMonkey2Path); + EXPECT_EQ(nullptr, token_record); + } + + { + ON_CALL(*GetContentTypeChecker(), CheckContentTypeSupported(_, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& cid, + base::OnceCallback)> callback) { + EXPECT_EQ(cid, "ipfs://bafy3"); + std::move(callback).Run(absl::nullopt); + })); + ON_CALL(*GetJsonRpcService(), GetERC721Metadata(_, _, _, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& contract_address, const std::string& token_id, + const std::string& chain_id, + MockJsonRpcService::GetERC721MetadataCallback callback) { + EXPECT_EQ("0x1", chain_id); + EXPECT_EQ("0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d", + contract_address); + EXPECT_EQ("0x1", token_id); + std::move(callback).Run(kMonkey3Url, kMonkey3, + mojom::ProviderError::kSuccess, ""); + })); + ON_CALL(*GetIpfsLocalPinService(), AddPins(_, _, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& prefix, const std::vector& cids, + ipfs::AddPinCallback callback) { + EXPECT_EQ(kMonkey3Path, prefix); + EXPECT_EQ("QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq", + cids.at(0)); + EXPECT_EQ("bafy3", cids.at(1)); + std::move(callback).Run(true); + })); + + auto scoped_override = OverrideWithTimeNow(base::Time::FromTimeT(123u)); + + mojom::BlockchainTokenPtr token = + BraveWalletPinService::TokenFromPrefPath(kMonkey1Path); + token->is_erc721 = true; + absl::optional call_status; + service()->AddPin( + std::move(token), absl::nullopt, + base::BindLambdaForTesting( + [&call_status](bool result, mojom::PinErrorPtr error) { + call_status = result; + EXPECT_TRUE(error); + })); + EXPECT_TRUE(call_status.value()); + + const base::Value::Dict* token_record = + GetPrefs() + ->GetDict(kPinnedNFTAssets) + .FindDictByDottedPath(kMonkey2Path); + EXPECT_EQ(nullptr, token_record); + } +} + TEST_F(BraveWalletPinServiceTest, RemovePin) { { ScopedDictPrefUpdate update(GetPrefs(), kPinnedNFTAssets); @@ -251,7 +468,7 @@ TEST_F(BraveWalletPinServiceTest, RemovePin) { item.Set("validation_timestamp", "123"); base::Value::List cids; cids.Append("QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq"); - cids.Append("Qmcyc7tm9sZB9JnvLgejPTwdzjjNjDMiRWCUvaZAfp6cUg"); + cids.Append("bafy1"); update_dict.SetByDottedPath(kMonkey1Path, std::move(item)); } diff --git a/components/brave_wallet/common/brave_wallet.mojom b/components/brave_wallet/common/brave_wallet.mojom index 2011011153d2..7b4a44852206 100644 --- a/components/brave_wallet/common/brave_wallet.mojom +++ b/components/brave_wallet/common/brave_wallet.mojom @@ -222,7 +222,8 @@ enum WalletPinServiceErrorCode { ERR_WRONG_METADATA_FORMAT = 4, ERR_ALREADY_PINNED = 5, ERR_NOT_PINNED = 6, - ERR_PINNING_FAILED = 7 + ERR_PINNING_FAILED = 7, + ERR_MEDIA_TYPE_UNSUPPORTED = 8 }; enum TokenPinStatusCode { From 754c1ef1facafbd4ec8d97420469dd1f676243a8 Mon Sep 17 00:00:00 2001 From: oisupov Date: Sat, 4 Mar 2023 18:47:53 +0400 Subject: [PATCH 2/2] Allow pinnning only if metadata and image has ipfs format --- .../browser/brave_wallet_pin_service.cc | 48 +++++++++++-------- .../brave_wallet_pin_service_unittest.cc | 48 +++++++++++++++++++ 2 files changed, 77 insertions(+), 19 deletions(-) diff --git a/components/brave_wallet/browser/brave_wallet_pin_service.cc b/components/brave_wallet/browser/brave_wallet_pin_service.cc index 4b070dc6b548..46592fe80869 100644 --- a/components/brave_wallet/browser/brave_wallet_pin_service.cc +++ b/components/brave_wallet/browser/brave_wallet_pin_service.cc @@ -88,6 +88,10 @@ absl::optional StringToErrorCode( absl::optional ExtractCID(const std::string& ipfs_url) { GURL gurl = GURL(ipfs_url); + if (!gurl.is_valid()) { + return absl::nullopt; + } + if (!gurl.SchemeIs(ipfs::kIPFSScheme)) { return absl::nullopt; } @@ -571,8 +575,8 @@ void BraveWalletPinService::OnTokenMetaDataReceived( return; } - GURL token_gurl = GURL(token_url); - if (!token_gurl.SchemeIs(ipfs::kIPFSScheme)) { + auto metadata_cid = ExtractCID(token_url); + if (!metadata_cid) { auto pin_error = mojom::PinError::New( mojom::WalletPinServiceErrorCode::ERR_NON_IPFS_TOKEN_URL, "Metadata has non-ipfs url"); @@ -595,25 +599,29 @@ void BraveWalletPinService::OnTokenMetaDataReceived( return; } - std::vector cids; - - cids.push_back(ExtractCID(token_url).value()); auto* image_url = parsed_result->FindStringKey("image"); - if (image_url) { - auto image_cid = ExtractCID(*image_url); - if (image_cid) { - cids.push_back(*image_cid); - content_type_checker_->CheckContentTypeSupported( - *image_url, - base::BindOnce(&BraveWalletPinService::OnContentTypeChecked, - weak_ptr_factory_.GetWeakPtr(), std::move(service), - std::move(token), std::move(cids), - std::move(callback))); - return; - } + auto image_cid = + image_url != nullptr ? ExtractCID(*image_url) : absl::nullopt; + + if (!image_cid) { + auto pin_error = mojom::PinError::New( + mojom::WalletPinServiceErrorCode::ERR_NON_IPFS_TOKEN_URL, + "Can't find proper image field"); + SetTokenStatus(service, token, + mojom::TokenPinStatusCode::STATUS_PINNING_FAILED, pin_error); + std::move(callback).Run(false, std::move(pin_error)); + return; } - OnContentTypeChecked(std::move(service), std::move(token), std::move(cids), - std::move(callback), true); + + std::vector cids; + cids.push_back(metadata_cid.value()); + cids.push_back(image_cid.value()); + + content_type_checker_->CheckContentTypeSupported( + *image_url, + base::BindOnce(&BraveWalletPinService::OnContentTypeChecked, + weak_ptr_factory_.GetWeakPtr(), std::move(service), + std::move(token), std::move(cids), std::move(callback))); } void BraveWalletPinService::OnContentTypeChecked( @@ -644,6 +652,8 @@ void BraveWalletPinService::OnContentTypeChecked( auto path = GetTokenPrefPath(service, token); if (!path) { + SetTokenStatus(service, token, + mojom::TokenPinStatusCode::STATUS_PINNING_FAILED, nullptr); std::move(callback).Run( false, mojom::PinError::New( mojom::WalletPinServiceErrorCode::ERR_WRONG_METADATA_FORMAT, diff --git a/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc b/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc index cb9f2c623e47..96ec7cf02c0e 100644 --- a/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc +++ b/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc @@ -38,6 +38,8 @@ const char kMonkey2Path[] = "nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x2"; const char kMonkey3Path[] = "nft.nftstorage.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x2"; +const char kMonkey4Path[] = + "nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x3"; const char kMonkey1Url[] = "ipfs://QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq/2413"; @@ -45,6 +47,8 @@ const char kMonkey2Url[] = "ipfs://QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq/2555"; const char kMonkey3Url[] = "ipfs://QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq/2777"; +const char kMonkey4Url[] = + "ipfs://QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq/2888"; const char kMonkey1[] = R"({"image":"ipfs://bafy1", @@ -73,6 +77,15 @@ const char kMonkey3[] = {"trait_type":"Eyes","value":"Closed"}, {"trait_type":"Clothes","value":"Toga"}, {"trait_type":"Hat","value":"Cowboy Hat"}]})"; +const char kMonkey4[] = + R"({"image":"https://image.com", + "attributes":[ + {"trait_type":"Mouth","value":"Bored Cigarette"}, + {"trait_type":"Fur","value":"Zombie"}, + {"trait_type":"Background","value":"Purple"}, + {"trait_type":"Eyes","value":"Closed"}, + {"trait_type":"Clothes","value":"Toga"}, + {"trait_type":"Hat","value":"Cowboy Hat"}]})"; base::Time g_overridden_now; std::unique_ptr OverrideWithTimeNow( @@ -458,6 +471,41 @@ TEST_F(BraveWalletPinServiceTest, AddPin_ContentVerification) { } } +TEST_F(BraveWalletPinServiceTest, AddPin_NonIpfsImage) { + { + ON_CALL(*GetJsonRpcService(), GetERC721Metadata(_, _, _, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& contract_address, const std::string& token_id, + const std::string& chain_id, + MockJsonRpcService::GetERC721MetadataCallback callback) { + std::move(callback).Run(kMonkey4Url, kMonkey4, + mojom::ProviderError::kSuccess, ""); + })); + + mojom::BlockchainTokenPtr token = + BraveWalletPinService::TokenFromPrefPath(kMonkey4Path); + token->is_erc721 = true; + absl::optional call_status; + service()->AddPin( + std::move(token), absl::nullopt, + base::BindLambdaForTesting( + [&call_status](bool result, mojom::PinErrorPtr error) { + call_status = result; + EXPECT_TRUE(error); + })); + EXPECT_FALSE(call_status.value()); + + const base::Value::Dict* token_record = + GetPrefs() + ->GetDict(kPinnedNFTAssets) + .FindDictByDottedPath(kMonkey4Path); + + EXPECT_EQ(BraveWalletPinService::StatusToString( + mojom::TokenPinStatusCode::STATUS_PINNING_FAILED), + *(token_record->FindString("status"))); + } +} + TEST_F(BraveWalletPinServiceTest, RemovePin) { { ScopedDictPrefUpdate update(GetPrefs(), kPinnedNFTAssets);