(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
--- a/aircraft_cmd.c Sun Jan 30 19:51:39 2005 +0000
+++ b/aircraft_cmd.c Sun Jan 30 20:50:06 2005 +0000
@@ -360,13 +360,15 @@
{
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 @@
{
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 @@
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 @@
{
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 @@
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;
--- a/roadveh_cmd.c Sun Jan 30 19:51:39 2005 +0000
+++ b/roadveh_cmd.c Sun Jan 30 20:50:06 2005 +0000
@@ -209,6 +209,8 @@
{
Vehicle *v;
+ if (!IsVehicleIndex(p1)) return CMD_ERROR;
+
v = GetVehicle(p1);
if (v->type != VEH_Road || !CheckOwnership(v->owner))
@@ -229,13 +231,15 @@
{
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 @@
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 @@
{
Vehicle *v;
+ if (!IsVehicleIndex(p1)) return CMD_ERROR;
+
v = GetVehicle(p1);
if (v->type != VEH_Road || !CheckOwnership(v->owner))
@@ -373,6 +383,8 @@
{
Vehicle *v;
+ if (!IsVehicleIndex(p1)) return CMD_ERROR;
+
v = GetVehicle(p1);
if (v->type != VEH_Road || !CheckOwnership(v->owner))
--- a/ship_cmd.c Sun Jan 30 19:51:39 2005 +0000
+++ b/ship_cmd.c Sun Jan 30 20:50:06 2005 +0000
@@ -915,13 +915,15 @@
{
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 @@
{
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 @@
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 @@
{
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 @@
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 @@
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;
--- a/train_cmd.c Sun Jan 30 19:51:39 2005 +0000
+++ b/train_cmd.c Sun Jan 30 20:50:06 2005 +0000
@@ -719,7 +719,10 @@
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 @@
{
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 @@
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 @@
{
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 @@
{
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 @@
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 @@
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 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) {
--- a/vehicle.c Sun Jan 30 19:51:39 2005 +0000
+++ b/vehicle.c Sun Jan 30 20:50:06 2005 +0000
@@ -1331,13 +1331,15 @@
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 @@
Vehicle *v;
StringID str;
+ if (!IsVehicleIndex(p1)) return CMD_ERROR;
+
v = GetVehicle(p1);
if (!CheckOwnership(v->owner))
--- a/vehicle.h Sun Jan 30 19:51:39 2005 +0000
+++ b/vehicle.h Sun Jan 30 20:50:06 2005 +0000
@@ -376,6 +376,14 @@
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++)