Skip to content
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

Exploitable Behavior with ADD_OWNED_EXPLOSION #2326

Closed
btwlouis opened this issue Jan 2, 2024 · 21 comments
Closed

Exploitable Behavior with ADD_OWNED_EXPLOSION #2326

btwlouis opened this issue Jan 2, 2024 · 21 comments
Labels

Comments

@btwlouis
Copy link

btwlouis commented Jan 2, 2024

What happened?

Bug Description:

Currently, there is a vulnerability in the handling of ADD_OWNED_EXPLOSION in various mod menus. This allows the abuse of the explosionEvent event, resulting in a manipulated sender field.

Here is a snippet of the data in the explosionEvent:

{
   "cameraShake": 0.69291341304779,
   "f164": 539292904,
   "isAudible": true,
   "posY224": 0,
   "unkX": 0,
   "f216": false,
   "f243": true,
   "damageScale": 1,
   "explosionType": 7,
   "unkY": 0,
   "posZ": -0.2144775390625,
   "f214": 0,
   "f189": false,
   "f186": 17,
   "f218": 0,
   "isInvisible": false,
   "f240": false,
   "posX": 0.29003921151161,
   "posX224": 0,
   "f104": 34,
   "f191": false,
   "f242": false,
   "unkZ": 0.99999397993087,
   "f210": 8189,
   "f241": false,
   "posY": -1.37109446525573,
   "f126": false,
   "ownerNetId": 31,
   "f208": 8189,
   "f190": false,
   "posZ224": 0
}

Consider implementing a solution similar to the network sounds handling (sv_enableNetworkedSounds variable). Introduce a variable to control the routing of owned explosions, preventing the manipulation of the sender field. (sv_enableNetworkedExplosions)

Expected result

The sender in the explosionEvent should accurately represent the entity responsible for the explosion, without being susceptible to manipulation or abuse.

Reproduction steps

  1. Trigger ADD_OWNED_EXPLOSION using various mod menus / runcode.
  2. Observe the explosionEvent data.
  3. Notice that the sender is being manipulated or "spoofed."

Importancy

Security issue

Area(s)

FXServer

Specific version(s)

FXServer-master SERVER v1.0.0.6683 win32

Additional information

Thank you for your attention to this matter. If you need further clarification or details, please don't hesitate to ask.

@btwlouis btwlouis added bug triage Needs a preliminary assessment to determine the urgency and required action labels Jan 2, 2024
@AvarianKnight
Copy link
Contributor

Why are you using the sender from the event at all? The events all pass the client source (as a string) of the person who actually called the event, and that cannot be "spoofed".

Also is it a weird coincidence that this passes 31, which is a dummy player on the client

@btwlouis
Copy link
Author

btwlouis commented Jan 2, 2024

The sender in the event is manipulated, and in server events, the "source" is not included, being nil when printed.

----------------------- [explosionEvent] -----------------------
Name	nil
RegisterNetEvent("explosionEvent", function(sender, data) -- sender getting spoofed
    print("----------------------- [explosionEvent] -----------------------")
    print("Name", GetPlayerName(source)) -- returns Name nil
    print(json.encode(data))
end)

@AvarianKnight
Copy link
Contributor

Do you have a way to reproduce this?

@btwlouis
Copy link
Author

btwlouis commented Jan 2, 2024

Nothing to reproduce except the print of the data parameter. Is currently done with a mod menu called “Lumia”.

@tens0rfl0w
Copy link
Contributor

tens0rfl0w commented Jan 2, 2024

the "source" is not included, being nil when printed.

sender is the source here, and as you're printing GetPlayerName(source) this ofc is undefined.

Also this shouldn't be defined as a net-event (use AddEventHandler instead), as this event isn't passed by the client and rather gets parsed by the server.

@AvarianKnight
Copy link
Contributor

the "source" is not included, being nil when printed.

sender is the source here, and as you're printing GetPlayerName(source) this ofc is undefined.

I did not notice that

@btwlouis
Copy link
Author

btwlouis commented Jan 2, 2024

Also this shouldn't be defined as a net-event (use AddEventHandler instead).

Of course not as a net event, I just tried it for testing purposes but the source is not sent to show it @AvarianKnight

@AvarianKnight
Copy link
Contributor

source isn't defined because this isn't a net event, try using the sender like @tens0rfl0w said.

@btwlouis
Copy link
Author

btwlouis commented Jan 2, 2024

The sender is the one being exploded (the victim from the modder). That's what's being manipulated. In the sender, I'm listed when I'm the one being exploded, even though I didn't trigger it. The manipulation occurs through AddOwnedExplosion, where the target ped is specified to manipulate the sender.

@tens0rfl0w
Copy link
Contributor

I don't see how that adds up, as sender isn't part of the parsed package data (msgNetGameEvent) and is in fact the peer-id of the sending-client. Trying to reproduce this with "normal" code-execution capabilities failed to show any result where sender is somehow not the peer-id of the client that invoked this native.

@btwlouis
Copy link
Author

btwlouis commented Jan 2, 2024

Server console:

----------------------- [explosionEvent] -----------------------
Name	Louis
{"posX":0.85693401098251,"f208":8191,"f210":8191,"f218":0,"isInvisible":false,"posZ":1.02783203125,"f242":false,"f240":false,"unkX":0,"f191":false,"posY":0.46142601966857,"f243":true,"posY224":0,"f214":0,"cameraShake":0.69291341304779,"posX224":0,"f164":539292904,"f126":true,"posZ224":0,"f241":false,"f186":1,"f104":34,"f216":false,"f189":false,"damageScale":1,"unkZ":0.99999397993087,"ownerNetId":0,"f190":false,"isAudible":true,"unkY":0,"explosionType":7}
true
0
8191
AddEventHandler("explosionEvent", function(sender, data)
    print("----------------------- [explosionEvent] -----------------------")
    print("Name", GetPlayerName(sender))
    print(json.encode(data))

    if data.ownerNetId == 0 then
        CancelEvent()
    end

    if data.posX == 0.0 and data.posY == 0.0 then
        CancelEvent()
    end

    local victim = NetworkGetEntityFromNetworkId(data.ownerNetId)
    victim = NetworkGetEntityOwner(victim)


    local explodePostion = vec3(data.posX, data.posY, data.posZ)
    local positionNearByZero = #(explodePostion - vec3(0, 0, 0)) < 3.0

    print(positionNearByZero)
    print(GetVehiclePedIsIn(GetPlayerPed(sender), false))
    print(data.f210)

Cheater execute following:

local playerIdx = GetPlayerFromServerId(16)
local ped = GetPlayerPed(playerIdx)

local coords = GetEntityCoords(ped)
print(coords)
AddOwnedExplosion(ped, coords.x, coords.y, coords.z, 7, 1.0, true, false, 0.6)

@btwlouis
Copy link
Author

btwlouis commented Jan 2, 2024

The only thing I notice so far is that the explosions are all near vector3(0.0, 0.0, 0.0).

@tens0rfl0w
Copy link
Contributor

local playerIdx = GetPlayerFromServerId(16)

And the sender is in fact 16 and not the server-id of the executing client, or no?

@btwlouis
Copy link
Author

btwlouis commented Jan 2, 2024

Correct, 16 is my id and is equal to sender.

@tens0rfl0w
Copy link
Contributor

I still cannot reproduce this with your provided code, executing this on a remotely-owned player ped still returns the peer-id of the sending client and not of the client that owns the ped.

For clarification:
Targeting the player-ped of the client with id 2 from the client with id 1 returns 1 as the event sender.

@btwlouis
Copy link
Author

btwlouis commented Jan 2, 2024

Then I think there's more built in there that you can't see. The executor with which the code was executed is called redEngine.

@btwlouis
Copy link
Author

btwlouis commented Jan 2, 2024

I can only repeat myself and say that the player ID that is specified in the sender for the explosionEvent is the one that was specified natively as the target ped for the AddOwnedExplosion.

@AvarianKnight
Copy link
Contributor

It is impossible that the sender argument is wrong here as the client has no control over it so they can't spoof it, unlike everything else reported by the data argument which is reported by the sender client

In newer game builds (tested on b3095 and b2545) calling this native on a remote player results in a network bail out, resulting in the msgNetGameEvent never getting sent to the server and the player getting stuck in a "broken" state where anything networking related via base GTA doesn't work.

Here's an example where the player no longer syncs its coordinates, weapons, or anything else sync node related.

Here's what client 2 sees (the one that was the target of this native)

Here's what client 1 sees (the one who called ADD_OWNED_EXPLOSION on Client 2)

@tens0rfl0w
Copy link
Contributor

tens0rfl0w commented Jan 3, 2024

This check has even been present since the initial release of GTA:O to avoid such abuse in MP sessions (BAIL_CTX_NETWORK_ERROR_CHEAT_DETECTED). However, there seems to be a bug in the bailing out logic where future explosions do get passed through if the initial bail out didn't work, but this still shouldn't return the wrong peer-id from the network event.

Did you verify the "cheater" used this specific native and not something like ADD_VEHICLE_PHONE_EXPLOSIVE_DEVICE which got fixed in 0fa9842?

Maybe try FXServer version 6821 or higher to see if that fixes your issue.

@btwlouis
Copy link
Author

btwlouis commented Jan 3, 2024

Did you verify the "cheater" used this specific native and not something like ADD_VEHICLE_PHONE_EXPLOSIVE_DEVICE

No, it's exactly the code I posted above. I'll make a video later

@btwlouis
Copy link
Author

btwlouis commented Jan 3, 2024

I think it solved by using sv_enableNetworkedPhoneExplosions but it's weird that this code also works for that "bypass"

@btwlouis btwlouis closed this as completed Jan 7, 2024
@github-actions github-actions bot removed the triage Needs a preliminary assessment to determine the urgency and required action label Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants