Source-Python-Dev-Team / Source.Python

This plugin aims to use boost::python and create an easily accessible wrapper around the Source Engine API for scripter use.
http://forums.sourcepython.com
GNU General Public License v3.0
164 stars 32 forks source link

[CS:GO, Windows] RecipientFilter.add_all_players() crash #194

Closed KirillMysnik closed 2 years ago

KirillMysnik commented 7 years ago

RecipientFilter.add_all_players() crashes when used on a filter provided by the game

Code to reproduce:

from engines.server import engine_server
from filters.recipients import RecipientFilter
from memory import Convention, DataType, get_object_pointer, make_object
from memory.hooks import PreHook

send_user_message = get_object_pointer(engine_server).make_virtual_function(
    45,   # Linux offset is 45 as well (at least before)
    Convention.THISCALL,
    [DataType.POINTER, DataType.POINTER, DataType.INT, DataType.POINTER],
    DataType.VOID
)

@PreHook(send_user_message)
def pre_send_user_message(args):
    recipients = make_object(RecipientFilter, args[1])
    recipients.add_all_players()

Note: add_all_players has to actually do something, i.e. you there must be somebody playing on the server. Tested message types: ProcessSpottedEntityUpdate, SayText2.

This code, however, doesn't crash:

from filters.recipients import RecipientFilter

recipients = RecipientFilter()
recipients.add_all_players()

I didn't test other methods like RecipientFilter.update. Also I didn't test Linux SRCDS.

Ayuto commented 7 years ago

Actually, the correct class would be BaseRecipientFilter and not RecipientFilter.

What's the output of "sp info"?

KirillMysnik commented 7 years ago
sp info

Date          : 2017-04-25 18:07:42.202935
OS            : Windows-10-10.0.14393
Game          : csgo
SP version    : 573
Server plugins:
   00: Source.Python, (C) 2012-2016, Source.Python Team.
SP plugins:
   00: crashtest
IedSupo commented 7 years ago

i think protobuf uses efficiency buffers, so you probably have to modify the buffer and add size to it according to the recipients you want to add. i dont think add_all_players() handles that internally ( its probably deprecated from bitbuffer) so the server crashes at write access. depending on what you want to do another option is to dump the old and recreate a new usermsg, if there is a way to reverse access the usrmsg type from within the prehook

KirillMysnik commented 7 years ago

Well, ProtobufMessage object itself is a different argument in the stack. It's unrelated to the RecipientFilter object.

So I don't change anything in the protobuf message in the example above.

Also it crashes immediately on calling add_all_players without even returning from the hook and passing control to the SendUserMessage.

Ayuto commented 7 years ago

you do realize add_all_players writes to the protobuf object that causes your server to crash?

No, it doesn't. Also, it's not a deprecated argument.

The actual problem is that an instance of IRecipientFilter (BaseRecipientFilter in python) is passed, but it's getting wrapped as MRecipientFilter (RecipientFilter in Python). Since IRecipientFilter is an abstract class, there don't exist any instances, but instances of a subclasses that implement IRecipientFilter. If the structure of those subclasses don't match with MRecipientFilter, there is a high chance that your server crashes. However, in this case the allocation/deallocation of memory probably just doesn't work properly.

When I get home I will do some tests. For the meantime this link might already point you in the right direction: https://github.com/Source-Python-Dev-Team/Source.Python/blob/master/src/core/modules/filters/filters_recipients.h#L114-L118

KirillMysnik commented 7 years ago

@Ayuto, Invincible tested this issue through me a bit and found out that add_all_players crashes when it's adding like 8th bot or so to the RecipientFilter.

Ayuto commented 7 years ago

Okay, I was able to reproduce the crash, but I'm not going to investigate this further today. For now, you can simply work around that issue by replacing the recipient instance.

from messages import get_message_index
from engines.server import engine_server
from filters.recipients import RecipientFilter
from memory.hooks import PreHook
from memory import get_virtual_function

my_filter = RecipientFilter()

SAY_TEXT2_INDEX = get_message_index('SayText2')

@PreHook(get_virtual_function(engine_server, 'SendUserMessage'))
def pre_send_user_message(args):
    if args[2] != SAY_TEXT2_INDEX:
        return

    my_filter.add_all_players()
    args[1] = my_filter
jordanbriere commented 2 years ago

I was also able to reproduce and after some tests it appears to crash whenever the memory exceeds 32 bytes. For example, reallocating it to 36 bytes is crashing:

m_Recipients.m_Memory.EnsureCapacity(9);

Being CSingleUserRecipientFilter instances, I suspect they may have a fixed size, or that we ends up owning the extra memory allocated which cause tracking issues because the game assumes it does, etc.

In any case, I don't plan to dig deeper into it, nor try to provide a fix for it, and will just go ahead and close this issue because users should use the messages.hooks package if they want to hook user messages instead of hooking it themselves.