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
This commit is contained in:
Peace-Maker 2020-06-23 19:12:23 +02:00 committed by David Anderson
parent 832519ab64
commit 15450a6d0c
2 changed files with 5 additions and 5 deletions

View File

@ -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<IRecipientFilter &>(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<IRecipientFilter &>(m_CellRecFilter), m_CurId,
*priv.GetPrivateMessage());
m_FakeEngineBuffer = nullptr;
break;
}
//case MRES_SUPERCEDE:

View File

@ -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_;