Skip to content

Added server side weapon related checks #3272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 17, 2023

Conversation

NanoBob
Copy link
Contributor

@NanoBob NanoBob commented Dec 16, 2023

  • Added a check to see if a player has a certain weapon before forwarding a bullet sync packet
  • Clear the weapon related data from a puresync packet if the player doesn't currently have that weapon

The reasoning for both of these changes has to do with people running hacks and/or Lua injectors client sided. In cases like these (which I've verified with a client running such cheats) clients will trigger puresync packets and/or bullet sync packets for weapons the player doesn't actually have.

While the server doesn't process these puresync packets, they still relay them to other players. Meaning these weapons are visible to other players (even though according to the server the player will not have the weapons).

For the bullet sync packet the server would blindly forward them, meaning a compromised client would be able to fire any weapon, without even having that weapon.

This PR aims to add two relatively simple checks to prevent this from happening.

…g, and don't send potentially incorrect weapon data
@PlatinMTA
Copy link
Contributor

So that was the problem all this time along. Lets hope this gets merged asap.

Copy link
Member

@Lpsd Lpsd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm :shipit:

@Lpsd Lpsd merged commit 86448ea into multitheftauto:master Dec 17, 2023
MTABot pushed a commit that referenced this pull request Dec 17, 2023
86448ea Added server side weapon related checks (#3272)
11c04b7 Bump build [ci skip]
20e3e9c Bump build [ci skip]
8d2ddea Bump build [ci skip]
7b1fa4a Bump build [ci skip]
4e6a4ec Bump build [ci skip]
4c7a72c Bump build [ci skip]
f23413a Update installer en_US pot
0923cc9 Update client en_US pot
@NanoBob NanoBob deleted the feature/weapon-sanity-checks branch December 17, 2023 01:18
@StrixG StrixG added the bugfix Solution to a bug of any kind label Dec 21, 2023
@StrixG StrixG added this to the 1.6.1 milestone Dec 21, 2023
@PlatinMTA
Copy link
Contributor

PlatinMTA commented Dec 21, 2023

https://streamable.com/uc3250

Seems to keep happening to some extent. Version r22335 (Linux x64).

Useful information:

  • The server knows the amount of bullets the user has in the slot, but returns 0 as a weapon.
  • The server does not know when the player is shooting ("onPlayerWeaponFire" is not working, probably because weapon == 0).

That's about it.

@Pirulax
Copy link
Contributor

Pirulax commented Dec 29, 2023

What about explosions/damage caused by those shots (rpgs for example)?
We should stop those from being relayed too (if not already, not sure how the weapon sync works).

@@ -45,7 +45,7 @@ jobs:
- name: Create build artifacts
run: utils\premake5 compose_files

- uses: actions/upload-artifact@master
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this intentional?

@@ -175,6 +175,12 @@ bool CPlayerPuresyncPacket::Read(NetBitStreamInterface& BitStream)
// Set weapon slot
if (bWeaponCorrect)
pSourcePlayer->SetWeaponSlot(uiSlot);
else
{
// remove invalid weapon data to prevent this from being relayed to other players
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nice to add a comment explaining what could cause this (Cheaters, etc).

@@ -2443,6 +2443,10 @@ void CGame::Packet_Bulletsync(CBulletsyncPacket& Packet)
CPlayer* pPlayer = Packet.GetSourcePlayer();
if (pPlayer && pPlayer->IsJoined())
{
// Early return when the player attempts to fire a weapon they do not have
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nice to add a comment explaining what could cause this (Cheaters, etc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Solution to a bug of any kind
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants