From 7d967ad12a6c2f8c33b662a47ad9845b359c83e2 Mon Sep 17 00:00:00 2001 From: bjarni Date: Sun, 30 Jan 2005 20:50:06 +0000 Subject: (svn r1741) - Fix: added IsVehicleIndex() so it's possible to protect GetVehicle() from reading an invalid vehicle index - Fix: added check for v->type in some commands, which expects v to be a specific type Checks like this is needed to protect network servers from people, who hack their clients to either cheat or crash the server NOTE: if I made a mistake here it can make a function unreachable when it should be used. Here is one place to look if something weird happens --- aircraft_cmd.c | 24 +++++++++++++++++++----- roadveh_cmd.c | 16 ++++++++++++++-- ship_cmd.c | 24 ++++++++++++++++++------ train_cmd.c | 42 ++++++++++++++++++++++++++++++++---------- vehicle.c | 12 ++++++++---- vehicle.h | 8 ++++++++ 6 files changed, 99 insertions(+), 27 deletions(-) diff --git a/aircraft_cmd.c b/aircraft_cmd.c index b66d0d6e7..f853b088a 100644 --- a/aircraft_cmd.c +++ b/aircraft_cmd.c @@ -360,13 +360,15 @@ int32 CmdSellAircraft(int x, int y, uint32 flags, uint32 p1, uint32 p2) { Vehicle *v; - SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES); + if (!IsVehicleIndex(p1)) return CMD_ERROR; v = GetVehicle(p1); if (v->type != VEH_Aircraft || !CheckOwnership(v->owner) || !CheckStoppedInHangar(v)) return CMD_ERROR; + SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES); + if (flags & DC_EXEC) { // Invalidate depot InvalidateWindow(WC_VEHICLE_DEPOT, v->tile); @@ -385,9 +387,11 @@ int32 CmdStartStopAircraft(int x, int y, uint32 flags, uint32 p1, uint32 p2) { Vehicle *v; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); - if (!CheckOwnership(v->owner)) + if (v->type != VEH_Aircraft || !CheckOwnership(v->owner)) return CMD_ERROR; // cannot stop airplane when in flight, or when taking off / landing @@ -417,9 +421,11 @@ int32 CmdSendAircraftToHangar(int x, int y, uint32 flags, uint32 p1, uint32 p2) Station *st; uint16 next_airport_index; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); - if (!CheckOwnership(v->owner)) + if (v->type != VEH_Aircraft || !CheckOwnership(v->owner)) return CMD_ERROR; if (HASBIT(p2, 16)) v->set_for_replacement = true; //now all clients knows that the vehicle wants to be replaced @@ -465,9 +471,11 @@ int32 CmdChangeAircraftServiceInt(int x, int y, uint32 flags, uint32 p1, uint32 { Vehicle *v; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); - if (!CheckOwnership(v->owner)) + if (v->type != VEH_Aircraft || !CheckOwnership(v->owner)) return CMD_ERROR; if (flags & DC_EXEC) { @@ -490,13 +498,19 @@ int32 CmdRefitAircraft(int x, int y, uint32 flags, uint32 p1, uint32 p2) byte new_cargo_type = p2 & 0xFF; //gets the cargo number AircraftVehicleInfo *avi; - SET_EXPENSES_TYPE(EXPENSES_AIRCRAFT_RUN); + if (!IsVehicleIndex(p1)) return CMD_ERROR; v = GetVehicle(p1); + + if (v->type != VEH_Aircraft) return CMD_ERROR; + avi = AircraftVehInfo(v->engine_type); + if (!CheckOwnership(v->owner) || (!CheckStoppedInHangar(v) && !(SkipStoppedInHangerCheck))) return CMD_ERROR; + SET_EXPENSES_TYPE(EXPENSES_AIRCRAFT_RUN); + switch (new_cargo_type) { case CT_PASSENGERS: pass = avi->passenger_capacity; diff --git a/roadveh_cmd.c b/roadveh_cmd.c index 1ad6d7104..21b28c41a 100644 --- a/roadveh_cmd.c +++ b/roadveh_cmd.c @@ -209,6 +209,8 @@ int32 CmdStartStopRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2) { Vehicle *v; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); if (v->type != VEH_Road || !CheckOwnership(v->owner)) @@ -229,13 +231,15 @@ int32 CmdSellRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2) { Vehicle *v; - SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES); + if (!IsVehicleIndex(p1)) return CMD_ERROR; v = GetVehicle(p1); if (v->type != VEH_Road || !CheckOwnership(v->owner)) return CMD_ERROR; + SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES); + if (!IsRoadDepotTile(v->tile) || v->u.road.state != 254 || !(v->vehstatus&VS_STOPPED)) return_cmd_error(STR_9013_MUST_BE_STOPPED_INSIDE); @@ -307,9 +311,13 @@ static int FindClosestRoadDepot(Vehicle *v) bit 2 = clear v->set_for_replacement */ int32 CmdSendRoadVehToDepot(int x, int y, uint32 flags, uint32 p1, uint32 p2) { - Vehicle *v = GetVehicle(p1); + Vehicle *v; int depot; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + + v = GetVehicle(p1); + if (v->type != VEH_Road || !CheckOwnership(v->owner)) return CMD_ERROR; @@ -348,6 +356,8 @@ int32 CmdTurnRoadVeh(int x, int y, uint32 flags, uint32 p1, uint32 p2) { Vehicle *v; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); if (v->type != VEH_Road || !CheckOwnership(v->owner)) @@ -373,6 +383,8 @@ int32 CmdChangeRoadVehServiceInt(int x, int y, uint32 flags, uint32 p1, uint32 p { Vehicle *v; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); if (v->type != VEH_Road || !CheckOwnership(v->owner)) diff --git a/ship_cmd.c b/ship_cmd.c index 0bab059ba..dec18586f 100644 --- a/ship_cmd.c +++ b/ship_cmd.c @@ -915,13 +915,15 @@ int32 CmdSellShip(int x, int y, uint32 flags, uint32 p1, uint32 p2) { Vehicle *v; - SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES); + if (!IsVehicleIndex(p1)) return CMD_ERROR; v = GetVehicle(p1); if (v->type != VEH_Ship || !CheckOwnership(v->owner)) return CMD_ERROR; + SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES); + if (!IsShipDepotTile(v->tile) || v->u.road.state != 0x80 || !(v->vehstatus&VS_STOPPED)) return_cmd_error(STR_980B_SHIP_MUST_BE_STOPPED_IN); @@ -943,9 +945,11 @@ int32 CmdStartStopShip(int x, int y, uint32 flags, uint32 p1, uint32 p2) { Vehicle *v; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); - if (!CheckOwnership(v->owner)) + if (v->type != VEH_Ship || !CheckOwnership(v->owner)) return CMD_ERROR; if (flags & DC_EXEC) { @@ -969,9 +973,11 @@ int32 CmdSendShipToDepot(int x, int y, uint32 flags, uint32 p1, uint32 p2) Vehicle *v; int depot; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); - if (!CheckOwnership(v->owner)) + if (v->type != VEH_Ship || !CheckOwnership(v->owner)) return CMD_ERROR; if (HASBIT(p2, 0)) v->set_for_replacement = true; @@ -1007,9 +1013,11 @@ int32 CmdChangeShipServiceInt(int x, int y, uint32 flags, uint32 p1, uint32 p2) { Vehicle *v; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); - if (!CheckOwnership(v->owner)) + if (v->type != VEH_Ship || !CheckOwnership(v->owner)) return CMD_ERROR; if (flags & DC_EXEC) { @@ -1031,10 +1039,12 @@ int32 CmdRefitShip(int x, int y, uint32 flags, uint32 p1, uint32 p2) byte SkipStoppedInDepotCheck = (p2 & 0x100) >> 8; //excludes the cargo value p2 = p2 & 0xFF; - SET_EXPENSES_TYPE(EXPENSES_SHIP_RUN); + + if (!IsVehicleIndex(p1)) return CMD_ERROR; v = GetVehicle(p1); - if (!CheckOwnership(v->owner)) + + if (v->type != VEH_Ship || !CheckOwnership(v->owner)) return CMD_ERROR; if (!( SkipStoppedInDepotCheck )) { @@ -1044,6 +1054,8 @@ int32 CmdRefitShip(int x, int y, uint32 flags, uint32 p1, uint32 p2) return_cmd_error(STR_980B_SHIP_MUST_BE_STOPPED_IN); } + SET_EXPENSES_TYPE(EXPENSES_SHIP_RUN); + cost = 0; if (IS_HUMAN_PLAYER(v->owner) && (byte)p2 != v->cargo_type) { cost = _price.ship_base >> 7; diff --git a/train_cmd.c b/train_cmd.c index b0951b53e..9c176c8df 100644 --- a/train_cmd.c +++ b/train_cmd.c @@ -719,7 +719,10 @@ int32 CmdMoveRailVehicle(int x, int y, uint32 flags, uint32 p1, uint32 p2) Vehicle *src, *dst, *src_head, *dst_head; bool is_loco; + if (!IsVehicleIndex(p1 & 0xFFFF)) return CMD_ERROR; + src = GetVehicle(p1 & 0xFFFF); + if (src->type != VEH_Train) return CMD_ERROR; is_loco = !(RailVehInfo(src->engine_type)->flags & RVI_WAGON) @@ -864,9 +867,11 @@ int32 CmdStartStopTrain(int x, int y, uint32 flags, uint32 p1, uint32 p2) { Vehicle *v; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); - if (!CheckOwnership(v->owner)) + if (v->type != VEH_Train || !CheckOwnership(v->owner)) return CMD_ERROR; if (flags & DC_EXEC) { @@ -888,13 +893,15 @@ int32 CmdSellRailWagon(int x, int y, uint32 flags, uint32 p1, uint32 p2) Vehicle *v, *first,*last; int32 cost; - SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES); + if (!IsVehicleIndex(p1)) return CMD_ERROR; v = GetVehicle(p1); if (v->type != VEH_Train || !CheckOwnership(v->owner)) return CMD_ERROR; + SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES); + // get first vehicle in chain first = v; if (first->subtype != TS_Front_Engine) { @@ -1172,9 +1179,11 @@ int32 CmdReverseTrainDirection(int x, int y, uint32 flags, uint32 p1, uint32 p2) { Vehicle *v; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); - if (!CheckOwnership(v->owner)) + if (v->type != VEH_Train || !CheckOwnership(v->owner)) return CMD_ERROR; _error_message = STR_EMPTY; @@ -1201,9 +1210,11 @@ int32 CmdForceTrainProceed(int x, int y, uint32 flags, uint32 p1, uint32 p2) { Vehicle *v; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); - if (!CheckOwnership(v->owner)) + if (v->type != VEH_Train || !CheckOwnership(v->owner)) return CMD_ERROR; if (flags & DC_EXEC) @@ -1225,12 +1236,15 @@ int32 CmdRefitRailVehicle(int x, int y, uint32 flags, uint32 p1, uint32 p2) p2 = p2 & 0xFF; - SET_EXPENSES_TYPE(EXPENSES_TRAIN_RUN); + if (!IsVehicleIndex(p1)) return CMD_ERROR; v = GetVehicle(p1); - if (!CheckOwnership(v->owner) || ((CheckStoppedInDepot(v) < 0) && !(SkipStoppedInDepotCheck))) + + if (v->type != VEH_Train || !CheckOwnership(v->owner) || ((CheckStoppedInDepot(v) < 0) && !(SkipStoppedInDepotCheck))) return CMD_ERROR; + SET_EXPENSES_TYPE(EXPENSES_TRAIN_RUN); + cost = 0; num = 0; @@ -1341,10 +1355,14 @@ static TrainFindDepotData FindClosestTrainDepot(Vehicle *v) bit 2 = clear v->set_for_replacement */ int32 CmdTrainGotoDepot(int x, int y, uint32 flags, uint32 p1, uint32 p2) { - Vehicle *v = GetVehicle(p1); + Vehicle *v; TrainFindDepotData tfdd; - if (!CheckOwnership(v->owner)) + if (!IsVehicleIndex(p1)) return CMD_ERROR; + + v = GetVehicle(p1); + + if (v->type != VEH_Train || !CheckOwnership(v->owner)) return CMD_ERROR; if (HASBIT(p2, 0)) v->set_for_replacement = true; @@ -1387,9 +1405,13 @@ int32 CmdTrainGotoDepot(int x, int y, uint32 flags, uint32 p1, uint32 p2) */ int32 CmdChangeTrainServiceInt(int x, int y, uint32 flags, uint32 p1, uint32 p2) { - Vehicle *v = GetVehicle(p1); + Vehicle *v; + + if (!IsVehicleIndex(p1)) return CMD_ERROR; - if (!CheckOwnership(v->owner)) + v = GetVehicle(p1); + + if (v->type != VEH_Train || !CheckOwnership(v->owner)) return CMD_ERROR; if (flags & DC_EXEC) { diff --git a/vehicle.c b/vehicle.c index 9ebf7801b..aa83185b5 100644 --- a/vehicle.c +++ b/vehicle.c @@ -1331,13 +1331,15 @@ int32 CmdReplaceVehicle(int x, int y, uint32 flags, uint32 p1, uint32 p2) the last 8 bit is the engine. The 8 bits in front of the engine is free so it have room for 16 bit engine entries */ uint16 new_engine_type = (uint16)(p2 & 0xFFFF); uint32 autorefit_money = (p2 >> 16) * 100000; - Vehicle *v = GetVehicle(p1); + Vehicle *v, *u; int cost, build_cost, rear_engine_cost = 0; - Vehicle *u = v; - byte old_engine_type = v->engine_type; + byte old_engine_type; - SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES); + if (!IsVehicleIndex(p1)) return CMD_ERROR; + + v = u = GetVehicle(p1); + old_engine_type = v->engine_type; // first we make sure that it's a valid type the user requested // check that it's an engine that is in the engine array @@ -1636,6 +1638,8 @@ int32 CmdNameVehicle(int x, int y, uint32 flags, uint32 p1, uint32 p2) Vehicle *v; StringID str; + if (!IsVehicleIndex(p1)) return CMD_ERROR; + v = GetVehicle(p1); if (!CheckOwnership(v->owner)) diff --git a/vehicle.h b/vehicle.h index 2a069e8b1..8c626908a 100644 --- a/vehicle.h +++ b/vehicle.h @@ -376,6 +376,14 @@ static inline Vehicle *GetVehicle(uint index) return &_vehicles[index]; } +static inline bool IsVehicleIndex(uint index) +{ + if (index < _vehicles_size) + return true; + else + return false; +} + #define FOR_ALL_VEHICLES(v) for(v = _vehicles; v != &_vehicles[_vehicles_size]; v++) #define FOR_ALL_VEHICLES_FROM(v, from) for(v = GetVehicle(from); v != &_vehicles[_vehicles_size]; v++) -- cgit v1.2.3-70-g09d2