From 15450a6d0c527165749cc03a3ad45382ecb75430 Mon Sep 17 00:00:00 2001 From: Peace-Maker Date: Tue, 23 Jun 2020 19:12:23 +0200 Subject: [PATCH] Fix use-after-free when creating custom user messages When creating our own "owned and local" protobuf message in `StartProtobufMessage`, `m_FakeEngineBuffer` is used to track that message. In `EndMessage` the message is optionally converted to a "private" one with the right abi on osx and passed to the engine's `SendUserMessage`. On linux and windows the same message as in the `m_FakeEngineBuffer` is passed though without conversion. `engine->SendUserMessage` has a vtable hook which sets `m_FakeEngineBuffer` to the passed argument. `m_FakeEngineBuffer` frees the message it previously held, since it's "owned" from `StartProtobufMessage`, but that's the same one that's passed in as argument so a use-after-free in the engine happens when the now-freed message pointer is forwarded to the real `SendUserMessage` in the engine. The message created in `StartProtobufMessage` wasn't free'd at all when hooks are blocked too. This fix moves the message buffer into a local variable which is destroyed at the end of the function. Fixes #1286 and #1296 --- core/UserMessages.cpp | 7 +++---- core/pb_handle.h | 3 ++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/UserMessages.cpp b/core/UserMessages.cpp index 922090455..48a10a8bc 100644 --- a/core/UserMessages.cpp +++ b/core/UserMessages.cpp @@ -312,12 +312,12 @@ bool UserMessages::EndMessage() } #if SOURCE_ENGINE == SE_CSGO || SOURCE_ENGINE == SE_BLADE + PbHandle localBuffer = std::move(m_FakeEngineBuffer); if (m_CurFlags & USERMSG_BLOCKHOOKS) { - PbHandle priv = m_FakeEngineBuffer.ToPrivate(m_CurId); + PbHandle priv = localBuffer.ToPrivate(m_CurId); ENGINE_CALL(SendUserMessage)(static_cast(m_CellRecFilter), m_CurId, *priv.GetPrivateMessage()); - m_FakeEngineBuffer = nullptr; } else { OnMessageEnd_Pre(); @@ -327,10 +327,9 @@ bool UserMessages::EndMessage() case MRES_HANDLED: case MRES_OVERRIDE: { - PbHandle priv = m_FakeEngineBuffer.ToPrivate(m_CurId); + PbHandle priv = localBuffer.ToPrivate(m_CurId); engine->SendUserMessage(static_cast(m_CellRecFilter), m_CurId, *priv.GetPrivateMessage()); - m_FakeEngineBuffer = nullptr; break; } //case MRES_SUPERCEDE: diff --git a/core/pb_handle.h b/core/pb_handle.h index 73e6f434c..5951b7af7 100644 --- a/core/pb_handle.h +++ b/core/pb_handle.h @@ -105,7 +105,8 @@ public: } PbHandle& operator =(PbHandle&& other) { - maybe_free(); + if (other.msg_ != msg_) + maybe_free(); msg_ = other.msg_; ownership_ = other.ownership_; locality_ = other.locality_;