Merge branch 'master' into fix/improve-aclrequest-detection

Author: MohabCodeX

Client/cefweb/CAjaxResourceHandler.cpp

@@ -18,6 +18,15 @@ CAjaxResourceHandler::CAjaxResourceHandler(std::vector<SString>& vecGet, std::ve
 {
 }
 
+CAjaxResourceHandler::~CAjaxResourceHandler()
+{
+    // Ensure callback is released if handler is destroyed before completion
+    if (m_callback)
+    {
+        m_callback = nullptr;
+    }
+}
+
 std::vector<SString>& CAjaxResourceHandler::GetGetData()
 {
     return m_vecGetData;
@@ -34,12 +43,18 @@ void CAjaxResourceHandler::SetResponse(const SString& data)
     m_bHasData = true;
 
     if (m_callback)
+    {
         m_callback->Continue();
+        // Release callback to prevent memory leak
+        m_callback = nullptr;
+    }
 }
 
 // CefResourceHandler implementation
 void CAjaxResourceHandler::Cancel()
 {
+    // Release callback reference on cancellation to prevent memory leak
+    m_callback = nullptr;
 }
 
 void CAjaxResourceHandler::GetResponseHeaders(CefRefPtr<CefResponse> response, int64& response_length, CefString& redirectUrl)

Client/cefweb/CAjaxResourceHandler.h

@@ -20,6 +20,7 @@ class CAjaxResourceHandler : public CefResourceHandler, public CAjaxResourceHand
 {
 public:
     CAjaxResourceHandler(std::vector<SString>& vecGet, std::vector<SString>& vecPost, const CefString& mimeType);
+    virtual ~CAjaxResourceHandler();
 
     virtual std::vector<SString>& GetGetData() override;
     virtual std::vector<SString>& GetPostData() override;

Client/cefweb/CWebCore.cpp

@@ -109,13 +109,22 @@ void CWebCore::DestroyWebView(CWebViewInterface* pWebViewInterface)
     CefRefPtr<CWebView> pWebView = dynamic_cast<CWebView*>(pWebViewInterface);
     if (pWebView)
     {
+        // Mark as being destroyed to prevent new events/tasks
+        pWebView->SetBeingDestroyed(true);
+        
         // Ensure that no attached events or tasks are in the queue
         RemoveWebViewEvents(pWebView.get());
         RemoveWebViewTasks(pWebView.get());
 
+        // Remove from list before closing to break reference cycles early
         m_WebViews.remove(pWebView);
-        // pWebView->Release(); // Do not release since other references get corrupted then
+        
+        // CloseBrowser will eventually trigger OnBeforeClose which clears m_pWebView
+        // This breaks the circular reference: CWebView -> CefBrowser -> CWebView
         pWebView->CloseBrowser();
+        
+        // Note: Do not call Release() - let CefRefPtr manage the lifecycle
+        // The circular reference is broken via OnBeforeClose setting m_pWebView = nullptr
     }
 }
 
@@ -164,6 +173,18 @@ void CWebCore::AddEventToEventQueue(std::function<void()> event, CWebView* pWebV
 
     std::lock_guard<std::mutex> lock(m_EventQueueMutex);
 
+    // Prevent unbounded queue growth - drop oldest events if queue is too large
+    if (m_EventQueue.size() >= MAX_EVENT_QUEUE_SIZE)
+    {
+        // Log warning even in release builds as this indicates a serious issue
+        g_pCore->GetConsole()->Printf("WARNING: Browser event queue size limit reached (%d), dropping oldest events", MAX_EVENT_QUEUE_SIZE);
+        
+        // Remove oldest 10% of events to make room
+        auto removeCount = MAX_EVENT_QUEUE_SIZE / 10;
+        for (size_t i = 0; i < removeCount && !m_EventQueue.empty(); ++i)
+            m_EventQueue.pop_front();
+    }
+
 #ifndef MTA_DEBUG
     m_EventQueue.push_back(EventEntry(event, pWebView));
 #else
@@ -215,6 +236,22 @@ void CWebCore::WaitForTask(std::function<void(bool)> task, CWebView* webView)
     std::future<void> result;
     {
         std::scoped_lock lock(m_TaskQueueMutex);
+        
+        // Prevent unbounded queue growth - if queue is too large, oldest tasks will be dropped during pulse
+        if (m_TaskQueue.size() >= MAX_TASK_QUEUE_SIZE)
+        {
+#ifdef MTA_DEBUG
+            g_pCore->GetConsole()->Printf("Warning: Task queue size limit reached (%d), task will be aborted", MAX_TASK_QUEUE_SIZE);
+#endif
+            // Must still queue the task to fulfill the future, but it will be aborted during processing
+            // Removing oldest task to make room
+            if (!m_TaskQueue.empty())
+            {
+                m_TaskQueue.front().task(true);  // Abort oldest task
+                m_TaskQueue.pop_front();
+            }
+        }
+        
         m_TaskQueue.emplace_back(TaskEntry{task, webView});
         result = m_TaskQueue.back().task.get_future();
     }
@@ -358,13 +395,39 @@ void CWebCore::AddAllowedPage(const SString& strURL, eWebFilterType filterType)
 {
     std::lock_guard<std::recursive_mutex> lock(m_FilterMutex);
 
+    // Prevent unbounded whitelist growth - remove old REQUEST entries if limit reached
+    if (m_Whitelist.size() >= MAX_WHITELIST_SIZE)
+    {
+        // Remove WEBFILTER_REQUEST entries (temporary session entries)
+        for (auto iter = m_Whitelist.begin(); iter != m_Whitelist.end();)
+        {
+            if (iter->second.second == eWebFilterType::WEBFILTER_REQUEST)
+                m_Whitelist.erase(iter++);
+            else
+                ++iter;
+        }
+    }
+
     m_Whitelist[strURL] = std::pair<bool, eWebFilterType>(true, filterType);
 }
 
 void CWebCore::AddBlockedPage(const SString& strURL, eWebFilterType filterType)
 {
     std::lock_guard<std::recursive_mutex> lock(m_FilterMutex);
 
+    // Prevent unbounded whitelist growth - remove old REQUEST entries if limit reached
+    if (m_Whitelist.size() >= MAX_WHITELIST_SIZE)
+    {
+        // Remove WEBFILTER_REQUEST entries (temporary session entries)
+        for (auto iter = m_Whitelist.begin(); iter != m_Whitelist.end();)
+        {
+            if (iter->second.second == eWebFilterType::WEBFILTER_REQUEST)
+                m_Whitelist.erase(iter++);
+            else
+                ++iter;
+        }
+    }
+
     m_Whitelist[strURL] = std::pair<bool, eWebFilterType>(false, filterType);
 }
 

Client/cefweb/CWebCore.h

@@ -20,6 +20,9 @@
 #define MTA_BROWSERDATA_PATH "mta/cef/browserdata.xml"
 #define BROWSER_LIST_UPDATE_INTERVAL (24*60*60)
 #define BROWSER_UPDATE_URL "https://cef.multitheftauto.com/get.php"
+#define MAX_EVENT_QUEUE_SIZE 10000
+#define MAX_TASK_QUEUE_SIZE 1000
+#define MAX_WHITELIST_SIZE 50000
 #define GetNextSibling(hwnd) GetWindow(hwnd, GW_HWNDNEXT) // Re-define the conflicting macro
 #define GetFirstChild(hwnd) GetTopWindow(hwnd)
 

Client/cefweb/CWebView.cpp

@@ -44,8 +44,15 @@ CWebView::~CWebView()
     // Make sure we don't dead lock the CEF render thread
     ResumeCefThread();
 
-    // Ensure that CefRefPtr::~CefRefPtr doesn't try to release it twice (it has already been released in CWebView::OnBeforeClose)
-    m_pWebView = nullptr;
+    // Clean up AJAX handlers to prevent accumulation
+    m_AjaxHandlers.clear();
+
+    // Break circular reference: ensure browser reference is cleared
+    // This is to prevent memory leaks from CWebView <-> CefBrowser cycles
+    if (m_pWebView)
+    {
+        m_pWebView = nullptr;
+    }
 
     OutputDebugLine("CWebView::~CWebView");
 }
@@ -83,6 +90,9 @@ void CWebView::CloseBrowser()
     // Make sure we don't dead lock the CEF render thread
     ResumeCefThread();
 
+    // Clear AJAX handlers early to prevent late event processing
+    m_AjaxHandlers.clear();
+
     if (m_pWebView)
         m_pWebView->GetHost()->CloseBrowser(true);
 }
@@ -500,8 +510,12 @@ bool CWebView::HasAjaxHandler(const SString& strURL)
 
 void CWebView::HandleAjaxRequest(const SString& strURL, CAjaxResourceHandler* pHandler)
 {
-    auto func = std::bind(&CWebBrowserEventsInterface::Events_OnAjaxRequest, m_pEventsInterface, pHandler, strURL);
-    g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "AjaxResourceRequest");
+    // Only queue event if not being destroyed to prevent UAF
+    if (!m_bBeingDestroyed)
+    {
+        auto func = std::bind(&CWebBrowserEventsInterface::Events_OnAjaxRequest, m_pEventsInterface, pHandler, strURL);
+        g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "AjaxResourceRequest");
+    }
 }
 
 bool CWebView::ToggleDevTools(bool visible)
@@ -719,6 +733,8 @@ void CWebView::OnPaint(CefRefPtr<CefBrowser> browser, CefRenderHandler::PaintEle
     m_RenderData.width = width;
     m_RenderData.height = height;
     m_RenderData.dirtyRects = dirtyRects;
+    // Prevent vector capacity growth memory leak - shrink excess capacity
+    m_RenderData.dirtyRects.shrink_to_fit();
     m_RenderData.changed = true;
 
     // Wait for the main thread to handle drawing the texture

Client/core/CAdditionalVertexStreamManager.cpp

@@ -12,6 +12,7 @@
 #include "StdInc.h"
 #include "CAdditionalVertexStreamManager.h"
 #include <limits>
+#include <mutex>
 
 CAdditionalVertexStreamManager* CAdditionalVertexStreamManager::ms_Singleton = nullptr;
 
@@ -75,6 +76,8 @@ namespace
         STriKey key = {a, b, c};
         return key;
     }
+
+    std::mutex g_singletonMutex;
 }            // namespace
 
 ///////////////////////////////////////////////////////////////
@@ -119,23 +122,26 @@ CAdditionalVertexStreamManager::~CAdditionalVertexStreamManager()
 ///////////////////////////////////////////////////////////////
 CAdditionalVertexStreamManager* CAdditionalVertexStreamManager::GetSingleton()
 {
+    std::lock_guard<std::mutex> guard(g_singletonMutex);
     if (!ms_Singleton)
         ms_Singleton = new CAdditionalVertexStreamManager();
     return ms_Singleton;
 }
 
-CAdditionalVertexStreamManager* CAdditionalVertexStreamManager::GetExistingSingleton() noexcept
+CAdditionalVertexStreamManager* CAdditionalVertexStreamManager::GetExistingSingleton()
 {
+    std::lock_guard<std::mutex> guard(g_singletonMutex);
     return ms_Singleton;
 }
 
 void CAdditionalVertexStreamManager::DestroySingleton()
 {
-    if (ms_Singleton)
-    {
-        delete ms_Singleton;
-        ms_Singleton = nullptr;
-    }
+    std::lock_guard<std::mutex> guard(g_singletonMutex);
+    if (!ms_Singleton)
+        return;
+
+    delete ms_Singleton;
+    ms_Singleton = nullptr;
 }
 
 ///////////////////////////////////////////////////////////////
@@ -259,7 +265,14 @@ bool CAdditionalVertexStreamManager::SetAdditionalVertexStream(SCurrentStateInfo
     if (FAILED(m_pDevice->GetVertexDeclaration(&pPreviousDecl)))
         return false;
 
-    if (FAILED(g_pProxyDevice->SetVertexDeclaration(pAdditionalInfo->pVertexDeclaration)))
+    CScopedActiveProxyDevice proxyDevice;
+    if (!proxyDevice)
+    {
+        SAFE_RELEASE(pPreviousDecl);
+        return false;
+    }
+
+    if (FAILED(proxyDevice->SetVertexDeclaration(pAdditionalInfo->pVertexDeclaration)))
     {
         SAFE_RELEASE(pPreviousDecl);
         return false;
@@ -268,7 +281,7 @@ bool CAdditionalVertexStreamManager::SetAdditionalVertexStream(SCurrentStateInfo
     uint OffsetInBytes = ConvertPTOffset(state.stream1.OffsetInBytes);
     if (FAILED(m_pDevice->SetStreamSource(2, pAdditionalInfo->pStreamData, OffsetInBytes, pAdditionalInfo->Stride)))
     {
-        g_pProxyDevice->SetVertexDeclaration(pPreviousDecl);
+        proxyDevice->SetVertexDeclaration(pPreviousDecl);
         SAFE_RELEASE(pPreviousDecl);
         return false;
     }
@@ -298,7 +311,9 @@ void CAdditionalVertexStreamManager::MaybeUnsetAdditionalVertexStream()
         if (bDeviceOperational)
         {
             // Set prev declaration
-            g_pProxyDevice->SetVertexDeclaration(m_pOldVertexDeclaration);
+            CScopedActiveProxyDevice proxyDevice;
+            if (proxyDevice)
+                proxyDevice->SetVertexDeclaration(m_pOldVertexDeclaration);
 
             // Unset additional stream
             m_pDevice->SetStreamSource(2, nullptr, 0, 0);

Client/core/CAdditionalVertexStreamManager.h

@@ -84,7 +84,7 @@ class CAdditionalVertexStreamManager
     void OnVertexBufferRangeInvalidated(IDirect3DVertexBuffer9* pStreamData, uint Offset, uint Size);
 
     static CAdditionalVertexStreamManager* GetSingleton();
-    static CAdditionalVertexStreamManager* GetExistingSingleton() noexcept;
+    static CAdditionalVertexStreamManager* GetExistingSingleton();
     static void DestroySingleton();
 
 protected: