Skip to content

Commit 15450a6

Browse files
peace-makerdvander
authored andcommitted
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 alliedmodders#1286 and alliedmodders#1296
1 parent 832519a commit 15450a6

File tree

2 files changed

+5
-5
lines changed

2 files changed

+5
-5
lines changed

core/UserMessages.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -312,12 +312,12 @@ bool UserMessages::EndMessage()
312312
}
313313

314314
#if SOURCE_ENGINE == SE_CSGO || SOURCE_ENGINE == SE_BLADE
315+
PbHandle localBuffer = std::move(m_FakeEngineBuffer);
315316
if (m_CurFlags & USERMSG_BLOCKHOOKS)
316317
{
317-
PbHandle priv = m_FakeEngineBuffer.ToPrivate(m_CurId);
318+
PbHandle priv = localBuffer.ToPrivate(m_CurId);
318319
ENGINE_CALL(SendUserMessage)(static_cast<IRecipientFilter &>(m_CellRecFilter), m_CurId,
319320
*priv.GetPrivateMessage());
320-
m_FakeEngineBuffer = nullptr;
321321
} else {
322322
OnMessageEnd_Pre();
323323

@@ -327,10 +327,9 @@ bool UserMessages::EndMessage()
327327
case MRES_HANDLED:
328328
case MRES_OVERRIDE:
329329
{
330-
PbHandle priv = m_FakeEngineBuffer.ToPrivate(m_CurId);
330+
PbHandle priv = localBuffer.ToPrivate(m_CurId);
331331
engine->SendUserMessage(static_cast<IRecipientFilter &>(m_CellRecFilter), m_CurId,
332332
*priv.GetPrivateMessage());
333-
m_FakeEngineBuffer = nullptr;
334333
break;
335334
}
336335
//case MRES_SUPERCEDE:

core/pb_handle.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ class PbHandle
105105
}
106106

107107
PbHandle& operator =(PbHandle&& other) {
108-
maybe_free();
108+
if (other.msg_ != msg_)
109+
maybe_free();
109110
msg_ = other.msg_;
110111
ownership_ = other.ownership_;
111112
locality_ = other.locality_;

0 commit comments

Comments
 (0)