Restrictions in the functions setElementPosition, blowVehicle, and setElementHealth on the client side

[FileEX]

Related #3392

This PR restricts synchronization of the functions setElementPosition, blowVehicle, and restricts setElementHealth.

• setElementPosition for vehicles is restricted and can only be used on local vehicles and those for which the client is the syncer.

• setElementHealth for the local player is restricted due to abuse.

• blowVehicle is restricted and can only be used on local vehicles and those for which the client is the syncer.

Of course, this does not permanently solve the problem with cheaters, but it will prevent the mentioned functions from being used through just any Lua executor.

[Dutchman101]

Concept (desirability) of these changes are approved by project management.
Of course, code review is pending

We have a small window for security, - server/client authoriativeness - changes to be included in the release of MTA 1.7, which is the moment for backwards-incompatible changes.

[ffsPLASMA]

I find this a little bit too intrusive. The blowVehicle abuse got fixed by nico making onVehicleExplode event and therefore the explosion cancellable. With this PR I assume the client cannot explode a serverside vehicle even if hes the active syncer? If so thats very annoying to work arround then. Same for setVehicleHandling. This will make us rewrite lots of logic for certain cases. AFAIK using setElementHealth/setElementPostion on an element the client is not in charge/sync of is useless anyway since it wont get synced to others.

[ffsPLASMA]

I simply see no benefit since there are already ways to detect all of those abuses within serverside lua and deal with it.

[Dutchman101]

Rewriting logic for a new major release is to be expected. It's reasonable.

Calling the same function (but server-side) with a trigger instead, isn't very annoying. If it is to you, then you can't upgrade to MTA 1.7 when it releases.. or for that matter, move along with any project that sometimes ships a new major release with deprecations & API differences.

[samr46]

Calling server-side functions with a trigger instead, isn't very annoying

This will result in performance degradation for things done by setElementPosition in real time and will certainly break a lot of features. Maybe changes to this particular function should be excluded from this PR.

[Dutchman101]

Calling server-side functions with a trigger instead, isn't very annoying

This will result in performance degradation for things done by setElementPosition in real time and will certainly break a lot of features. Maybe changes to this particular function should be excluded from this PR.

using setElementPosition on a non-local object will already result in desyncs for remote players, doesn't it? It's not getting synced to server or other players/RPC'd. I thought that this particular disablement in this PR (setElementPosition) was more of a formality to avoid issues.

Otherwise, it can certainly be excluded from the PR.

Perhaps the PR is misconstrued, if the majority of sets on non-local, remote elements with most of these functions, will only result in desyncs for remote players. One thing that will harm cheaters though is to disallow setElementHealth on themselves (localPlayer), as that is synced onwards to server.

Well. i admit that i talked about approval without sufficient investigation on these things. Let's explore everything first.

[ffsPLASMA]

Calling server-side functions with a trigger instead, isn't very annoying

This will result in performance degradation for things done by setElementPosition in real time and will certainly break a lot of features. Maybe changes to this particular function should be excluded from this PR.

using setElementPosition on a non-local object will already result in desyncs for remote players, doesn't it? It's not getting synced to server or other players/RPC'd. I thought that this particular disablement in this PR (setElementPosition) was more of a formality to avoid issues.

Otherwise, we can certainly consider not including it.

If server spawns a vehicle and localplayer enters it and becomes syncer, he can use setElementPosition on that vehicle to change position. So the client would have to trigger a server event now to change it. Depending on the script supplied by race map, this can cause event spamming.

[samr46]

using setElementPosition on a non-local object will already result in desyncs for remote players, doesn't it

It doesn't if it's implemented correctly (or if you change the position of local player - a lot of legitimate use cases besides cheating).

I thought that this particular disablement in this PR (setElementPosition) was more of a formality to avoid issues.

Well, it will create issues for many projects for sure. Changes to other functions are fine but this one is important.

To be honest, if this PR is merged in current state, my project will have to consider staying on 1.6 or creating custom build. I don't really use setElementPosition in most of the scripts. But some features use it and if I mitigate it in any other way then it will not be smooth anymore. So some good features will have to be removed and it's a problem.

[FileEX]

It’s true that onVehicleExplode addresses abuse issues, but still, using any random executor, one could send junk explosion packets just to have them canceled on the server side in the onVehicleExplode event.

I know cheaters can send packets anyway, but a large part of them are simply low-IQ people who rely solely on executing malicious Lua code.

It’s fine - perhaps setElementPosition is a bit too “brutal” here and worth reconsidering.

[ffsPLASMA]

It’s true that onVehicleExplode addresses abuse issues, but still, using any random executor, one could send junk explosion packets just to have them canceled on the server side in the onVehicleExplode event.

I know cheaters can send packets anyway, but a large part of them are simply low-IQ people who rely solely on executing malicious Lua code.

It’s fine - perhaps setElementPosition is a bit too “brutal” here and worth reconsidering.

this works perfectly fine, can also be used for onExplosion event

function handleVehicleExplosions(bWithExplosion, uInstigator)
-- check if vehicle and player are in same dimension, if not cancel it

-- check if vehicle and player are in near distance, if not cancel it

-- check if player is active syncer or driver, if not cancel it

-- add any other logic checks that fits your needs

-- add +1 for the client sending vehicle explosion
-- if client sent 5+ explosions withing 5 seconds, kick him
end
addEventHandler("onVehicleExplode", root, handleVehicleExplosions);

[Dutchman101]

Maybe it'll be better if the PR is changed to that it'll allow setElementPosition on a serverside vehicle if local player is also the syncer?

[samr46]

Maybe it'll be better if the PR is changed to that it'll allow setElementPosition on a serverside vehicle if local player is also the syncer?

Vehicles isn't the biggest problem. Objects, peds and local player on the other hand..

[Dutchman101]

Then i propose that the setElementPosition restriction is modified: only block setting the position of a serverside vehicle of which you as local player are not the syncer.

Well, like i already admitted earlier, details on the PR's implementation have to be worked out.

So far i can see:

• Restrict client-side setElementPosition on server vehicles which client isn't the syncer of
• Restrict client-side setElementHealth on element type player, this will block cheats that facilitate godmode by spamming full hp set continuously or when hacker gets damaged (This will then propagate to server, as do GTA health values)

[samr46]

• Restrict client-side setElementPosition on server vehicles which client isn't the syncer of
• Restrict client-side setElementHealth on element type player, this will block cheats that facilitate godmode by spamming full hp set continuously or when hacker gets damaged (This will then propagate to server, as do GTA health values)

This sounds good. There is no reason to restrict setElementPosition for all server side elements or in particular for players, peds and objects.

Changes to setElementHealth will likely make some honeypots useless but it's fine.

[xLive]

setVehicleHandling is important for setting the handling on new allocated vehicles models using engineRequestModel.

When you set a vehicle to use a new allocated ID, all clients need to update the vehicle model on the client-side to the new allocated ID, and then use setVehicleHandling to set the proper handling for the custom vehicle.

If this PR were merged, the only way to do that would be for the client-side to tell the server-side to use setVehicleHandling every time a client loads a custom vehicle, which is a bad approach.

That's why setVehicleHandling was changed to work on remote vehicles on the client-side: #1935 (comment)
https://discord.com/channels/801330706252038164/834070650007978011/1149988962606256218 (mta dev discord) for more info.

[FileEX]

• setElementPosition has been restricted only to vehicles – it’s now available either for local vehicles or for the current syncer.

• setVehicleHandling is still available, but we need to think of another solution, since handling is currently one of the most abused features.

[ffsPLASMA]

• setElementPosition has been restricted only to vehicles – it’s now available either for local vehicles or for the current syncer.

  • setVehicleHandling is still available, but we need to think of another solution, since handling is currently one of the most abused features.

this should also apply for blowVehicle being available for vehicles the client is active syncer of

[xLive]

Restricting setVehicleHandling isn’t going to solve the problem anyway. Cheaters will just switch to functions like setElementVelocity or setElementAngularVelocity.

Are we going to restrict those too? They’re mostly used client-side for real-time changes, like pressing a key to boost a vehicle or boosting/stopping when entering a marker. Doing that server-side would cause a delay, which isn’t great and doesn’t work for a lot of situations. The same goes for plenty of other functions as well.

[sacr1ficez]

It is also worth to note that setElementPosition has been used in many noclip/airbreak or even ladder scripts.

[FileEX]

• setElementHealth is restricted to localPlayer only.

[FileEX]

• setElementPosition has been restricted only to vehicles – it’s now available either for local vehicles or for the current syncer.

  • setVehicleHandling is still available, but we need to think of another solution, since handling is currently one of the most abused features.

this should also apply for blowVehicle being available for vehicles the client is active syncer of

For what? What is an example of a useful application for this? Cheaters can easily become the syncer of a vehicle, and then this whole PR simply makes no sense. Such highly negative feedback in response to attempts at fighting cheaters and limiting their capabilities is strange and incomprehensible to me. For now, this inclines me to abandon this PR if it is not welcome. Perhaps it will be limited only to setElementHealth.

[MegadreamsBE]

This PR, in its original form, will kill Sphene. We have to rely on functions like setElementPosition client-side, for peds created server-side to ensure changes are applied fast. We can work around this to some extend, but it will cause serious delays and related issues that we may never be able to fix. I'm not opposed to implementing it, but at the very least it should be configurable for resources/servers that rely on it.

Otherwise, a middle-ground on allowing elements that have the client as their syncer to use these functions is perfectly acceptable. The introduction of persist to setElementSyncer a while ago, will allow servers to control this themselves to help counter cheaters (we already use this in Sphene).

[ffsPLASMA]

• setElementPosition has been restricted only to vehicles – it’s now available either for local vehicles or for the current syncer.

  • setVehicleHandling is still available, but we need to think of another solution, since handling is currently one of the most abused features.

this should also apply for blowVehicle being available for vehicles the client is active syncer of

For what? What is an example of a useful application for this? Cheaters can easily become the syncer of a vehicle, and then this whole PR simply makes no sense. Such highly negative feedback in response to attempts at fighting cheaters and limiting their capabilities is strange and incomprehensible to me. For now, this inclines me to abandon this PR if it is not welcome. Perhaps it will be limited only to setElementHealth.

I just dont see any benefit to such a restriction for elements where client is active syncer of. We already got setElementSyncer to enforce client to be a syncer of a certain element. There are a bunch of events to detect abuse/cheating clients and cancel/revert them. IMO this will cause lots of harm if we deny the client to use the functions on elements hes active syncer of. This will also cause lots of headache and more event triggering/delays to get the same job done as before.

And even then, the skilled cheater will modify network packets then instead of using lua.

[FileEX]

And even then, the skilled cheater will modify network packets then instead of using lua.

According to what the AC department stated, there aren’t too many such cheaters. The vast majority are simply kids buying ready-made menus and running malicious Lua code in a basic injector/executor. If packet manipulation were really that easy, they could destroy any server within seconds by sending fake synchronization packets, causing total chaos and desynchronization — but that’s not the case.

However, since this PR has such undesirable consequences, it will be modified as follows:

• setElementPosition will be restricted only to vehicles; it can be used on local vehicles and those for which the player is the syncer.

• setVehicleHandling remains untouched.

• setElementHealth for localPlayer remains restricted due to abuse and self-granted godmode.

• blowVehicle will be restricted to local vehicles and those for which the player is the syncer.

I want to emphasize again that the overwhelming majority of cheaters are kids with open menus that rely on malicious Lua code, so we should gradually remove their ability to do so. For this reason, even though onVehicleExplode exists, the blowVehicle function is still being restricted.

Beyond this point, I won’t make further concessions -otherwise this PR becomes meaningless. If this solution is not accepted, then the decision is up to the staff, since this PR was opened with the approval of Dutchman and botder.

[MegadreamsBE]

And even then, the skilled cheater will modify network packets then instead of using lua.
* setElementPosition will be restricted only to vehicles; it can be used on local vehicles and those for which the player is the syncer.

Can we still use it on peds? If so, I'm happy with the compromise. Not having client-side access to health is annoying, but workable for us.

EDIT: I checked the code changes. I approve of the approach. Thank you for listening!