Protected Element Data

[TriangleToo]

Is your feature request related to a problem? Please describe.

Nowadays, when Lua injectors have become commonplace, and considering how a large portion of MTA servers use Element Data without knowing its security weaknesses/not being able to secure them, I think it would be a good idea to make this small - but facilitating change to help secure such servers.

Describe the solution you'd like

My suggestion is to add an additional parameter to the setElementData function - protected=false/true (default: false). If the parameter is set to true then all changes to the given element data from clientside will be ignored and printed in the debugscript. Eg. by rejecting network packets.

bool setElementData ( element theElement, string key, var value [, var syncMode="broadcast", var protected=false] )

Describe alternatives you've considered

For the time being, the only solution if you don't want to give up Element Data (and with the introduction of subscriptions, it's a very good feature for optimally storing very little data) is to use the onElementDataChange event and undo the changes and then kick/ban the invasive user. Unfortunately, this solution has a significant drawback - this event is triggered already AFTER the element data change (which could have already performed some actions, for example), so we are forced to undo it.

Additional context

In my opinion, this is a fairly simple and quick solution to help secure an important element datas on MTA servers that should not be modified from the client side. In addition, we can send such a flag to the client, so that if you try to send such a change, no network traffic is sent to the server (which can try to overload it) - I realize that everything can be bypassed by cheaters and such traffic can still go out (so the the server side must verify such a change by the protected flag), but it will already be much more difficult for them.

I realize that servers can secure the current Element Data system with the above-mentioned event or by switching to their own data system sent by events, but I think that such a change, because of its simplicity, will positively affect the security of smaller servers or servers created by less experienced scripters who, for some reason, do not want to use the above-mentioned solutions. I think that everyone is also familiar with the performance of the onElementDataChange event, so the introduction of this feature will also increase the performance of servers, because they will be able to avoid using this event too intensively by cheaters.

Security Policy

• [X] I have read and understood the Security Policy and this issue is not about a cheat or security vulnerability.

[PlatinMTA]

Maybe make onElementDataChange work with cancelEvent() instead? The idea is good nonetheless, but maybe it's just too convoluted. ElementData was always regarded as unsafe and also slow, and is useful only in certain scenarios.

People that are already miss using elementdatas will not use protected = true btw.

[TriangleToo]

Maybe make onElementDataChange work with cancelEvent() instead?

If it's easier to do, then why not. Always something more for security

The idea is good nonetheless, but maybe it's just too convoluted. ElementData was always regarded as unsafe and also slow, and is useful only in certain scenarios.

I agree that this was and is unsafe and slow, but yet somehow it has become established among scripters and certainly a large portion of RPG/DayZ (and similar) servers store important data in element data.

People that are already miss using elementdatas will not use protected = true btw.

The premise of this proposal, however, is that scripters can read documentation and will understand that with the default option, such data can be modified by potential cheaters and such a change of a single parameter should not be too much effort for them to protect important and client unmodifiable data stored there.

And from another point of view - the above proposal is also an easier way to implement security for people who are already trying to reverse changes in the eldata change event.

[PlatinMTA]

is that scripters can read documentation and will understand that with the default option

a fact that was acknowledge in the same setElementData wiki page since at least 2017 (but I think it was first introduced to that page by ccw in 2013).

I'm not against adding the protected argument, but using onElementDataChange is way better imo because you have control over it and not just some log. Also you could make them protected for some users and not others. I think making cancelEvent() work with that event will give us more benefits, specially considering that you wouldn't need to re-set the elementdata. However I doubt this is possible or easy to do, otherwise it would have happened sooner.

[tederis]

onElementDataChange may look better but must be avoided due to the performance impact. ElementData is often used as a high frequency synchronization conduit and Lua events will make it slower. I mean internally onElementDataChange is called after the element data change. And for the cancelling the element data must be reverted which is slow. It can be improved but it's still not that efficient as a protected argument.

[tederis]

And the better argument name is authoritative which is more common in this context.

[TriangleToo]

@tederis You're right, additionally, as part of the persuasion to introduce the authoritative parameter, we can add that in case someone uses the subscribe/false synchronization type, when trying to reverse the changes in onElementDataChange there is the additional problem of finding out what synchronization type the element data had, so in case of such an attack, the cheater can affect the performance of the whole server when, for example tries to overwrite previously unsynced/subscribed element data, and the potential developer in onElementDataChange will restore the element data to its previous state with default sync value (broadcast), because he won't be able to determine without additional effort (some saving when setting?) what state it was in previously.

Now another thought that came to my mind, what in the situation when we will update (and not set for the first time as I assumed in the proposal) the element data - will it be necessary to specify the authoritative parameter every time? Maybe it would be worthwhile to approach it from the side similar to subscription and add it as a function like setElementDataProtected(element, key, bool)?

[TheNormalnij]

setElementDataProtected looks fine. But what about behavior.

[tederis]

setElementDataProtected can do two things (simultaneously):

• send a lock signal to clients so clients won't be able to send element data changes
• forbid incoming from client packets

[TheNormalnij]

send a lock signal to clients so clients won't be able to send element data changes

A hacked client can ignore this flag.

forbid incoming from client packets

Forbid element data changes and send event "onElementDataBadAccess". You can kick or ban hackers in this event.

[TriangleToo]

setElementDataProtected can do two things (simultaneously):

• send a lock signal to clients so clients won't be able to send element data changes
• forbid incoming from client packets

I agree with this. In addition, I would like to point out that it is important to note here that the function should take syncMode into account - in the case of subscribe it should send a flag only to subscribers, broadcast - to all, and false to no one. In case someone try to set element data which has subscribe/false syncMode it should just reject such packet and notify server owners in debugscript (just like clientside triggered serverside event that doesn't exist).

send a lock signal to clients so clients won't be able to send element data changes

A hacked client can ignore this flag.

Let's not kid ourselves, but nowadays most of the cheats that this solution can be useful against are simply bypasses with lua injector. In such a basic case, this flag will not be ignored by them. Modified clients that could bypass this are probably less than 1% of cheat cases.

forbid incoming from client packets

Forbid element data changes and send event "onElementDataBadAccess". You can kick or ban hackers in this event.

Ignoring such packets is the main reason I would like to see this introduced. The solution you describe has its drawbacks:

• Such a change of element data has already been sent to the server (can be avoided by the flag from the first point), so we lost time for processing and server cpu/bandwith.
• The element data change has been triggered, so all event handlers with this type of event have been processed (which can lead to bad things in some bad scripts), because we have no way to cancel such an event.
• As for rejecting the change - the only possibility is to restore the element data to its previous value, which involves resending new (but old - valid one) data to all players, as well as losing the syncMode state (which I described in my previous comment: https://github.com/multitheftauto/mtasa-blue/issues/3286#issuecomment-1884538239)
• In addition to the above point - such data will be sent twice to ALL players ignoring syncMode - because that's how relay works from clientside (first violated value, and later restored value due to point above).

[TheNormalnij]

Let's not kid ourselves, but nowadays most of the cheats that this solution can be useful against are simply bypasses with lua injector. In such a basic case, this flag will not be ignored by them. Modified clients that could bypass this are probably less than 1% of cheat cases.

Yes. It's harder to change this flag on client. But it provides better security with simple solution.

Such a change of element data has already been sent to the server (can be avoided by the flag from the first point), so we lost time for processing and server cpu/bandwith.

You can kick or ban cheaters after first bad package. The package doesn't make any changes in element data. It only triggers "onElementDataBadAccess" . There is no cpu or bandwidth loss. I would rather ban cheaters than keep hacking attempts on server.

[TriangleToo]

Such a change of element data has already been sent to the server (can be avoided by the flag from the first point), so we lost time for processing and server cpu/bandwith.

You can kick or ban cheaters after first bad package. The package doesn't make any changes in element data. It only triggers "onElementDataBadAccess" . There is no cpu or bandwidth loss. I would rather ban cheaters than keep hacking attempts on server.

Ah, I misunderstood you earlier and thought you meant to use the onElementDataChange event and create your own event by AddEvent called onElementDataBadAccess :) If I understand correctly, you would want to add such an event in addition to ignoring the packet instead of outputting to debugscript, which is a good idea if I'm right👍

Let's not kid ourselves, but nowadays most of the cheats that this solution can be useful against are simply bypasses with lua injector. In such a basic case, this flag will not be ignored by them. Modified clients that could bypass this are probably less than 1% of cheat cases.

Yes. It's harder to change this flag on client. But it provides better security with simple solution.

I'm not quite sure what you meant here, but if we're talking about introducing a protected/authoritative flag (simple solution?) then I'm all for it (perhaps my earlier statement sounded different)