YQ-1174 Fix heap use after free in http gateway

Fix heap use after free in http gateway

ref:aa593826388c79d2fa192761fffb08bb63d45f9b

2 files changed, 66 insertions, 39 deletions

diff --git a/ydb/library/yql/providers/common/http_gateway/yql_http_gateway.cpp b/ydb/library/yql/providers/common/http_gateway/yql_http_gateway.cpp
index 8895c05587..0b60b1e515 100644
--- a/ydb/library/yql/providers/common/http_gateway/yql_http_gateway.cpp
+++ b/ydb/library/yql/providers/common/http_gateway/yql_http_gateway.cpp
@@ -4,6 +4,7 @@
 #include <util/stream/str.h>
 #include <util/string/builder.h>
 #include <util/generic/size_literals.h>
+#include <util/generic/yexception.h>
 
 #include <thread>
 #include <mutex>
@@ -275,68 +276,84 @@ public:
         }
 
         TaskScheduler.Start();
+
+        InitCurl();
     }
 
     ~THTTPMultiGateway() {
-        if (Handle)
-            curl_multi_wakeup(Handle);
-        Thread.join();
+        curl_multi_wakeup(Handle);
+        if (Thread.joinable()) {
+            Thread.join();
+        }
+        UninitCurl();
     }
+
 private:
     size_t MaxHandlers = 1024U;
     size_t MaxSimulatenousDownloadsSize = 8_GB;
     size_t BuffersSizePerStream = CURL_MAX_WRITE_SIZE << 1U;
 
-    static void Perform(const TWeakPtr& weak) {
+    void InitCurl() {
+        const CURLcode globalInitResult = curl_global_init(CURL_GLOBAL_ALL);
+        if (globalInitResult == CURLE_OK) {
+            Handle = curl_multi_init();
+            if (!Handle) {
+                Cerr << "curl_multi_init error" << Endl;
+            }
+        } else {
+            Cerr << "curl_global_init error " << int(globalInitResult) << ": " << curl_easy_strerror(globalInitResult) << Endl;
+        }
+    }
+
+    void UninitCurl() {
+        if (Handle) {
+            const CURLMcode multiCleanupResult = curl_multi_cleanup(Handle);
+            if (multiCleanupResult != CURLM_OK) {
+                Cerr << "curl_multi_cleanup error " << int(multiCleanupResult) << ": " << curl_multi_strerror(multiCleanupResult) << Endl;
+            }
+        }
+        curl_global_cleanup();
+    }
+
+    static void Perform(const TWeakPtr& weak, CURLM* handle) {
         OutputSize.store(0ULL);
-        curl_global_init(CURL_GLOBAL_ALL);
 
-        if (const auto handle = curl_multi_init()) {
+        for (size_t handlers = 0U;;) {
             if (const auto& self = weak.lock()) {
-                self->Handle = handle;
+                handlers = self->FillHandlers();
+                self->PerformCycles->Inc();
+                self->OutputMemory->Set(OutputSize);
+            } else {
+                break;
             }
 
-            for (size_t handlers = 0U;;) {
+            int running = 0;
+            if (const auto c = curl_multi_perform(handle, &running); CURLM_OK != c) {
                 if (const auto& self = weak.lock()) {
-                    handlers = self->FillHandlers();
-                    self->PerformCycles->Inc();
-                    self->OutputMemory->Set(OutputSize);
-                } else {
-                    break;
-                }
-
-                int running = 0;
-                if (const auto c = curl_multi_perform(handle, &running); CURLM_OK != c) {
-                    if (const auto& self = weak.lock()) {
-                        self->Fail(c);
-                    }
-                    break;
+                    self->Fail(c);
                 }
+                break;
+            }
 
-                if (running < int(handlers)) {
-                    if (const auto& self = weak.lock()) {
-                        for (int messages = int(handlers) - running; messages;) {
-                            if (const auto msg = curl_multi_info_read(handle, &messages)) {
-                                if(msg->msg == CURLMSG_DONE) {
-                                    self->Done(msg->easy_handle, msg->data.result);
-                                }
+            if (running < int(handlers)) {
+                if (const auto& self = weak.lock()) {
+                    for (int messages = int(handlers) - running; messages;) {
+                        if (const auto msg = curl_multi_info_read(handle, &messages)) {
+                            if(msg->msg == CURLMSG_DONE) {
+                                self->Done(msg->easy_handle, msg->data.result);
                             }
                         }
                     }
-                } else {
-                    if (const auto c = curl_multi_poll(handle, nullptr, 0, 1024, nullptr); CURLM_OK != c) {
-                        if (const auto& self = weak.lock()) {
-                            self->Fail(c);
-                        }
-                        break;
+                }
+            } else {
+                if (const auto c = curl_multi_poll(handle, nullptr, 0, 1024, nullptr); CURLM_OK != c) {
+                    if (const auto& self = weak.lock()) {
+                        self->Fail(c);
                     }
+                    break;
                 }
             }
-
-            curl_multi_cleanup(handle);
         }
-
-        curl_global_cleanup();
     }
 
     size_t FillHandlers() {
@@ -505,6 +522,10 @@ private:
         THTTPMultiGateway::TWeakPtr Gateway;
     };
 
+    CURLM* GetHandle() const {
+        return Handle;
+    }
+
 private:
     CURLM* Handle = nullptr;
 
@@ -611,7 +632,11 @@ IHTTPGateway::Make(const THttpGatewayConfig* httpGatewaysCfg, NMonitoring::TDyna
 
     const auto gateway = std::make_shared<THTTPMultiGateway>(httpGatewaysCfg, std::move(counters));
     THTTPMultiGateway::Singleton = gateway;
-    gateway->Thread = std::thread(std::bind(&THTTPMultiGateway::Perform, THTTPMultiGateway::Singleton));
+    if (gateway->GetHandle()) {
+        gateway->Thread = std::thread(std::bind(&THTTPMultiGateway::Perform, THTTPMultiGateway::Singleton, gateway->GetHandle()));
+    } else {
+        ythrow yexception() << "Failed to initialize http gateway";
+    }
     return gateway;
 }
 
diff --git a/ydb/library/yql/providers/common/http_gateway/yql_http_gateway.h b/ydb/library/yql/providers/common/http_gateway/yql_http_gateway.h
index d5979bcce1..15e554a7d8 100644
--- a/ydb/library/yql/providers/common/http_gateway/yql_http_gateway.h
+++ b/ydb/library/yql/providers/common/http_gateway/yql_http_gateway.h
@@ -21,6 +21,8 @@ public:
 
     virtual ~IHTTPGateway() = default;
 
+    // Makes http gateways.
+    // Throws on error.
     static TPtr Make(
         const THttpGatewayConfig* httpGatewaysCfg = nullptr,
         NMonitoring::TDynamicCounterPtr counters = MakeIntrusive<NMonitoring::TDynamicCounters>());