feat(gamestate/server): add blockedEvents system

[Ekinoxx0]

Goal of this PR

Rather than doing individual convars or event handler for each and every events that could be potentially exploited in the future (See #3461 for the most recent exploit discovered),
it would be best to allow developers to block individual events that correspond best to their usage.

There is currently a 3 convars blocking already in place related to blocking events, but each one of them needed a PR to be properly made and accepted, causing some abuse during this period.
Doing this using a native would allow more fine tuning from servers owners and be future proof (even for RDR2).
Related convar are : sv_enableNetworkedSounds/sv_enableNetworkedPhoneExplosions/sv_enableNetworkedScriptEntityStates

This method also reduce the need for event handlers for some events that are mostly blocked by servers like "clearPedTasksEvent".
For example, in my codebase :

AddEventHandler('giveWeaponEvent', function(playerId, eventData)
    CancelEvent()
end)
AddEventHandler('removeAllWeaponsEvent', function(playerId, eventData)
    CancelEvent()
end)
AddEventHandler('clearPedTasksEvent', function(playerId, eventData)
    CancelEvent()
end)
AddEventHandler('ptFxEvent', function(playerId, eventData)
    CancelEvent()
end)

Would be replaced with

-- Using msgNetGameEvent (v1)
DisableNetGameEvent(12, true)
DisableNetGameEvent(14, true)
DisableNetGameEvent(43, true)
DisableNetGameEvent(74, true)

-- Using msgNetGameEventV2
DisableNetGameEvent(`GIVE_WEAPON_EVENT`, true)
DisableNetGameEvent(`REMOVE_ALL_WEAPONS_EVENT`, true)
DisableNetGameEvent(`NETWORK_CLEAR_PED_TASKS_EVENT`, true)
DisableNetGameEvent(`NETWORK_PTFX_EVENT`, true)

-- Add new discovered exploit
DisableNetGameEvent(64, true)
DisableNetGameEvent(`GIVE_PICKUP_REWARDS_EVENT`, true)

For my personnal use case, I would also block these events on my server:
CLEAR_AREA_EVENT (I'm never using this, don't want this to be exploited)
CLEAR_RECTANGLE_AREA_EVENT (Same)
PLAYER_TAUNT_EVENT
NETWORK_PLAY_AIRDEFENSE_FIRE_EVENT
NETWORK_BANK_REQUEST_EVENT
REQUEST_DOOR_EVENT (My doors are not networked)
BLOCK_WEAPON_SELECTION (Currently exploitable)
NETWORK_SPECIAL_FIRE_EQUIPPED_WEAPON (Currently exploitable)

Potential issues

• Servers owners or developers could block events that are absolutely needed for the game to work properl, we could prevent some events from being blocked.

• msgNetGameEvent being in a weird "v1 still the main thing, but v2 on it's way" state, we are supporting both format in the same native and with the same parameter, this could be handled differently.

How is this PR achieving the goal

Add a native to allow developers to block individual events, when the server receive one of the specified events, it just deny them.

This PR applies to the following area(s)

Server

Successfully tested on

Game builds: 1604

Platforms: Windows

Checklist

• Code compiles and has been tested successfully.
• Code explains itself well and/or is documented.
• My commit message explains what the changes do and what they are for.
• No extra compilation warnings are added by these changes.

Fixes issues

Related PR : #3461
Also see Cfx discord or engineering discord for recent discussion related to pickup exploit.

[Ekinoxx0]

I guess i didn't explain the CPickup exploit properly before all this.
Currently, cheaters can send a CGivePickupRewardsEvent, defining the "receiving player" argument to any player, and the reward to any weapon or heal.

Some servers (including my own) are checking that the player has any weapon "not given by my script", causing a ban.
This can easily be fixed by toggling the use of pickups with "ToggleUsePickupsForPlayer", like so:

ToggleUsePickupsForPlayer(PlayerId(), `PICKUP_WEAPON_STUNGUN`, false)
-- To implement this correctly, disable all pickups, not just this one, complete list of pickups can be found easily

[AvarianKnight]

Not sure Cfx's view on anti-cheat related stuff, or if it changed compared to how it previously was, but having callbacks for when these get hit would be nice.

[Scullyy]

Yeah callbacks for this would be nice so you can actually ban the player if needed, otherwise the abuser would still be roaming around in the server. If you're just cancelling the event like the example you provided above it's handy but many people implement logic to ban within certain conditions.

[william-des]

Relying on convars/toggle functions to block game events, instead of handlers/callbacks just to discourage server-side “anti-cheat,” also kills flexibility. Some events like REQUEST_CONTROL_EVENT, RAGDOLL_REQUEST_EVENT, NETWORK_PLAY_SOUND_EVENT, or CLEAR_AREA_EVENT might need to be allowed selectively based on player permissions, game mode, bucket, etc. Hard blocking them globally limits more advanced use cases.

[FabianTerhorst]

You could expose a method of the server game state to check if the event is blocked. This would allow a much earlier interception inside

fivem/code/components/citizen-server-impl/src/packethandlers/NetGameEventPacketHandler.cpp

Line 63 in ef8008b

serverNetEvent.eventNameHash = clientNetEvent.eventNameHash;

But do not forget the shared lock when you wanna use this approach.

[n3xuuu]

It's a nice addition to block network events server-wide, but it would be even better to have a native to allow/disallow those events based on a player.

[prikolium-cfx]

This approach looks better than previous try, but I would prefer to have this in server.cfg instead, because it shouldn't be a resource responsibility, but server owners choice to disable some events replication completely

[Ekinoxx0]

This PR now proposes the addition of two commands instead of a native, allowing the configuration to be done during the initialization step (via server.cfg) rather than at runtime.

Some have also suggested that this PR should include callbacks/events. I don't have a strong opinion on this, but it could be implemented behind a convar (i.e., enabling the convar adds events; disabling it omits them), which would reduce the performance impact for servers that don't intend to use these events.

Please decide whether adding EventHandlers for every rejected event is a good idea.
and also determine whether maintaining compatibility with the v1 netGameEvent is beneficial, as it could potentially confuse server owners about which arguments they should use in the commands (either event id, or event name hash).

[Scullyy]

Maybe instead of two commands it could be done under one command with a parameter to toggle it like some of the other commands. EventHandlers would be a nice addition as some net events could be allowed under certain conditions.

[Korioz]

Support for netGameEvent v1 for this system should be dropped, at this point v2 is pretty stable, the fact that IDs could change if you update the game build is an issue, server owners could be lost and don't know why all of a sudden something does not work.

I support @william-des point about flexibility, a simple convar to block would be enough for most of events for most of servers but if we force it to be filtered only through a convar it will kills all other servers that have different use cases, or at least kills some of theirs functionalities. Maybe making the convar have 3 type would be better, like "set_net_game_event" which accepts type 0 = DEFAULT, 1 = EVENT to resources for flexibility (only for events that supports it), 2 = BLOCK.

To ensure users don't shoot themselves in the foot or bad actors, there should be a list of forced net game events behaviors that cannot be overwritten and probably a default list that ensures same behavior with the old system.

[Yum1x]

which accepts type 0 = DEFAULT, 1 = EVENT to resources for flexibility (only for events that supports it), 2 = BLOCK.

IMO, the philosophy behind allowing a blocked event to also be handled is worth careful consideration.

The ability to block an exploitable event is adequate if the server owner wants to safeguard their player base. The server is secured and the exploit attempt is unsuccessful as soon as the event is blocked.

Allowing this same event to be used to issue a ban creates a paradox: We would be punishing a player for an intention, not for a concrete result. The action was prevented; therefore, punishing for something that "didn't happen" is questionable, as we would be banning based on an intercepted intention, not on actual damage.

[Korioz]

I'm not arguing for flexibility to ban users triggering thoses but to block on specific cases or launch others actions example ptFxEvent maybe you want to allow certain particles and not block everything or fireEvent maybe you just want to intercept networked fires server side to trigger a logic not related to anything about blocking abuses.

[FabianTerhorst]

Let's not over complicate this. One command that only works with net game events v2 is fine. Also use the same command with hash and true/false to add and remove it from the list.

Add the command handler to the https://github.com/citizenfx/fivem/blob/ef8008bd96c881f9cec47cf927fd9f613a882434/code/components/citizen-server-impl/src/packethandlers/NetGameEventPacketHandler.cpp ctor and check the hash at

fivem/code/components/citizen-server-impl/src/packethandlers/NetGameEventPacketHandler.cpp

Line 63 in ef8008b

serverNetEvent.eventNameHash = clientNetEvent.eventNameHash;

[FabianTerhorst]

Keeping two commands is also fine.

[prikolium-cfx]

It's almost ready to be merged, just move this as Fabian proposed.
Also please rename second command to unblock, not allow, as it's not disallowed by default and can be blocked only by user.

[Scullyy]

which accepts type 0 = DEFAULT, 1 = EVENT to resources for flexibility (only for events that supports it), 2 = BLOCK.

IMO, the philosophy behind allowing a blocked event to also be handled is worth careful consideration.

The ability to block an exploitable event is adequate if the server owner wants to safeguard their player base. The server is secured and the exploit attempt is unsuccessful as soon as the event is blocked.

Allowing this same event to be used to issue a ban creates a paradox: We would be punishing a player for an intention, not for a concrete result. The action was prevented; therefore, punishing for something that "didn't happen" is questionable, as we would be banning based on an intercepted intention, not on actual damage.

Why would banning them be an issue though? Attempting to cheat on a server is still a valid reason to ban someone regardless of if they were successful or not.

[JamesEU]

I'm not arguing for flexibility to ban users triggering thoses but to block on specific cases or launch others actions example ptFxEvent maybe you want to allow certain particles and not block everything or fireEvent maybe you just want to intercept networked fires server side to trigger a logic not related to anything about blocking abuses.

I agree with this, you'd want to allow certain things, there shouldn't just be a blanket block.

[Yum1x]

Why would banning them be an issue though? Attempting to cheat on a server is still a valid reason to ban someone regardless of if they were successful or not.

Anti-cheat measures should be the responsibility of the Cfx.re team, not delegated to server owners. Creating measures that shift this responsibility to the user allows a shady market of third-party 'anti-cheat solutions' to proliferate. These are basically obfuscated code with full access to the host system, doing who-knows-what, and they don't bring any real sense of security to the community.

[JamesEU]

Why would banning them be an issue though? Attempting to cheat on a server is still a valid reason to ban someone regardless of if they were successful or not.

Anti-cheat measures should be the responsibility of the Cfx.re team, not delegated to server owners. Creating measures that shift this responsibility to the user allows a shady market of third-party 'anti-cheat solutions' to proliferate. These are basically obfuscated code with full access to the host system, doing who-knows-what, and they don't bring any real sense of security to the community.

Regardless it's obviously the ship that sailed with Cfx.re in maintaining adhesive a long time ago. I agree to an extent but not everyone is trying to run a paid anti cheat service, some will want these features for genuine reasons, either way functionality would need to be opened up for the ability to see what has been ran and by who, to allow filtering to be done as certain things will/should be allowed and would be required like certain particles etc, while still allowing server owners to cancel out what is not intended or used by a script on their server, thus filtering out these bad requests.

So if your whole point of not implementing this or even considering the functionality of having that handler to allow filtering of requests, rather than the blanket block on it, all just because there's concerns paid anti cheats will adopt it, then that's preposterous. Why should paid anti cheats affect what features we can implement and have.. If there's that grave of concerns, the team should tackle the anti cheats that breach TOS.

[AvarianKnight]

Anti-cheat measures should be the responsibility of the Cfx.re team, not delegated to server owners. Creating measures that shift this responsibility to the user allows a shady market of third-party 'anti-cheat solutions' to proliferate.

We should also remember this would be introduced for server owners not the cheat developers.

We shouldn't keep blocking useful prevention methods for server owners just because the same thing would be used by anticheat developers too, you fuck over the majority because of the minority.

[Nexxed]

Anti-cheat measures should be the responsibility of the Cfx.re team, not delegated to server owners. Creating measures that shift this responsibility to the user allows a shady market of third-party 'anti-cheat solutions' to proliferate.

We should also remember this would be introduced for server owners not the cheat developers.

We shouldn't keep blocking useful prevention methods for server owners just because the same thing would be used by anticheat developers too, you fuck over the majority because of the minority.

It's also worth noting that a platform-wide anti-cheat will be generic by nature, so it will not be able to "detect" server or game-mode specific behavior, which should instead be handled by server owners using additional features and tooling provided by Cfx.re.

[FabianTerhorst]

👍
Thanks a lot for your contribution.

[Scullyy]

Why would banning them be an issue though? Attempting to cheat on a server is still a valid reason to ban someone regardless of if they were successful or not.

Anti-cheat measures should be the responsibility of the Cfx.re team, not delegated to server owners. Creating measures that shift this responsibility to the user allows a shady market of third-party 'anti-cheat solutions' to proliferate. These are basically obfuscated code with full access to the host system, doing who-knows-what, and they don't bring any real sense of security to the community.

The implemented anti-cheat doesn't do much to protect servers immediately, that's why servers need to implement their own measures. It's widely known that measures implemented by servers themselves are far more effective at protecting themselves and CFX can't implement such measures to their own ban system as every server requires different measures to prevent false positives.