[Vulnerability/Bug] simulateSeed is set by client

[ljars]

In DefaultCharacterUseSkillComponent.UseSkill(), simulateSeed is randomly chosen by the client: On the server, ReadClientUseSkillStateAtServer() reads the seed, and it gets used in UseSkillRoutine().

A cheating client could manipulate the seed that is sent to gain an advantage. For example, the seed is used to compute random damage in DefaultGameplayRule.RandomAttackDamage(). The attacker could find a seed that gives good damage and send that seed every time.

The same problem exists in DefaultCharacterUseSkillComponent.UseItemSkill(), and DefaultCharacterAttackComponent.Attack().

The seed should chosen by the server so the client can't cheat.

[insthync]

Seems like you have knowledge, can you help me to improve it?, you may send the codes which you think it is working to me.

[ljars]

The simplest solution would be to overwrite the randomSeed parameter from DamageableEntity.ApplyDamage(), and generate a new seed (this function happens on the server only). This is the only code change (It would be cleaner to delete the parameter and edit all of its usages, though):

This will prevent cheaters from controlling the damage randomization. However, it won't prevent them from controlling the random attack animation or the random fire stagger.

Oddly, it looks like random attack animation for non-skill attacks is not synchronized at all, since it uses Random.Range() without using the seed. So the attack animation will be different on different clients. (This is in DefaultCharacterAttackComponent.AttackRoutine())

For skill attacks, though, it uses the seed and GetRandomAttackAnimation() to pick the animation. So the animation will be synchronized on all clients, but it will be hackable by the client who does the attack. (This is in DefaultCharacterUseSkillComponent.UseSkillRoutine())

Hacking animation is not as bad as hacking damage. However, some animations may be shorter and have more hits, so a hacker would prefer to use that animation every time. Preventing this hack will be more complicated, though.

[ljars]

If we had the server generate the seed and send it to the client for each attack, then we couldn't do client-prediction because we would have to wait a whole RTT to get the seed from the server before we know what attack animation to play.

But maybe there is a solution where the server sends a seed to the client one time when they connect. Whenever the client attacks or uses a skill, the client and server both increment their shared seed by 1. This way, the client and server are always using a matching seed, but the client can't manipulate the seed because the server independently knows the seed.

Cons

• There would need to be a way to recover if a dropped packet caused the client and server seeds to be out of sync.
• The hacker can't control what animation it will get, but the hacker can still know in advance what animations it will get.

[insthync]

The animation index random codes is incorrect, I will change it to

                animationIndex = GenericUtils.RandomInt(simulateSeed, 0, randomMax);

[insthync]

mmorpg_kit-attackanduseskillhackprevention-230823.zip

You may help me review this one

[insthync]

I intend to use simulate-seed just to make clients play an animations, firing, and so on in the same way with server and other clients). I've changed it, now server will generate new simulate-seed to all clients, then it will re-generate when player do an actions, client may play the wrong animation/firing to a bit difference direction if they are lag and didn't receive new simulate-seed before do an actions.

[ljars]

I took a look at the code. It looks like it will work well for melee combat, where there is plenty of time between attacks for the next seed to be synced.

The only problem I see is with high fire rate attacks. When shooting the machine gun, AttackRoutine() may be called 10 times per second (100 ms apart). If the RTT isn't always below 100 ms, some of those shots will use an outdated seed.

Here is my idea for an incremented seed. I only implemented it for DefaultCharacterAttackComponent, but I can implement it for DefaultCharacterUseSkillComponent too. DefaultCharacterAttackComponent.zip [EDIT: Deleted and re-posted because I had a bug in my code]

Description: In EntityStart(), the server chooses a random seed and sends it to the owner client via RPC. The owner client uses that seed for its first action, but increments it by 1 each time it does an action. The owner client sends the seed it uses to the server in WriteClientAttackState() when doing an action. In ReadClientAttackStateAtServer() the server checks if the seed sent by the client matches its own incremented seed. It calculates the difference between the client's seed and its own seed (_clientSeedSkew) in case the client's seed is wrong. The server uses its own incremented seed for simulating. The server sends its own incremented seed and the seed skew to all the clients in WriteServerAttackState(). In ReadServerAttackStateAtClient(), non-owner clients use the server's seed to simulate, and the owner client uses the seed skew to correct its own seed if it's wrong.

This approach doesn't require the network round trip to happen before doing the next attack, because the client just increments the seed to get the next seed. So the client could send state for multiple machine gun shots, all using correctly incremented seeds, before the server responds. The seed incrementing getting skewed should be rare since the delivery method is ReliableOrdered, but this method still allows the owner client to recover in case it does get skewed.

[insthync]

I've changed to use synced timestamp as a seed, you can check it it 1.85 which I will release today.