diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md index 573f1822684..dfaf8826528 100644 --- a/Firestore/CHANGELOG.md +++ b/Firestore/CHANGELOG.md @@ -1,3 +1,6 @@ +# Unreleased +- [fixed] Fix crash while fetching Auth and App Check tokens. (#15281) + # 12.4.0 - [fixed] Implemented an internal workaround to fix [CVE-2025-0838](https://nvd.nist.gov/vuln/detail/CVE-2025-0838). (#15300) diff --git a/Firestore/core/src/credentials/firebase_app_check_credentials_provider_apple.h b/Firestore/core/src/credentials/firebase_app_check_credentials_provider_apple.h index a6fb58cec5b..eb8823b2a0f 100644 --- a/Firestore/core/src/credentials/firebase_app_check_credentials_provider_apple.h +++ b/Firestore/core/src/credentials/firebase_app_check_credentials_provider_apple.h @@ -38,7 +38,8 @@ namespace firestore { namespace credentials { class FirebaseAppCheckCredentialsProvider - : public CredentialsProvider { + : public CredentialsProvider, + public std::enable_shared_from_this { public: FirebaseAppCheckCredentialsProvider(FIRApp* app, id app_check); diff --git a/Firestore/core/src/credentials/firebase_app_check_credentials_provider_apple.mm b/Firestore/core/src/credentials/firebase_app_check_credentials_provider_apple.mm index 9afb38e0eae..5cc96a9ad2a 100644 --- a/Firestore/core/src/credentials/firebase_app_check_credentials_provider_apple.mm +++ b/Firestore/core/src/credentials/firebase_app_check_credentials_provider_apple.mm @@ -77,8 +77,11 @@ void FirebaseAppCheckCredentialsProvider::GetToken( TokenListener completion) { - std::weak_ptr weak_contents = contents_; - if (contents_->app_check) { + // Capture self via a shared_ptr to ensure that the credentials provider + // is not deallocated while the getToken request is outstanding. + std::shared_ptr self = + shared_from_this(); + if (self->contents_->app_check) { void (^get_token_callback)(id) = ^(id result) { if (result.error != nil) { @@ -90,13 +93,13 @@ // Retrieve a cached or generate a new FAC Token. If forcingRefresh == YES // always generates a new token and updates the cache. - [contents_->app_check getTokenForcingRefresh:force_refresh_ - completion:get_token_callback]; + [self->contents_->app_check getTokenForcingRefresh:self->force_refresh_ + completion:get_token_callback]; } else { // If there's no AppCheck provider, call back immediately with a nil token. completion(std::string{""}); } - force_refresh_ = false; + self->force_refresh_ = false; } void FirebaseAppCheckCredentialsProvider::SetCredentialChangeListener( diff --git a/Firestore/core/src/credentials/firebase_auth_credentials_provider_apple.h b/Firestore/core/src/credentials/firebase_auth_credentials_provider_apple.h index 0f6b80824e8..1bcb10a4b53 100644 --- a/Firestore/core/src/credentials/firebase_auth_credentials_provider_apple.h +++ b/Firestore/core/src/credentials/firebase_auth_credentials_provider_apple.h @@ -53,7 +53,8 @@ namespace credentials { * For non-Apple desktop build, this is right now just a stub. */ class FirebaseAuthCredentialsProvider - : public CredentialsProvider { + : public CredentialsProvider, + public std::enable_shared_from_this { public: // TODO(zxu123): Provide a ctor to accept the C++ Firebase Games App, which // deals all platforms. Right now, only works for FIRApp*. diff --git a/Firestore/core/src/credentials/firebase_auth_credentials_provider_apple.mm b/Firestore/core/src/credentials/firebase_auth_credentials_provider_apple.mm index 0760e524cb9..b5dd77910d1 100644 --- a/Firestore/core/src/credentials/firebase_auth_credentials_provider_apple.mm +++ b/Firestore/core/src/credentials/firebase_auth_credentials_provider_apple.mm @@ -81,26 +81,23 @@ // fail if there is a token change while the request is outstanding. int initial_token_counter = contents_->token_counter; - std::weak_ptr weak_contents = contents_; + // Capture self via a shared_ptr to ensure that the credentials provider + // is not deallocated while the getToken request is outstanding. + std::shared_ptr self = shared_from_this(); void (^get_token_callback)(NSString*, NSError*) = ^(NSString* _Nullable token, NSError* _Nullable error) { - std::shared_ptr contents = weak_contents.lock(); - if (!contents) { - return; - } - - std::unique_lock lock(contents->mutex); - if (initial_token_counter != contents->token_counter) { + std::unique_lock lock(self->contents_->mutex); + if (initial_token_counter != self->contents_->token_counter) { // Cancel the request since the user changed while the request was // outstanding so the response is likely for a previous user (which // user, we can't be sure). LOG_DEBUG("GetToken aborted due to token change."); - return GetToken(completion); + return self->GetToken(completion); } else { if (error == nil) { if (token != nil) { - completion( - AuthToken{util::MakeString(token), contents->current_user}); + completion(AuthToken{util::MakeString(token), + self->contents_->current_user}); } else { completion(AuthToken::Unauthenticated()); } diff --git a/Firestore/core/test/unit/credentials/firebase_app_check_credentials_provider_test.mm b/Firestore/core/test/unit/credentials/firebase_app_check_credentials_provider_test.mm index f743ab69d6c..826fa310592 100644 --- a/Firestore/core/test/unit/credentials/firebase_app_check_credentials_provider_test.mm +++ b/Firestore/core/test/unit/credentials/firebase_app_check_credentials_provider_test.mm @@ -97,8 +97,9 @@ - (nonnull NSString*)tokenDidChangeNotificationName { auto token_promise = std::make_shared>(); FIRApp* app = testutil::AppForUnitTesting(); - FirebaseAppCheckCredentialsProvider credentials_provider(app, nil); - credentials_provider.GetToken( + auto credentials_provider = + std::make_shared(app, nil); + credentials_provider->GetToken( [token_promise](util::StatusOr result) { EXPECT_TRUE(result.ok()); const std::string& token = result.ValueOrDie(); @@ -115,8 +116,9 @@ - (nonnull NSString*)tokenDidChangeNotificationName { FIRApp* app = testutil::AppForUnitTesting(); FSTAppCheckFake* app_check = [[FSTAppCheckFake alloc] initWithToken:@"fake token"]; - FirebaseAppCheckCredentialsProvider credentials_provider(app, app_check); - credentials_provider.GetToken([](util::StatusOr result) { + auto credentials_provider = + std::make_shared(app, app_check); + credentials_provider->GetToken([](util::StatusOr result) { EXPECT_TRUE(result.ok()); const std::string& token = result.ValueOrDie(); EXPECT_EQ("fake token", token); @@ -127,20 +129,22 @@ - (nonnull NSString*)tokenDidChangeNotificationName { FIRApp* app = testutil::AppForUnitTesting(); FSTAppCheckFake* app_check = [[FSTAppCheckFake alloc] initWithToken:@"fake token"]; - FirebaseAppCheckCredentialsProvider credentials_provider(app, app_check); - credentials_provider.SetCredentialChangeListener( + auto credentials_provider = + std::make_shared(app, app_check); + credentials_provider->SetCredentialChangeListener( [](std::string token) { EXPECT_EQ("", token); }); - credentials_provider.SetCredentialChangeListener(nullptr); + credentials_provider->SetCredentialChangeListener(nullptr); } TEST(FirebaseAppCheckCredentialsProviderTest, InvalidateToken) { FIRApp* app = testutil::AppForUnitTesting(); FSTAppCheckFake* app_check = [[FSTAppCheckFake alloc] initWithToken:@"fake token"]; - FirebaseAppCheckCredentialsProvider credentials_provider(app, app_check); - credentials_provider.InvalidateToken(); - credentials_provider.GetToken( + auto credentials_provider = + std::make_shared(app, app_check); + credentials_provider->InvalidateToken(); + credentials_provider->GetToken( [&app_check](util::StatusOr result) { EXPECT_TRUE(result.ok()); EXPECT_TRUE(app_check.forceRefreshTriggered); @@ -154,9 +158,10 @@ - (nonnull NSString*)tokenDidChangeNotificationName { FIRApp* app = testutil::AppForUnitTesting(); FSTAppCheckFake* app_check = [[FSTAppCheckFake alloc] initWithToken:@""]; - FirebaseAppCheckCredentialsProvider credentials_provider(app, app_check); + auto credentials_provider = + std::make_shared(app, app_check); - credentials_provider.SetCredentialChangeListener( + credentials_provider->SetCredentialChangeListener( [token_promise](const std::string& result) { if (result != "") { token_promise->set_value(result); @@ -186,9 +191,10 @@ - (nonnull NSString*)tokenDidChangeNotificationName { FIRApp* app = testutil::AppForUnitTesting(); FSTAppCheckFake* app_check = [[FSTAppCheckFake alloc] initWithToken:@""]; - FirebaseAppCheckCredentialsProvider credentials_provider(app, app_check); + auto credentials_provider = + std::make_shared(app, app_check); - credentials_provider.SetCredentialChangeListener( + credentials_provider->SetCredentialChangeListener( [token_promise](const std::string& result) { if (result != "") { token_promise->set_value(result); @@ -227,9 +233,10 @@ - (nonnull NSString*)tokenDidChangeNotificationName { FIRApp* app = testutil::AppForUnitTesting(); FSTAppCheckFake* app_check = [[FSTAppCheckFake alloc] initWithToken:@""]; - FirebaseAppCheckCredentialsProvider credentials_provider(app, app_check); + auto credentials_provider = + std::make_shared(app, app_check); - credentials_provider.SetCredentialChangeListener( + credentials_provider->SetCredentialChangeListener( [token_promise](const std::string& result) { if (result != "") { token_promise->set_value(result); diff --git a/Firestore/core/test/unit/credentials/firebase_auth_credentials_provider_test.mm b/Firestore/core/test/unit/credentials/firebase_auth_credentials_provider_test.mm index 226c28dba81..c93d3712f28 100644 --- a/Firestore/core/test/unit/credentials/firebase_auth_credentials_provider_test.mm +++ b/Firestore/core/test/unit/credentials/firebase_auth_credentials_provider_test.mm @@ -75,8 +75,9 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh auto token_promise = std::make_shared>(); FIRApp* app = testutil::AppForUnitTesting(); - FirebaseAuthCredentialsProvider credentials_provider(app, nil); - credentials_provider.GetToken( + auto credentials_provider = + std::make_shared(app, nil); + credentials_provider->GetToken( [token_promise](util::StatusOr result) { EXPECT_TRUE(result.ok()); const AuthToken& token = result.ValueOrDie(); @@ -99,8 +100,9 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh TEST(FirebaseAuthCredentialsProviderTest, GetTokenUnauthenticated) { FIRApp* app = testutil::AppForUnitTesting(); FSTAuthFake* auth = [[FSTAuthFake alloc] initWithToken:nil uid:nil]; - FirebaseAuthCredentialsProvider credentials_provider(app, auth); - credentials_provider.GetToken([](util::StatusOr result) { + auto credentials_provider = + std::make_shared(app, auth); + credentials_provider->GetToken([](util::StatusOr result) { EXPECT_TRUE(result.ok()); const AuthToken& token = result.ValueOrDie(); EXPECT_ANY_THROW(token.token()); @@ -114,8 +116,9 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh FIRApp* app = testutil::AppForUnitTesting(); FSTAuthFake* auth = [[FSTAuthFake alloc] initWithToken:@"token for fake uid" uid:@"fake uid"]; - FirebaseAuthCredentialsProvider credentials_provider(app, auth); - credentials_provider.GetToken([](util::StatusOr result) { + auto credentials_provider = + std::make_shared(app, auth); + credentials_provider->GetToken([](util::StatusOr result) { EXPECT_TRUE(result.ok()); const AuthToken& token = result.ValueOrDie(); EXPECT_EQ("token for fake uid", token.token()); @@ -129,22 +132,24 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh FIRApp* app = testutil::AppForUnitTesting(); FSTAuthFake* auth = [[FSTAuthFake alloc] initWithToken:@"default token" uid:@"fake uid"]; - FirebaseAuthCredentialsProvider credentials_provider(app, auth); - credentials_provider.SetCredentialChangeListener([](User user) { + auto credentials_provider = + std::make_shared(app, auth); + credentials_provider->SetCredentialChangeListener([](User user) { EXPECT_EQ("fake uid", user.uid()); EXPECT_TRUE(user.is_authenticated()); }); - credentials_provider.SetCredentialChangeListener(nullptr); + credentials_provider->SetCredentialChangeListener(nullptr); } TEST(FirebaseAuthCredentialsProviderTest, InvalidateToken) { FIRApp* app = testutil::AppForUnitTesting(); FSTAuthFake* auth = [[FSTAuthFake alloc] initWithToken:@"token for fake uid" uid:@"fake uid"]; - FirebaseAuthCredentialsProvider credentials_provider(app, auth); - credentials_provider.InvalidateToken(); - credentials_provider.GetToken([&auth](util::StatusOr result) { + auto credentials_provider = + std::make_shared(app, auth); + credentials_provider->InvalidateToken(); + credentials_provider->GetToken([&auth](util::StatusOr result) { EXPECT_TRUE(result.ok()); EXPECT_TRUE(auth.forceRefreshTriggered); const AuthToken& token = result.ValueOrDie(); @@ -160,14 +165,15 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh FIRApp* app = testutil::AppForUnitTesting(); FSTAuthFake* auth = [[FSTAuthFake alloc] initWithToken:@"token for fake uid" uid:@"fake uid"]; - FirebaseAuthCredentialsProvider credentials_provider(app, auth); + auto credentials_provider = + std::make_shared(app, auth); - std::thread thread1([&credentials_provider] { - credentials_provider.GetToken( + std::thread thread1([credentials_provider] { + credentials_provider->GetToken( [](util::StatusOr result) { EXPECT_TRUE(result.ok()); }); }); - std::thread thread2([&credentials_provider] { - credentials_provider.GetToken( + std::thread thread2([credentials_provider] { + credentials_provider->GetToken( [](util::StatusOr result) { EXPECT_TRUE(result.ok()); }); });