(svn r26130) -Codechange: add some guards against using invalid image indices

7 files changed, 87 insertions, 6 deletions

diff --git a/src/aircraft_cmd.cpp b/src/aircraft_cmd.cpp
index 8cc009d3d..e5dea8b75 100644
--- a/src/aircraft_cmd.cpp
+++ b/src/aircraft_cmd.cpp
@@ -99,6 +99,12 @@ static const SpriteID _aircraft_sprite[] = {
 	0x0EBD, 0x0EC5
 };
 
+template <>
+bool IsValidImageIndex<VEH_AIRCRAFT>(uint8 image_index)
+{
+	return image_index < lengthof(_aircraft_sprite);
+}
+
 /** Helicopter rotor animation states */
 enum HelicopterRotorStates {
 	HRS_ROTOR_STOPPED,
@@ -160,6 +166,7 @@ SpriteID Aircraft::GetImage(Direction direction, EngineImageType image_type) con
 		spritenum = this->GetEngine()->original_image_index;
 	}
 
+	assert(IsValidImageIndex<VEH_AIRCRAFT>(spritenum));
 	return direction + _aircraft_sprite[spritenum];
 }
 
@@ -189,6 +196,7 @@ static SpriteID GetAircraftIcon(EngineID engine, EngineImageType image_type)
 		spritenum = e->original_image_index;
 	}
 
+	assert(IsValidImageIndex<VEH_AIRCRAFT>(spritenum));
 	return DIR_W + _aircraft_sprite[spritenum];
 }
 
diff --git a/src/newgrf.cpp b/src/newgrf.cpp
index e3cca8b95..4a25b1dd8 100644
--- a/src/newgrf.cpp
+++ b/src/newgrf.cpp
@@ -189,6 +189,18 @@ public:
 static GrfProcessingState _cur;
 
 
+/**
+ * Helper to check whether an image index is valid for a particular NewGRF vehicle.
+ * @param <T> The type of vehicle.
+ * @param image_index The image index to check.
+ * @return True iff the image index is valid, or 0xFD (use new graphics).
+ */
+template <VehicleType T>
+static inline bool IsValidNewGRFImageIndex(uint8 image_index)
+{
+	return image_index == 0xFD || IsValidImageIndex<T>(image_index);
+}
+
 class OTTDByteReaderSignal { };
 
 /** Class to read from a NewGRF file */
@@ -1006,12 +1018,18 @@ static ChangeInfoResult RailVehicleChangeInfo(uint engine, int numinfo, int prop
 
 			case 0x12: { // Sprite ID
 				uint8 spriteid = buf->ReadByte();
+				uint8 orig_spriteid = spriteid;
 
 				/* TTD sprite IDs point to a location in a 16bit array, but we use it
 				 * as an array index, so we need it to be half the original value. */
 				if (spriteid < 0xFD) spriteid >>= 1;
 
-				rvi->image_index = spriteid;
+				if (IsValidNewGRFImageIndex<VEH_TRAIN>(spriteid)) {
+					rvi->image_index = spriteid;
+				} else {
+					grfmsg(1, "RailVehicleChangeInfo: Invalid Sprite %d specified, ignoring", orig_spriteid);
+					rvi->image_index = 0;
+				}
 				break;
 			}
 
@@ -1252,13 +1270,19 @@ static ChangeInfoResult RoadVehicleChangeInfo(uint engine, int numinfo, int prop
 
 			case 0x0E: { // Sprite ID
 				uint8 spriteid = buf->ReadByte();
+				uint8 orig_spriteid = spriteid;
 
 				/* cars have different custom id in the GRF file */
 				if (spriteid == 0xFF) spriteid = 0xFD;
 
 				if (spriteid < 0xFD) spriteid >>= 1;
 
-				rvi->image_index = spriteid;
+				if (IsValidNewGRFImageIndex<VEH_ROAD>(spriteid)) {
+					rvi->image_index = spriteid;
+				} else {
+					grfmsg(1, "RoadVehicleChangeInfo: Invalid Sprite %d specified, ignoring", orig_spriteid);
+					rvi->image_index = 0;
+				}
 				break;
 			}
 
@@ -1422,13 +1446,19 @@ static ChangeInfoResult ShipVehicleChangeInfo(uint engine, int numinfo, int prop
 		switch (prop) {
 			case 0x08: { // Sprite ID
 				uint8 spriteid = buf->ReadByte();
+				uint8 orig_spriteid = spriteid;
 
 				/* ships have different custom id in the GRF file */
 				if (spriteid == 0xFF) spriteid = 0xFD;
 
 				if (spriteid < 0xFD) spriteid >>= 1;
 
-				svi->image_index = spriteid;
+				if (IsValidNewGRFImageIndex<VEH_SHIP>(spriteid)) {
+					svi->image_index = spriteid;
+				} else {
+					grfmsg(1, "ShipVehicleChangeInfo: Invalid Sprite %d specified, ignoring", orig_spriteid);
+					svi->image_index = 0;
+				}
 				break;
 			}
 
@@ -1588,13 +1618,19 @@ static ChangeInfoResult AircraftVehicleChangeInfo(uint engine, int numinfo, int
 		switch (prop) {
 			case 0x08: { // Sprite ID
 				uint8 spriteid = buf->ReadByte();
+				uint8 orig_spriteid = spriteid;
 
 				/* aircraft have different custom id in the GRF file */
 				if (spriteid == 0xFF) spriteid = 0xFD;
 
 				if (spriteid < 0xFD) spriteid >>= 1;
 
-				avi->image_index = spriteid;
+				if (IsValidNewGRFImageIndex<VEH_AIRCRAFT>(spriteid)) {
+					avi->image_index = spriteid;
+				} else {
+					grfmsg(1, "AircraftVehicleChangeInfo: Invalid Sprite %d specified, ignoring", orig_spriteid);
+					avi->image_index = 0;
+				}
 				break;
 			}
 
diff --git a/src/roadveh_cmd.cpp b/src/roadveh_cmd.cpp
index 88bdb5622..1fc6d0d96 100644
--- a/src/roadveh_cmd.cpp
+++ b/src/roadveh_cmd.cpp
@@ -38,7 +38,7 @@
 
 #include "table/strings.h"
 
-static const uint16 _roadveh_images[63] = {
+static const uint16 _roadveh_images[] = {
 	0xCD4, 0xCDC, 0xCE4, 0xCEC, 0xCF4, 0xCFC, 0xD0C, 0xD14,
 	0xD24, 0xD1C, 0xD2C, 0xD04, 0xD1C, 0xD24, 0xD6C, 0xD74,
 	0xD7C, 0xC14, 0xC1C, 0xC24, 0xC2C, 0xC34, 0xC3C, 0xC4C,
@@ -49,7 +49,7 @@ static const uint16 _roadveh_images[63] = {
 	0xC5C, 0xC64, 0xC6C, 0xC74, 0xC84, 0xC94, 0xCA4
 };
 
-static const uint16 _roadveh_full_adder[63] = {
+static const uint16 _roadveh_full_adder[] = {
 	 0,  88,   0,   0,   0,   0,  48,  48,
 	48,  48,   0,   0,  64,  64,   0,  16,
 	16,   0,  88,   0,   0,   0,   0,  48,
@@ -59,6 +59,13 @@ static const uint16 _roadveh_full_adder[63] = {
 	 0,  16,  16,   0,   8,   8,   8,   8,
 	 0,   0,   0,   8,   8,   8,   8
 };
+assert_compile(lengthof(_roadveh_images) == lengthof(_roadveh_full_adder));
+
+template <>
+bool IsValidImageIndex<VEH_ROAD>(uint8 image_index)
+{
+	return image_index < lengthof(_roadveh_images);
+}
 
 /** 'Convert' the DiagDirection where a road vehicle enters to the trackdirs it can drive onto */
 static const TrackdirBits _road_enter_dir_to_reachable_trackdirs[DIAGDIR_END] = {
@@ -116,6 +123,7 @@ static SpriteID GetRoadVehIcon(EngineID engine, EngineImageType image_type)
 		spritenum = e->original_image_index;
 	}
 
+	assert(IsValidImageIndex<VEH_ROAD>(spritenum));
 	return DIR_W + _roadveh_images[spritenum];
 }
 
@@ -131,6 +139,7 @@ SpriteID RoadVehicle::GetImage(Direction direction, EngineImageType image_type)
 		spritenum = this->GetEngine()->original_image_index;
 	}
 
+	assert(IsValidImageIndex<VEH_ROAD>(spritenum));
 	sprite = direction + _roadveh_images[spritenum];
 
 	if (this->cargo.StoredCount() >= this->cargo_cap / 2U) sprite += _roadveh_full_adder[spritenum];
diff --git a/src/ship_cmd.cpp b/src/ship_cmd.cpp
index bafce981f..f1d09d40c 100644
--- a/src/ship_cmd.cpp
+++ b/src/ship_cmd.cpp
@@ -57,6 +57,12 @@ WaterClass GetEffectiveWaterClass(TileIndex tile)
 
 static const uint16 _ship_sprites[] = {0x0E5D, 0x0E55, 0x0E65, 0x0E6D};
 
+template <>
+bool IsValidImageIndex<VEH_SHIP>(uint8 image_index)
+{
+	return image_index < lengthof(_ship_sprites);
+}
+
 static inline TrackBits GetTileShipTrackStatus(TileIndex tile)
 {
 	return TrackStatusToTrackBits(GetTileTrackStatus(tile, TRANSPORT_WATER, 0));
@@ -74,6 +80,7 @@ static SpriteID GetShipIcon(EngineID engine, EngineImageType image_type)
 		spritenum = e->original_image_index;
 	}
 
+	assert(IsValidImageIndex<VEH_SHIP>(spritenum));
 	return DIR_W + _ship_sprites[spritenum];
 }
 
@@ -115,6 +122,7 @@ SpriteID Ship::GetImage(Direction direction, EngineImageType image_type) const
 		spritenum = this->GetEngine()->original_image_index;
 	}
 
+	assert(IsValidImageIndex<VEH_SHIP>(spritenum));
 	return _ship_sprites[spritenum] + direction;
 }
 
diff --git a/src/table/train_cmd.h b/src/table/train_cmd.h
index ec92069fc..15ac6f2aa 100644
--- a/src/table/train_cmd.h
+++ b/src/table/train_cmd.h
@@ -64,3 +64,7 @@ static const byte _wagon_full_adder[] = {
 	 0,  0, 24, 24, 24, 24,  0,  0,
 	32, 32
 };
+
+assert_compile(lengthof(_engine_sprite_base) == lengthof(_engine_sprite_and));
+assert_compile(lengthof(_engine_sprite_base) == lengthof(_engine_sprite_add));
+assert_compile(lengthof(_engine_sprite_base) == lengthof(_wagon_full_adder));
diff --git a/src/train_cmd.cpp b/src/train_cmd.cpp
index 5ccddc79a..4e9ab2a15 100644
--- a/src/train_cmd.cpp
+++ b/src/train_cmd.cpp
@@ -49,6 +49,11 @@ static void CheckNextTrainTile(Train *v);
 static const byte _vehicle_initial_x_fract[4] = {10, 8, 4,  8};
 static const byte _vehicle_initial_y_fract[4] = { 8, 4, 8, 10};
 
+template <>
+bool IsValidImageIndex<VEH_TRAIN>(uint8 image_index)
+{
+	return image_index < lengthof(_engine_sprite_base);
+}
 
 /**
  * Determine the side in which the train will leave the tile
@@ -456,6 +461,7 @@ int Train::GetDisplayImageWidth(Point *offset) const
 
 static SpriteID GetDefaultTrainSprite(uint8 spritenum, Direction direction)
 {
+	assert(IsValidImageIndex<VEH_TRAIN>(spritenum));
 	return ((direction + _engine_sprite_add[spritenum]) & _engine_sprite_and[spritenum]) + _engine_sprite_base[spritenum];
 }
 
@@ -479,6 +485,7 @@ SpriteID Train::GetImage(Direction direction, EngineImageType image_type) const
 		spritenum = this->GetEngine()->original_image_index;
 	}
 
+	assert(IsValidImageIndex<VEH_TRAIN>(spritenum));
 	sprite = GetDefaultTrainSprite(spritenum, direction);
 
 	if (this->cargo.StoredCount() >= this->cargo_cap / 2U) sprite += _wagon_full_adder[spritenum];
diff --git a/src/vehicle_func.h b/src/vehicle_func.h
index 758ad8305..757f1b6e6 100644
--- a/src/vehicle_func.h
+++ b/src/vehicle_func.h
@@ -29,6 +29,15 @@
 static const int VEHICLE_PROFIT_MIN_AGE = DAYS_IN_YEAR * 2; ///< Only vehicles older than this have a meaningful profit.
 static const Money VEHICLE_PROFIT_THRESHOLD = 10000;        ///< Threshold for a vehicle to be considered making good profit.
 
+/**
+ * Helper to check whether an image index is valid for a particular vehicle.
+ * @param <T> The type of vehicle.
+ * @param image_index The image index to check.
+ * @return True iff the image index is valid.
+ */
+template <VehicleType T>
+bool IsValidImageIndex(uint8 image_index);
+
 typedef Vehicle *VehicleFromPosProc(Vehicle *v, void *data);
 
 void VehicleServiceInDepot(Vehicle *v);