multitheftauto / mtasa-blue

Multi Theft Auto is a game engine that incorporates an extendable network play element into a proprietary commercial single-player game.
https://multitheftauto.com
GNU General Public License v3.0
1.4k stars 431 forks source link

onClientVehicleEnter requires connection with server causing possibility to drive cars #1130

Open MrDadosz opened 4 years ago

MrDadosz commented 4 years ago

Describe the bug Clientside onClientVehicleEnter requires connection with server. If player try to enter car and disconnect his internet cable, he can drive cars even if your clientside script is turning off engine. This can cause problems for RPG servers, where every player has his own car and can enter open cars without freeze - he can move them without permission.

To reproduce

  1. use this script:
    addEventHandler("onClientVehicleEnter", root, function(player, seat)
    if localPlayer ~= player or seat ~= 0 then
    return
    end
    outputChatBox("off")
    setVehicleEngineState(source, false)
    setTimer(function(vehicle)
    outputChatBox("off timer")
    setVehicleEngineState(vehicle, false)
    end, 100, 10, source)
    end)
  2. click F to enter a car
  3. disconnect your internet cable when your character is running to vehicle
  4. ped will enter the vehicle and engine will turn on, now you can move every car that isn't frozen
  5. after connecting your internet cable, you will see "off" and "off timer" on the chat

Expected behaviour Engine sould turn off.

Version Multi Theft Auto v1.5.7-release-20288 - client MTA:SA Server v1.5.7-release-18957 - server

qaisjp commented 4 years ago

I'd recommend you to also set the vehicle engine state serverside. Never trust the client!

MrDadosz commented 4 years ago

Yeah, I did it, but when you start entering a car and you unplug your internet, onVehicleEnter won't trigger and you can't even block it clientside. I have another suggestion how to fix it - in server config could be start_vehicle_engine_on_enter default set to true determining if engine will start when you enter a car. Most of gamemodes other than race and freeroam would set it to false. It could be fixed now by getPedSimplest task attached to onClientRender - when it changes first time to entering a car, then script could get player's vehicle and turn off engine. In my opinion that event should work without connection with a server cause you could steal other players cars, destroy them etc.

jessejeremias commented 4 years ago

when you start entering a car

I noticed the above when reading your comment. Perhaps it is an idea to use onClientVehicleStartEnter instead, then verify server-side and send a message back to client-side.

https://wiki.multitheftauto.com/wiki/OnClientVehicleStartEnter

Lpsd commented 4 years ago

This is really a non-issue as it can be solved by verification on the server, rather than client.

MrDadosz commented 4 years ago

It is a issue - you can drive every car and you can't block it by clientside and serverside. When you start entering a vehicle and you disconnect your internet cable, server events won't work because on the server your character will stay in one place. You can't even block it clientside with onClientVehicleStartEnter and onClientVehicleEnter because events will trigger AFTER you connect again your internet cable - after you move car with running engine by GTA - engine will be disabled after ~5 seconds of driving a car. I'll put today resource and video explaining why it's a problem.

MrDadosz commented 4 years ago

https://youtu.be/R1y29W8EBkM mta-engine_bug.zip

Lpsd commented 4 years ago

You can use the onClientPlayerNetworkStatus event to implement safeguards for when a client loses connection to the server

MrDadosz commented 4 years ago
local networkTimer = nil
local function networkStatusTimer()
    local vehicle = getPedOccupiedVehicle(localPlayer)
    local vehicleSeat = getPedOccupiedVehicleSeat(localPlayer)
    if vehicle and vehicleSeat and vehicleSeat == 0 then
        setVehicleEngineState(vehicle, false)
    end
end

addEventHandler("onClientPlayerNetworkStatus", localPlayer, function(status, ticks)
    if status == 0 then
        if isTimer(networkTimer) then
            killTimer(networkTimer)
        end
        networkTimer = setTimer(networkStatusTimer, 500, 0)
    else
        if isTimer(networkTimer) then
            killTimer(networkTimer)
        end
    end
end)

Works fine but in my opinion should be fixed in MTA or at least adding information on the wiki.

qaisjp commented 4 years ago

Copying my text from Discord:

I think adding functionality to MTA that disables the engine when you enter vehicles would be a hacky solution.

As Caz said (#80), entering vehicles is a hippity hop back and forth of packets. It seems that we're not enforcing that, and we should probably enforce it.