sa-mp-fixes

sa-mp-fixes copied to clipboard

Some time ago, crashes related to "Array index out of bounds / Attempted to read/write array element at negative index -400" appeared in the server logs. After some investigation, the problem was caused by the following code:

new ac_a = GetPlayerSurfingVehicleID(playerid);
if(!IsVehicleStreamedIn(ac_a, playerid) || !IsVehicleSurfable(ac_a)) //<- crash indicated here #2 (chain crash)
{
    //Do the things...
}

IsVehicleSurfable(vehicleid) return VehArray[GetVehicleModel(vehicleid) - 400][vSurfers];  //<- crash indicated here #1 (the real place of crash)

So, you can already see that crash is occuring due to lack of checks for vehicle validity in "IsVehicleSurfable" as GetVehicleModel is obviously returning 0, and after some math it becomes -400 which fully fits the crashlog description.

Yay, the problem is detected and can easily be solved by just adding "IsValidVehicle" before we get & put the model as an array index. But wait, we already have "IsVehicleStreamedIn" before the other checks. Isn't it should also protect us from such behaviour by internal validation checks? So, after some searches I'm completely sure that it is not doing it properly. As an indirect confirmation I found some implementations of these functions based on open-samp (which ofc cannot be a reliable source, however it only confirms this): https://imgur.com/XkmmZDt

As seen, there are only base checks for validity there (the vehicle ID isn't less than 1 && no more than MAX_VEHICLES - 1, but nothing about whether it exists on the server).