Add element data client trust options.

[TheNormalnij]

Added elementdata_whitelisted setting for mtaserver.conf

• Enabled: the client can't change element data on server, except allowed keys.
• Disabled: the client can change any element data. (default, unsafe)

Added clientTrustMode parameter for setElementData

-- Allow changes from the client
bool setElementData(root,  key, value, syncType, "allow" )

-- Deny changes from the client
bool setElementData(root,  key, value, syncType, "deny" )

-- Respect elementdata_whitelisted setting
bool setElementData(root,  key, value, syncType, "default" )

Added event onPlayerChangesProtectedData
source: player
arguments:

• element,
• key,
• value

[TracerDS]

Whats the purpose of these functions?

[ffsPLASMA]

isnt this kinda redundant with the way you can already detect elementdata changes in lua? thats why I added it to security resource.

https://github.com/multitheftauto/mtasa-resources/blob/master/%5Badmin%5D/security/players.lua#L9

[FileEX]

From what I understand, the issue is that if we have an elementData key, like "user:points", which we only use in server-side scripts and it’s not synchronized on the server, a potential cheater could still overwrite it by using setElementData client-side with the sync parameter set to true. My suggestion is to add a new server-side argument, something like ignoreClientSync. Then, in the server’s elementData synchronization packet, if a particular elementData key has the ignoreClientSync parameter set to true, this synchronization packet would simply be ignored, and the value would remain unchanged.

In my opinion, a single simple argument is a much better solution than having entirely separate functions for this

[ffsPLASMA]

From what I understand, the issue is that if we have an elementData key, like "user:points", which we only use in server-side scripts and it’s not synchronized on the server, a potential cheater could still overwrite it by using setElementData client-side with the sync parameter set to true. My suggestion is to add a new server-side argument, something like ignoreClientSync. Then, in the server’s elementData synchronization packet, if a particular elementData key has the ignoreClientSync parameter set to true, this synchronization packet would simply be ignored, and the value would remain unchanged.

In my opinion, a single simple argument is a much better solution than having entirely separate functions for this

Ye I agree something like:

bool setElementData ( element theElement, string key, var value [, bool synchronize = true ], [bool acceptClientUpdate = true] )

If synchronize is set to false, acceptClientUpdate should also be set to false.

[PlatinMTA]

From what I understand, the issue is that if we have an elementData key, like "user:points", which we only use in server-side scripts and it’s not synchronized on the server, a potential cheater could still overwrite it by using setElementData client-side with the sync parameter set to true. My suggestion is to add a new server-side argument, something like ignoreClientSync. Then, in the server’s elementData synchronization packet, if a particular elementData key has the ignoreClientSync parameter set to true, this synchronization packet would simply be ignored, and the value would remain unchanged.

In my opinion, a single simple argument is a much better solution than having entirely separate functions for this

What if the elementdata is not set serverside? Should it be ignored by default? I do believe this should be the case but I also think its too late to be making these changes...

[FileEX]

What if the elementdata is not set serverside? Should it be ignored by default? I do believe this should be the case but I also think its too late to be making these changes...

If an element data doesn’t exist on the server side for a given element, like player:vip, the scripter should simply set this element data to false by default rather than leaving it unset. If any element data isn’t used at all on the server side, it doesn’t matter if a potential cheater sets it. Yes, there’s still the issue that a cheater could send a packet manually, bypassing all checks, but I’ve already suggested a potential solution for this to Dutchman. In any case, an argument like acceptClientUpdate would make manipulating element data with executors impossible, as the server would ignore that packet

[wassmas]

From what I understand, the issue is that if we have an elementData key, like "user:points", which we only use in server-side scripts and it’s not synchronized on the server, a potential cheater could still overwrite it by using setElementData client-side with the sync parameter set to true. My suggestion is to add a new server-side argument, something like ignoreClientSync. Then, in the server’s elementData synchronization packet, if a particular elementData key has the ignoreClientSync parameter set to true, this synchronization packet would simply be ignored, and the value would remain unchanged.

In my opinion, a single simple argument is a much better solution than having entirely separate functions for this

Thats why i request for this: #3696

[TheNormalnij]

it doesn’t matter if a potential cheater sets it

Nope. The data will be synced with all clients.

What if the elementdata is not set serverside? Should it be ignored by default?

I'll add a server setting to ignore all unknown element data from clients.

[ffsPLASMA]

Wouldnt it be better to just outright dismiss all client sync updates to an elementdata if the data wasnt shared to client in first place?

eg. setElementData(root, "coolData", "mreow", false)

^doesnt get synced to client, so why not disregard any client updates for said key in first place?

[TheNormalnij]

Wouldnt it be better to just outright dismiss all client sync updates to an elementdata if the data wasnt shared to client in first place?

eg. setElementData(root, "coolData", "mreow", false)

^doesnt get synced to client, so why not disregard any client updates for said key in first place?

It is not enough. Element data may be broadcasted by server only. In this case the element data is shared, but the server deny all changes from client. Whitelisting is the safest option.

[ffsPLASMA]

Wouldnt it be better to just outright dismiss all client sync updates to an elementdata if the data wasnt shared to client in first place?
eg. setElementData(root, "coolData", "mreow", false)
^doesnt get synced to client, so why not disregard any client updates for said key in first place?

It is not enough. Element data may be broadcasted by server only. In this case the element data is shared, but the server deny all changes from client. Whitelisting is the safest option.

I dont quite understand.

When I create an elementdata on server and specifically set it to not get synced to any clients, then the server should not accept/retrieve any updates from client for said key in first place. If I explicitly set an elementdata to get synced to other clients, I can use onElementDataChange to see if said client is allowed to change it or not.

Adding yet another parameter to setElementData to pay attention to and a server config setting is kinda nonsense.

[TheNormalnij]

I agree that local serverside element data may be protected by default.
But broadcasted element data requests protection too

[Dutchman101]

Thanks!
Regarding desirability: the PR has been requested by AC team. You don't have to use it if you don't want to.

[Fernando-A-Rocha]

Cool, script security wiki article should be updated