cpp/openttd-patchpack/source Changeset - r1237:bcfd11754153

Changeset - r1237:bcfd11754153

Parent rev.

Child rev.

[Not reviewed]

master

0 6 0

bjarni - 19 years ago 2005-01-30 20:50:06
bjarni@openttd.org

(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

6 files changed with 100 insertions and 28 deletions:

aircraft_cmd.c

roadveh_cmd.c

ship_cmd.c

train_cmd.c

vehicle.c

vehicle.h

0 comments (0 inline, 0 general)

aircraft_cmd.c

➞

Show inline comments

@@ @@ -360,13 +360,15 @@ int32 CmdSellAircraft(int x, int y, uint @@
+{
 	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, @@
+{
 	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 @@
 	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, @@
+{
 	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, uin @@
 	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;

roadveh_cmd.c

➞

Show inline comments

@@ @@ -209,6 +209,8 @@ int32 CmdStartStopRoadVeh(int x, int y, @@
+{
 	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, uint3 @@
+{
 	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 @@
 		 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, uint3 @@
+{
 	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, @@
+{
 	Vehicle *v;
 	if (!IsVehicleIndex(p1)) return CMD_ERROR;
 	v = GetVehicle(p1);
 	if (v->type != VEH_Road || !CheckOwnership(v->owner))

ship_cmd.c

➞

Show inline comments

@@ @@ -915,13 +915,15 @@ int32 CmdSellShip(int x, int y, uint32 f @@
+{
 	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, uin @@
+{
 	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, u @@
 	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 @@
+{
 	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 @@
 	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 @@
 			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;

train_cmd.c

➞

Show inline comments

@@ @@ -719,7 +719,10 @@ int32 CmdMoveRailVehicle(int x, int y, u @@
 	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, ui @@
+{
 	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, uin @@
 	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, in @@
+{
 	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, @@
+{
 	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, @@
 	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 FindClosestTra @@
 		 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, ui @@
  */
 int32 CmdChangeTrainServiceInt(int x, int y, uint32 flags, uint32 p1, uint32 p2)
+{
 	Vehicle *v = GetVehicle(p1);
 	if (!CheckOwnership(v->owner))
 	Vehicle *v;
 	if (!IsVehicleIndex(p1)) return CMD_ERROR;
 	v = GetVehicle(p1);
 	if (v->type != VEH_Train || !CheckOwnership(v->owner))
 		return CMD_ERROR;
 	if (flags & DC_EXEC) {

vehicle.c

➞

Show inline comments

@@ @@ -1331,13 +1331,15 @@ int32 CmdReplaceVehicle(int x, int y, ui @@
 	 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;
 	if (!IsVehicleIndex(p1)) return CMD_ERROR;
-	SET_EXPENSES_TYPE(EXPENSES_NEW_VEHICLES);
+	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, uint3 @@
 	Vehicle *v;
 	StringID str;
 	if (!IsVehicleIndex(p1)) return CMD_ERROR;
 	v = GetVehicle(p1);
 	if (!CheckOwnership(v->owner))

vehicle.h

➞

Show inline comments

@@ @@ -376,6 +376,14 @@ static inline Vehicle *GetVehicle(uint i @@
 	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++)

0 comments (0 inline, 0 general)