fivem icon indicating copy to clipboard operation
fivem copied to clipboard

State bags set on the server do not respect replicated flag

Open jxckUK opened this issue 1 year ago • 6 comments

What happened?

As reported originally back in 2021 by @AvarianKnight here https://forum.cfx.re/t/server-side-entity-state-bags-sync-when-replicate-is-false/2313177 and seems it was never opened here.

I ran into this issue and did some research and found the original report.

I would argue that this is a security issue because currently when scripting developers are setting this value to be false there could be an expectation that its working correctly, therefore data stored with the intent to not replicate in fact is replicating which would be a data security issue.

Expected result

It should respect the replicated flag being set to false and not replicate the state to the clients.

Reproduction steps

  1. Create an entity or player state server side and set the replicated flag to false.
  2. Query the state client side and see that the data is replicated to other players when entering their scope.

There is also a reproduction script in @AvarianKnight's original report.

Importancy

Security issue

Area(s)

OneSync

Specific version(s)

FiveM, Server 7752, Linux

Additional information

No response

jxckUK avatar Mar 27 '24 15:03 jxckUK

Simply using:

Player(source).state:set("test", "hi", false)

works as expected and does not replicate the state bag value to any client. (Same for entity state bags)

So there have to be other factors to make this not work as expected.

tens0rfl0w avatar Mar 27 '24 19:03 tens0rfl0w

iirc this requires you to send a replicated value afterwards and/or re-enter the entities/player scope.

The underlying implementation for state bags doesn't have anything to respect the replication flag sent by the caller after the first call

https://github.com/citizenfx/fivem/blob/b5ed6d642b65e83bbcdc9e64af2b5494f2f1255a/code/components/citizen-scripting-core/src/ResourceScriptFunctions.cpp#L320-L340

https://github.com/citizenfx/fivem/blob/b5ed6d642b65e83bbcdc9e64af2b5494f2f1255a/code/components/citizen-resources-core/src/StateBagComponent.cpp#L175-L178

https://github.com/citizenfx/fivem/blob/b5ed6d642b65e83bbcdc9e64af2b5494f2f1255a/code/components/citizen-resources-core/src/StateBagComponent.cpp#L234-L269

AvarianKnight avatar Mar 27 '24 19:03 AvarianKnight

Looking at @AvarianKnight's repro from the initial issue: Replication state does seem to get ignored when calling a 'set' operation on entities too fast after init creation.

Simply waiting a few ticks after entity creation correctly applies the replication state to the state bag key.

For example, waiting 100 ticks after calling CreateAutomobile and another 100 ticks after the two 'set' operations then correctly replicates the state bag values: (second wait is just to ensure values got synced to the client) image

(This also works if setting a replicated state bag key on the same entity afterwards.)

tens0rfl0w avatar Mar 27 '24 20:03 tens0rfl0w

This ties into the original issue of

The underlying implementation for state bags doesn't have anything to respect the replication flag sent by the caller after the first call

When the state bag is initially created it doesn't send the updates to anyone until the client has said that it has the entity created, this will call AddRoutingTarget which will sync all the key/values to the client (without respect for the replication flag)

https://github.com/citizenfx/fivem/blob/b5ed6d642b65e83bbcdc9e64af2b5494f2f1255a/code/components/citizen-server-impl/src/state/ServerGameState.cpp#L1815-L1826

AvarianKnight avatar Mar 27 '24 20:03 AvarianKnight

Verified exactly what you just described: Replication state is only applied when the client is in scope of the entity/the entity already exists on the client before the 'set' operation is executed.👍🏼Never noticed that before.

tens0rfl0w avatar Mar 27 '24 20:03 tens0rfl0w

Thanks both for the much more detailed insight into the issue than I would have been able to provide!

and/or re-enter the entities/player scope.

This particular issue is the one that alerted me to the problem, delaying the set operation in this instance won't provide a valid mitigation (at least for Player state) because the players within a users scope is subject to change at any time.

I assume this doesn't affect states set on the client side which are set to not replicate by default? Or are they also affected by the underlying implementation not respecting replication flags post creation/first call?

jxckUK avatar Mar 27 '24 21:03 jxckUK