Skip to content

Commit 93bea6c

Browse files
Dutchman101Dutchman101
committed
CEF safety fixes (fix UAF/UB/race)
1 parent 708afab commit 93bea6c

File tree

6 files changed

+197
-39
lines changed

6 files changed

+197
-39
lines changed

‎Client/cefweb/CWebView.cpp‎

Lines changed: 106 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "CWebDevTools.h"
1616
#include <chrono>
1717
#include "CWebViewAuth.h" // AUTH: IPC validation helpers
18+
#include <utility>
1819

1920
namespace
2021
{
@@ -23,6 +24,7 @@ namespace
2324

2425
CWebView::CWebView(bool bIsLocal, CWebBrowserItem* pWebBrowserRenderItem, bool bTransparent)
2526
{
27+
m_pEventTarget = std::make_shared<FEventTarget>();
2628
m_bIsLocal = bIsLocal;
2729
m_bIsTransparent = bTransparent;
2830
m_pWebBrowserRenderItem = pWebBrowserRenderItem;
@@ -42,6 +44,9 @@ CWebView::~CWebView()
4244
{
4345
m_bBeingDestroyed = true;
4446

47+
if (m_pEventTarget)
48+
m_pEventTarget->Clear(m_pEventsInterface);
49+
4550
if (IsMainThread())
4651
{
4752
if (auto pWebCore = g_pCore->GetWebCore(); pWebCore)
@@ -86,6 +91,41 @@ CWebView::~CWebView()
8691
OutputDebugLine("CWebView::~CWebView");
8792
}
8893

94+
void CWebView::SetWebBrowserEvents(CWebBrowserEventsInterface* pInterface)
95+
{
96+
m_pEventsInterface = pInterface;
97+
98+
if (m_pEventTarget)
99+
m_pEventTarget->Assign(pInterface);
100+
}
101+
102+
void CWebView::ClearWebBrowserEvents(CWebBrowserEventsInterface* pInterface)
103+
{
104+
if (m_pEventTarget)
105+
m_pEventTarget->Clear(pInterface);
106+
107+
if (m_pEventsInterface == pInterface)
108+
m_pEventsInterface = nullptr;
109+
}
110+
111+
void CWebView::QueueBrowserEvent(const char* name, std::function<void(CWebBrowserEventsInterface*)>&& fn)
112+
{
113+
auto target = m_pEventTarget;
114+
if (!target)
115+
return;
116+
117+
const auto token = target->CreateDispatchToken();
118+
119+
g_pCore->GetWebCore()->AddEventToEventQueue(
120+
[target, token, fn = std::move(fn)]() mutable {
121+
if (!target)
122+
return;
123+
124+
target->Dispatch(token, fn);
125+
},
126+
this, name);
127+
}
128+
89129
void CWebView::Initialise()
90130
{
91131
// Initialise the web session (which holds the actual settings) in in-memory mode
@@ -784,7 +824,11 @@ bool CWebView::GetFullPathFromLocal(SString& strPath)
784824
if (aborted)
785825
return;
786826

787-
result = m_pEventsInterface->Events_OnResourcePathCheck(strPath);
827+
auto* events = m_pEventsInterface;
828+
if (!events)
829+
return;
830+
831+
result = events->Events_OnResourcePathCheck(strPath);
788832
},
789833
this);
790834

@@ -813,8 +857,11 @@ void CWebView::HandleAjaxRequest(const SString& strURL, CAjaxResourceHandler* pH
813857
// Only queue event if not being destroyed to prevent UAF
814858
if (!m_bBeingDestroyed)
815859
{
816-
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnAjaxRequest, m_pEventsInterface, pHandler, strURL);
817-
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "AjaxResourceRequest");
860+
QueueBrowserEvent(
861+
"AjaxResourceRequest",
862+
[handler = pHandler, url = strURL](CWebBrowserEventsInterface* iface) {
863+
iface->Events_OnAjaxRequest(handler, url);
864+
});
818865
}
819866
}
820867

@@ -835,7 +882,11 @@ bool CWebView::VerifyFile(const SString& strPath, CBuffer& outFileData)
835882
if (aborted)
836883
return;
837884

838-
result = m_pEventsInterface->Events_OnResourceFileCheck(strPath, outFileData);
885+
auto* events = m_pEventsInterface;
886+
if (!events)
887+
return;
888+
889+
result = events->Events_OnResourceFileCheck(strPath, outFileData);
839890
},
840891
this);
841892

@@ -1115,8 +1166,11 @@ void CWebView::OnLoadStart(CefRefPtr<CefBrowser> browser, CefRefPtr<CefFrame> fr
11151166
return;
11161167

11171168
// Queue event to run on the main thread
1118-
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnLoadingStart, m_pEventsInterface, strURL, frame->IsMain());
1119-
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "OnLoadStart");
1169+
QueueBrowserEvent(
1170+
"OnLoadStart",
1171+
[url = strURL, isMain = frame->IsMain()](CWebBrowserEventsInterface* iface) {
1172+
iface->Events_OnLoadingStart(url, isMain);
1173+
});
11201174
}
11211175

11221176
////////////////////////////////////////////////////////////////////
@@ -1135,8 +1189,11 @@ void CWebView::OnLoadEnd(CefRefPtr<CefBrowser> browser, CefRefPtr<CefFrame> fram
11351189
SString strURL = UTF16ToMbUTF8(frame->GetURL());
11361190

11371191
// Queue event to run on the main thread
1138-
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnDocumentReady, m_pEventsInterface, strURL);
1139-
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "OnLoadEnd");
1192+
QueueBrowserEvent(
1193+
"OnLoadEnd",
1194+
[url = strURL](CWebBrowserEventsInterface* iface) {
1195+
iface->Events_OnDocumentReady(url);
1196+
});
11401197
}
11411198
}
11421199

@@ -1153,8 +1210,11 @@ void CWebView::OnLoadError(CefRefPtr<CefBrowser> browser, CefRefPtr<CefFrame> fr
11531210
SString strURL = UTF16ToMbUTF8(frame->GetURL());
11541211

11551212
// Queue event to run on the main thread
1156-
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnLoadingFailed, m_pEventsInterface, strURL, errorCode, SString(errorText));
1157-
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "OnLoadError");
1213+
QueueBrowserEvent(
1214+
"OnLoadError",
1215+
[url = strURL, errorCode, errorDescription = SString(errorText)](CWebBrowserEventsInterface* iface) mutable {
1216+
iface->Events_OnLoadingFailed(url, errorCode, errorDescription);
1217+
});
11581218
}
11591219

11601220
////////////////////////////////////////////////////////////////////
@@ -1200,8 +1260,11 @@ bool CWebView::OnBeforeBrowse(CefRefPtr<CefBrowser> browser, CefRefPtr<CefFrame>
12001260
bool bIsMainFrame = frame->IsMain();
12011261

12021262
// Queue event to run on the main thread
1203-
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnNavigate, m_pEventsInterface, SString(request->GetURL()), bResult, bIsMainFrame);
1204-
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "OnNavigate");
1263+
QueueBrowserEvent(
1264+
"OnNavigate",
1265+
[url = SString(request->GetURL()), blocked = bResult, isMain = bIsMainFrame](CWebBrowserEventsInterface* iface) mutable {
1266+
iface->Events_OnNavigate(url, blocked, isMain);
1267+
});
12051268

12061269
// Return execution to CEF
12071270
return bResult;
@@ -1264,9 +1327,12 @@ CefResourceRequestHandler::ReturnValue CWebView::OnBeforeResourceLoad(CefRefPtr<
12641327
if (urlState != eURLState::WEBPAGE_ALLOWED)
12651328
{
12661329
// Trigger onClientBrowserResourceBlocked event
1267-
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnResourceBlocked, m_pEventsInterface, SString(request->GetURL()), domain,
1268-
urlState == eURLState::WEBPAGE_NOT_LISTED ? 0 : 1);
1269-
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "OnResourceBlocked");
1330+
QueueBrowserEvent(
1331+
"OnResourceBlocked",
1332+
[url = SString(request->GetURL()), domain, reason = static_cast<unsigned char>(urlState == eURLState::WEBPAGE_NOT_LISTED ? 0 : 1)](
1333+
CWebBrowserEventsInterface* iface) mutable {
1334+
iface->Events_OnResourceBlocked(url, domain, reason);
1335+
});
12701336

12711337
return RV_CANCEL; // Block if explicitly forbidden
12721338
}
@@ -1283,9 +1349,11 @@ CefResourceRequestHandler::ReturnValue CWebView::OnBeforeResourceLoad(CefRefPtr<
12831349
}
12841350

12851351
// Trigger onClientBrowserResourceBlocked event
1286-
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnResourceBlocked, m_pEventsInterface, SString(request->GetURL()), "",
1287-
2); // reason 1 := blocked protocol scheme
1288-
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "OnResourceBlocked");
1352+
QueueBrowserEvent(
1353+
"OnResourceBlocked",
1354+
[url = SString(request->GetURL())](CWebBrowserEventsInterface* iface) mutable {
1355+
iface->Events_OnResourceBlocked(url, "", 2);
1356+
});
12891357

12901358
// Block everything else
12911359
return RV_CANCEL;
@@ -1331,8 +1399,11 @@ bool CWebView::OnBeforePopup(CefRefPtr<CefBrowser> browser, CefRefPtr<CefFrame>
13311399
SString strOpenerURL = UTF16ToMbUTF8(frame->GetURL());
13321400

13331401
// Queue event to run on the main thread
1334-
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnPopup, m_pEventsInterface, strTagetURL, strOpenerURL);
1335-
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "OnBeforePopup");
1402+
QueueBrowserEvent(
1403+
"OnBeforePopup",
1404+
[target = strTagetURL, opener = strOpenerURL](CWebBrowserEventsInterface* iface) {
1405+
iface->Events_OnPopup(target, opener);
1406+
});
13361407

13371408
// Block popups generally
13381409
return true;
@@ -1356,8 +1427,11 @@ void CWebView::OnAfterCreated(CefRefPtr<CefBrowser> browser)
13561427
m_pWebView = browser;
13571428

13581429
// Call created event callback
1359-
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnCreated, m_pEventsInterface);
1360-
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "OnAfterCreated");
1430+
QueueBrowserEvent(
1431+
"OnAfterCreated",
1432+
[](CWebBrowserEventsInterface* iface) {
1433+
iface->Events_OnCreated();
1434+
});
13611435
}
13621436

13631437
////////////////////////////////////////////////////////////////////
@@ -1413,8 +1487,11 @@ void CWebView::OnTitleChange(CefRefPtr<CefBrowser> browser, const CefString& tit
14131487
bool CWebView::OnTooltip(CefRefPtr<CefBrowser> browser, CefString& title)
14141488
{
14151489
// Queue event to run on the main thread
1416-
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnTooltip, m_pEventsInterface, UTF16ToMbUTF8(title));
1417-
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "OnTooltip");
1490+
QueueBrowserEvent(
1491+
"OnTooltip",
1492+
[tooltip = UTF16ToMbUTF8(title)](CWebBrowserEventsInterface* iface) mutable {
1493+
iface->Events_OnTooltip(tooltip);
1494+
});
14181495

14191496
return true;
14201497
}
@@ -1454,8 +1531,11 @@ bool CWebView::OnCursorChange(CefRefPtr<CefBrowser> browser, CefCursorHandle cur
14541531
unsigned char cursorIndex = static_cast<unsigned char>(type);
14551532

14561533
// Queue event to run on the main thread
1457-
auto func = std::bind(&CWebBrowserEventsInterface::Events_OnChangeCursor, m_pEventsInterface, cursorIndex);
1458-
g_pCore->GetWebCore()->AddEventToEventQueue(func, this, "OnCursorChange");
1534+
QueueBrowserEvent(
1535+
"OnCursorChange",
1536+
[cursorIndex](CWebBrowserEventsInterface* iface) {
1537+
iface->Events_OnChangeCursor(cursorIndex);
1538+
});
14591539

14601540
return false;
14611541
}

‎Client/cefweb/CWebView.h‎

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,28 @@
2121
#include <cef3/cef/include/cef_life_span_handler.h>
2222
#include <cef3/cef/include/cef_context_menu_handler.h>
2323
#include <cef3/cef/include/cef_resource_request_handler.h>
24+
#include <cef3/cef/include/cef_values.h>
2425
#include <SString.h>
25-
#include <mmdeviceapi.h>
2626
#include <audiopolicy.h>
2727
#include <condition_variable>
28+
#include <functional>
29+
#include <memory>
30+
#include <mmdeviceapi.h>
31+
#include <mutex>
32+
#include <cstdint>
2833
#define GetNextSibling(hwnd) GetWindow(hwnd, GW_HWNDNEXT) // Re-define the conflicting macro
2934
#define GetFirstChild(hwnd) GetTopWindow(hwnd)
3035

3136
#define MTA_CEF_USERAGENT "Multi Theft Auto: San Andreas Client " MTA_DM_BUILDTAG_LONG
3237

38+
// Forward declaration for WebViewAuth namespace functions (defined in CWebViewAuth.h)
39+
class CWebView;
40+
namespace WebViewAuth
41+
{
42+
bool HandleTriggerLuaEvent(CWebView*, CefRefPtr<CefListValue>, const bool);
43+
bool HandleInputFocus(CWebView*, CefRefPtr<CefListValue>, const bool);
44+
}
45+
3346
enum class ECefThreadState
3447
{
3548
Running = 0, // CEF thread is currently running
@@ -48,11 +61,15 @@ class CWebView : public CWebViewInterface,
4861
private CefDisplayHandler,
4962
private CefContextMenuHandler
5063
{
64+
friend bool WebViewAuth::HandleTriggerLuaEvent(CWebView*, CefRefPtr<CefListValue>, const bool);
65+
friend bool WebViewAuth::HandleInputFocus(CWebView*, CefRefPtr<CefListValue>, const bool);
66+
5167
public:
5268
CWebView(bool bIsLocal, CWebBrowserItem* pWebBrowserRenderItem, bool bTransparent = false);
5369
virtual ~CWebView();
5470
void Initialise();
55-
void SetWebBrowserEvents(CWebBrowserEventsInterface* pInterface) { m_pEventsInterface = pInterface; };
71+
void SetWebBrowserEvents(CWebBrowserEventsInterface* pInterface);
72+
void ClearWebBrowserEvents(CWebBrowserEventsInterface* pInterface);
5673
void CloseBrowser();
5774
CefRefPtr<CefBrowser> GetCefBrowser() { return m_pWebView; };
5875

@@ -183,6 +200,60 @@ class CWebView : public CWebViewInterface,
183200

184201
private:
185202
void ResumeCefThread();
203+
void QueueBrowserEvent(const char* name, std::function<void(CWebBrowserEventsInterface*)>&& fn);
204+
205+
struct FEventTarget
206+
{
207+
struct DispatchToken
208+
{
209+
uint64_t generation = 0;
210+
};
211+
212+
DispatchToken CreateDispatchToken() const
213+
{
214+
std::lock_guard<std::mutex> lock(mutex);
215+
return DispatchToken{generation};
216+
}
217+
218+
void Assign(CWebBrowserEventsInterface* ptr)
219+
{
220+
std::lock_guard<std::mutex> lock(mutex);
221+
if (target == ptr)
222+
return;
223+
224+
target = ptr;
225+
++generation;
226+
}
227+
228+
void Clear(CWebBrowserEventsInterface* expected)
229+
{
230+
std::lock_guard<std::mutex> lock(mutex);
231+
if (target != expected)
232+
return;
233+
234+
target = nullptr;
235+
++generation;
236+
}
237+
238+
template <typename Fn>
239+
void Dispatch(const DispatchToken& token, Fn&& fn)
240+
{
241+
CWebBrowserEventsInterface* current = nullptr;
242+
{
243+
std::lock_guard<std::mutex> lock(mutex);
244+
if (generation != token.generation || !target)
245+
return;
246+
current = target;
247+
}
248+
249+
fn(current);
250+
}
251+
252+
private:
253+
mutable std::mutex mutex;
254+
CWebBrowserEventsInterface* target = nullptr;
255+
uint64_t generation = 0;
256+
};
186257

187258
CefRefPtr<CefBrowser> m_pWebView;
188259
CWebBrowserItem* m_pWebBrowserRenderItem;
@@ -198,6 +269,7 @@ class CWebView : public CWebViewInterface,
198269
std::map<SString, SString> m_Properties;
199270
bool m_bHasInputFocus;
200271
std::set<std::string> m_AjaxHandlers;
272+
std::shared_ptr<FEventTarget> m_pEventTarget;
201273

202274
struct
203275
{

0 commit comments

Comments
 (0)