fix a bug where destroying a creds provider during a operation can lead to a use after free

Author: sbiscigl

src/aws-cpp-sdk-core/source/auth/CrtCredentialsProvider.cpp

@@ -4,16 +4,28 @@
  */
 #include <aws/core/auth/CrtCredentialsProvider.h>
 #include <aws/core/client/UserAgent.h>
+#include <aws/core/utils/memory/AWSMemory.h>
 #include <aws/core/utils/threading/ReaderWriterLock.h>
 #include <aws/crt/auth/Credentials.h>
 
+#include <condition_variable>
+#include <mutex>
+
 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<std::shared_ptr<Aws::Crt::Auth::ICredentialsProvider>()>& 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<Crt::Auth::Credentials>& crtCredentials, int errorCode) -> void {
+  auto state = Aws::MakeShared<RefreshState>(CRT_CREDS_PROVIDER_TAG);
+
+  m_credentialsProvider->GetCredentials([state](const std::shared_ptr<Crt::Auth::Credentials>& crtCredentials, int errorCode) -> void {
+    (void)errorCode;
     {
-      const std::unique_lock<std::mutex> lock(refresh_mutex);
-      (void)errorCode;
-      credentials = ExtractCredentialsFromCrt(*crtCredentials);
-      refresh_complete = true;
+      const std::unique_lock<std::mutex> lock(state->mutex);
+      if (crtCredentials) {
+        state->credentials = ExtractCredentialsFromCrt(*crtCredentials);
+      }
+      state->complete = true;
+      state->condition.notify_all();
     }
-    refresh_condition.notify_all();
   });
 
-  std::unique_lock<std::mutex> lock(refresh_mutex);
-  refresh_condition.wait_for(lock, m_providerFuturesTimeoutMs, [&refresh_complete]() -> bool { return refresh_complete; });
+  AWSCredentials credentials{};
+  {
+    std::unique_lock<std::mutex> 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() {

tests/aws-cpp-sdk-core-tests/aws/auth/CrtCredentialsProviderTest.cpp

@@ -7,6 +7,11 @@
 #include <aws/crt/auth/Credentials.h>
 #include <aws/testing/AwsCppSdkGTestSuite.h>
 
+#include <atomic>
+#include <chrono>
+#include <memory>
+#include <thread>
+
 namespace {
 const char* CRT_CREDS_TEST_LOG = "CrtCredentialsProviderTest";
 }
@@ -56,8 +61,65 @@ class MockedCredsProvider : public Aws::Auth::CrtCredentialsProvider {
   std::shared_ptr<MockCrtCredentialsProvider> m_provider;
 };
 
+class AsyncMockCrtCredentialsProvider : public Aws::Crt::Auth::ICredentialsProvider {
+ public:
+  explicit AsyncMockCrtCredentialsProvider(std::chrono::milliseconds callbackDelay,
+                                           std::shared_ptr<Aws::Crt::Auth::Credentials> 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<Aws::Crt::Auth::Credentials> 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<Aws::Crt::Auth::Credentials> m_credentials;
+  mutable std::thread m_worker;
+};
+
+class AsyncMockedCredsProvider : public Aws::Auth::CrtCredentialsProvider {
+ public:
+  AsyncMockedCredsProvider(std::shared_ptr<AsyncMockCrtCredentialsProvider> provider, std::chrono::milliseconds timeout)
+      : Aws::Auth::CrtCredentialsProvider([provider]() -> std::shared_ptr<Aws::Crt::Auth::ICredentialsProvider> { return provider; },
+                                          timeout, Aws::Client::UserAgentFeature::CREDENTIALS_LOGIN, "AsyncMockedCredsProvider"),
+        m_provider{provider} {}
+
+ private:
+  std::shared_ptr<AsyncMockCrtCredentialsProvider> m_provider;
+};
+
 class CrtCredentialsProviderTest : public Aws::Testing::AwsCppSdkGTestSuite {};
 
+TEST_F(CrtCredentialsProviderTest, ShouldNotUseFreedStateWhenRefreshOutlivesTimeout) {
+  auto crtCreds = Aws::MakeShared<Aws::Crt::Auth::Credentials>(
+      CRT_CREDS_TEST_LOG, Aws::Crt::ByteCursorFromCString("access"), Aws::Crt::ByteCursorFromCString("secret"),
+      Aws::Crt::ByteCursorFromCString("token"), static_cast<uint64_t>((Aws::Utils::DateTime::Now() + std::chrono::minutes(100)).Seconds()));
+
+  auto underlying_mock = Aws::MakeShared<AsyncMockCrtCredentialsProvider>(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<MockCrtCredentialsProvider>(CRT_CREDS_TEST_LOG);
   underlying_mock->AddCredentialForReturn(Aws::MakeShared<Aws::Crt::Auth::Credentials>(