From ab40c25cd8f597bf0ccf2bf7685bb5ed31ea43d1 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Sat, 29 Nov 2025 08:05:18 -0800 Subject: [PATCH 1/5] [Firestore] Fix crash fetching Auth and App Check tokens --- Firestore/CHANGELOG.md | 3 +++ ...se_app_check_credentials_provider_apple.mm | 13 ++++++++----- ...irebase_auth_credentials_provider_apple.mm | 19 ++++++++----------- 3 files changed, 19 insertions(+), 16 deletions(-) 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.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.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()); } From a9193c5ef8a613d83a511a18e3e5b082354678ea Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Sat, 29 Nov 2025 08:42:43 -0800 Subject: [PATCH 2/5] build fix --- .../firebase_app_check_credentials_provider_apple.h | 3 ++- .../src/credentials/firebase_auth_credentials_provider_apple.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) 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_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*. From 5bf8d1264510c6dfb66104cfa902a58358ae6240 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Sat, 29 Nov 2025 19:11:56 -0800 Subject: [PATCH 3/5] update tests --- ...ase_app_check_credentials_provider_test.mm | 41 ++++++++++-------- ...firebase_auth_credentials_provider_test.mm | 43 +++++++++++-------- 2 files changed, 49 insertions(+), 35 deletions(-) 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..85d466f8621 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); @@ -261,4 +268,4 @@ - (nonnull NSString*)tokenDidChangeNotificationName { } // namespace credentials } // namespace firestore -} // namespace firebase +} // namespace firebase \ No newline at end of file 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..820cdd1da69 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 @@ -9,6 +9,7 @@ * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. @@ -75,8 +76,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 +101,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 +117,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 +133,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 +166,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()); }); }); @@ -177,4 +184,4 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh } // namespace credentials } // namespace firestore -} // namespace firebase +} // namespace firebase \ No newline at end of file From feb0af2826c00a5dfcf932b86d043766411615a2 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Mon, 1 Dec 2025 06:16:23 -0800 Subject: [PATCH 4/5] Restore deleted newlines --- .../credentials/firebase_app_check_credentials_provider_test.mm | 2 +- .../unit/credentials/firebase_auth_credentials_provider_test.mm | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 85d466f8621..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 @@ -268,4 +268,4 @@ - (nonnull NSString*)tokenDidChangeNotificationName { } // namespace credentials } // namespace firestore -} // namespace firebase \ No newline at end of file +} // namespace firebase 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 820cdd1da69..1500871e257 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 @@ -184,4 +184,4 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh } // namespace credentials } // namespace firestore -} // namespace firebase \ No newline at end of file +} // namespace firebase From 272cefe4811079268af462e5a11bd7e78a7131c5 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Thu, 4 Dec 2025 15:53:57 -0800 Subject: [PATCH 5/5] Remove inadvertent newline --- .../unit/credentials/firebase_auth_credentials_provider_test.mm | 1 - 1 file changed, 1 deletion(-) 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 1500871e257..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 @@ -9,7 +9,6 @@ * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License.