Fix nullptr dereference

If the callbacks structure is freed/reused before the last call to
callbacks->linked_data_callback there would be a use-after-free.

This patch fixes this by coping the callbacks structure to the thread
context so it can be used safely without imposing restrictions on the
lifetime of the callbacks structure on the user.

Change-Id: I20c6ef9f142f9163e0fa16adb042bd13e9e65e12

diff --git a/srcs/client/client-request.h b/srcs/client/client-request.h
index 66a20a24446b463575b518305e09b9d24f2e5ab4..37086cfda84e89debf6717f808d85b1106fc2728 100644 (file)
--- a/srcs/client/client-request.h
+++ b/srcs/client/client-request.h
@@ -218,25 +218,26 @@ public:
     }
 };
 
-template <typename A, typename B>
-void cb_worker(std::shared_ptr<GenericClientRequest> request, A &callbacks, B cred)
+template <typename Request>
+void cb_worker(std::shared_ptr<Request> request, const typename Request::Callbacks &callbacks)
 {
     static constexpr const char *REQUEST_KIND_PREFIX =
-         std::is_same_v<B, wauthn_pubkey_credential_attestation_s*> ? "MC: " : "GA: ";
+         std::is_same_v<typename Request::Callbacks, wauthn_mc_callbacks_s> ? "MC: " : "GA: ";
 
     int ret = try_catch([&]() -> int {
-        if (callbacks->qrcode_callback == nullptr)
+        typename Request::PubKeyCred *cred = nullptr;
+        if (callbacks.qrcode_callback == nullptr)
             LogDebug(REQUEST_KIND_PREFIX << "There is no qrcode_callback");
-        else{ //callbacks->qrcode_callback != nullptr
+        else{ //callbacks.qrcode_callback != nullptr
             std::string qr_code;
             if (request->Recv(qr_code).Failed())
                 LogError(REQUEST_KIND_PREFIX << "Error on receive qrcode");
             LogDebug(REQUEST_KIND_PREFIX << "Received qr_contents: " << qr_code);
-            callbacks->qrcode_callback(qr_code.c_str(), callbacks->user_data);
+            callbacks.qrcode_callback(qr_code.c_str(), callbacks.user_data);
         }
         request->Recv(&cred);
-        callbacks->response_callback(cred, wauthn_error_e(request->GetStatus()),
-                                     callbacks->user_data);
+        callbacks.response_callback(cred, wauthn_error_e(request->GetStatus()),
+                                    callbacks.user_data);
         if(request->Failed())
         {
             LogError(REQUEST_KIND_PREFIX << "Error on receive response: " << request->GetStatus());
@@ -248,14 +249,14 @@ void cb_worker(std::shared_ptr<GenericClientRequest> request, A &callbacks, B cr
         while (request->Recv(&linked_data).Incompleted())
         {
             LogLinkedDevice(REQUEST_KIND_PREFIX, "[linked_data_callback]", linked_data);
-            callbacks->linked_data_callback(linked_data, wauthn_error_e(request->GetStatus()),
-                                            callbacks->user_data);
+            callbacks.linked_data_callback(linked_data, wauthn_error_e(request->GetStatus()),
+                                           callbacks.user_data);
             LogDebug(REQUEST_KIND_PREFIX << "More update linked_data can be received. Waiting to recv()");
         }
         LogLinkedDevice(REQUEST_KIND_PREFIX, "[linked_data_callback]", linked_data);
         LogDebug(REQUEST_KIND_PREFIX << "There is no more updated linked_data");
-        callbacks->linked_data_callback(linked_data, wauthn_error_e(request->GetStatus()),
-                                        callbacks->user_data);
+        callbacks.linked_data_callback(linked_data, wauthn_error_e(request->GetStatus()),
+                                       callbacks.user_data);
         return WAUTHN_ERROR_NONE;
     });
     if (ret != WAUTHN_ERROR_NONE)
@@ -282,8 +283,9 @@ int wauthn_process(const wauthn_client_data_s *client_data,
             return request->GetStatus();
         LogDebug("Response: " << wauthn_error_to_string(request->GetStatus()));
 
-        typename T::PubKeyCred *cred = nullptr;
-        std::thread worker([request, callbacks, cred]{cb_worker(std::move(request), callbacks, cred);});
+        std::thread worker([request = std::move(request), callbacks = *callbacks] {
+            cb_worker(std::move(request), callbacks);
+        });
         worker.detach();
         return WAUTHN_ERROR_NONE;
     });