(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

1 files changed, 19 insertions, 5 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;