From d5b12a9e9390f3516bd953b678a8199e0fd1c440 Mon Sep 17 00:00:00 2001 From: sbiscigl Date: Thu, 30 Jul 2026 16:45:37 -0400 Subject: [PATCH] fix a bug where destroying a creds provider during a operation can lead to a use after free --- .../source/auth/CrtCredentialsProvider.cpp | 46 +++++++++----- .../aws/auth/CrtCredentialsProviderTest.cpp | 61 +++++++++++++++++++ 2 files changed, 92 insertions(+), 15 deletions(-) diff --git a/src/aws-cpp-sdk-core/source/auth/CrtCredentialsProvider.cpp b/src/aws-cpp-sdk-core/source/auth/CrtCredentialsProvider.cpp index 540f9bad2a12..f58deb94c41a 100644 --- a/src/aws-cpp-sdk-core/source/auth/CrtCredentialsProvider.cpp +++ b/src/aws-cpp-sdk-core/source/auth/CrtCredentialsProvider.cpp @@ -4,16 +4,28 @@ */ #include #include +#include #include #include +#include +#include + using namespace Aws::Auth; using namespace Aws::Utils; using namespace Aws::Utils::Threading; namespace { const int FIVE_MINUTES_IN_MILLIS = 5 * 60 * 1000; -} +const char* CRT_CREDS_PROVIDER_TAG = "CrtCredentialsProvider"; + +struct RefreshState { + std::mutex mutex; + std::condition_variable condition; + bool complete{false}; + AWSCredentials credentials; +}; +} // namespace CrtCredentialsProvider::CrtCredentialsProvider( const std::function()>& credentialsProviderFactory, @@ -39,27 +51,31 @@ AWSCredentials CrtCredentialsProvider::GetAWSCredentials() { } void CrtCredentialsProvider::Reload() { - AWSCredentials credentials{}; - std::mutex refresh_mutex{}; - std::condition_variable refresh_condition; - bool refresh_complete{false}; - m_credentialsProvider->GetCredentials([&credentials, &refresh_mutex, &refresh_complete, &refresh_condition]( - const std::shared_ptr& crtCredentials, int errorCode) -> void { + auto state = Aws::MakeShared(CRT_CREDS_PROVIDER_TAG); + + m_credentialsProvider->GetCredentials([state](const std::shared_ptr& crtCredentials, int errorCode) -> void { + (void)errorCode; { - const std::unique_lock lock(refresh_mutex); - (void)errorCode; - credentials = ExtractCredentialsFromCrt(*crtCredentials); - refresh_complete = true; + const std::unique_lock lock(state->mutex); + if (crtCredentials) { + state->credentials = ExtractCredentialsFromCrt(*crtCredentials); + } + state->complete = true; + state->condition.notify_all(); } - refresh_condition.notify_all(); }); - std::unique_lock lock(refresh_mutex); - refresh_condition.wait_for(lock, m_providerFuturesTimeoutMs, [&refresh_complete]() -> bool { return refresh_complete; }); + AWSCredentials credentials{}; + { + std::unique_lock lock(state->mutex); + state->condition.wait_for(lock, m_providerFuturesTimeoutMs, [&state]() -> bool { return state->complete; }); + credentials = state->credentials; + } + if (!credentials.IsEmpty()) { credentials.AddUserAgentFeature(m_userAgentFeature); } - m_credentials = credentials; + m_credentials = std::move(credentials); } void CrtCredentialsProvider::RefreshIfExpired() { diff --git a/tests/aws-cpp-sdk-core-tests/aws/auth/CrtCredentialsProviderTest.cpp b/tests/aws-cpp-sdk-core-tests/aws/auth/CrtCredentialsProviderTest.cpp index 445b15d2b50c..265a9f16c5cf 100644 --- a/tests/aws-cpp-sdk-core-tests/aws/auth/CrtCredentialsProviderTest.cpp +++ b/tests/aws-cpp-sdk-core-tests/aws/auth/CrtCredentialsProviderTest.cpp @@ -7,6 +7,10 @@ #include #include +#include +#include +#include + namespace { const char* CRT_CREDS_TEST_LOG = "CrtCredentialsProviderTest"; } @@ -56,8 +60,65 @@ class MockedCredsProvider : public Aws::Auth::CrtCredentialsProvider { std::shared_ptr m_provider; }; +class AsyncMockCrtCredentialsProvider : public Aws::Crt::Auth::ICredentialsProvider { + public: + explicit AsyncMockCrtCredentialsProvider(std::chrono::milliseconds callbackDelay, + std::shared_ptr credentials) + : m_callbackDelay{callbackDelay}, m_credentials{std::move(credentials)} {} + + ~AsyncMockCrtCredentialsProvider() override { + if (m_worker.joinable()) { + m_worker.join(); + } + } + + bool GetCredentials(const Aws::Crt::Auth::OnCredentialsResolved& onCredentialsResolved) const override { + const std::chrono::milliseconds delay = m_callbackDelay; + const std::shared_ptr creds = m_credentials; + m_worker = std::thread([delay, creds, onCredentialsResolved]() { + std::this_thread::sleep_for(delay); + onCredentialsResolved(creds, AWS_OP_SUCCESS); + }); + return true; + } + + aws_credentials_provider* GetUnderlyingHandle() const noexcept override { return nullptr; } + bool IsValid() const noexcept override { return true; } + + private: + std::chrono::milliseconds m_callbackDelay; + std::shared_ptr m_credentials; + mutable std::thread m_worker; +}; + +class AsyncMockedCredsProvider : public Aws::Auth::CrtCredentialsProvider { + public: + AsyncMockedCredsProvider(std::shared_ptr provider, std::chrono::milliseconds timeout) + : Aws::Auth::CrtCredentialsProvider([provider]() -> std::shared_ptr { return provider; }, + timeout, Aws::Client::UserAgentFeature::CREDENTIALS_LOGIN, "AsyncMockedCredsProvider"), + m_provider{provider} {} + + private: + std::shared_ptr m_provider; +}; + class CrtCredentialsProviderTest : public Aws::Testing::AwsCppSdkGTestSuite {}; +TEST_F(CrtCredentialsProviderTest, ShouldNotUseFreedStateWhenRefreshOutlivesTimeout) { + auto crtCreds = Aws::MakeShared( + CRT_CREDS_TEST_LOG, Aws::Crt::ByteCursorFromCString("access"), Aws::Crt::ByteCursorFromCString("secret"), + Aws::Crt::ByteCursorFromCString("token"), static_cast((Aws::Utils::DateTime::Now() + std::chrono::minutes(100)).Seconds())); + + auto underlying_mock = Aws::MakeShared(CRT_CREDS_TEST_LOG, std::chrono::milliseconds(300), crtCreds); + + { + AsyncMockedCredsProvider provider(underlying_mock, std::chrono::milliseconds(50)); + EXPECT_TRUE(provider.GetAWSCredentials().IsExpiredOrEmpty()); + } + + std::this_thread::sleep_for(std::chrono::milliseconds(400)); +} + TEST_F(CrtCredentialsProviderTest, ShouldCache) { auto underlying_mock = Aws::MakeShared(CRT_CREDS_TEST_LOG); underlying_mock->AddCredentialForReturn(Aws::MakeShared(