Changed anim/static PieceDesc loading, to allow for more sanity checks.

This should take care of all invalid reads in renderStatic(), updateStatic() and updateAnim()

svn-id: r32292

2 files changed, 98 insertions, 75 deletions

diff --git a/engines/gob/scenery.cpp b/engines/gob/scenery.cpp
index 2da6b092e0..6b52cdbbd6 100644
--- a/engines/gob/scenery.cpp
+++ b/engines/gob/scenery.cpp
@@ -100,7 +100,6 @@ int16 Scenery::loadStatic(char search) {
 	byte *extData = 0;
 	byte *dataPtr;
 	Static *ptr;
-	int16 pictDescId;
 	int16 width;
 	int16 height;
 	int16 sprResId;
@@ -143,8 +142,6 @@ int16 Scenery::loadStatic(char search) {
 	dataPtr += 2;
 
 	ptr->layers = new StaticLayer[ptr->layersCount];
-	ptr->pieces = new PieceDesc*[picsCount];
-	ptr->piecesFromExt = new bool[picsCount];
 
 	for (int i = 0; i < ptr->layersCount; i++) {
 		int16 offset = READ_LE_UINT16(dataPtr + i * 2);
@@ -166,18 +163,13 @@ int16 Scenery::loadStatic(char search) {
 		backsPtr++;
 	}
 
+	ptr->pieces = new PieceDesc*[picsCount];
+	ptr->piecesCount = new uint32[picsCount];
+
 	for (int i = 0; i < picsCount; i++) {
-		pictDescId = _vm->_inter->load16();
+		int16 pictDescId = _vm->_inter->load16();
 
-		if (pictDescId >= 30000) {
-			ptr->pieces[i] =
-				(PieceDesc *) _vm->_game->loadExtData(pictDescId, 0, 0);
-			ptr->piecesFromExt[i] = true;
-		} else {
-			ptr->pieces[i] =
-				(PieceDesc *) _vm->_game->loadTotResource(pictDescId);
-			ptr->piecesFromExt[i] = false;
-		}
+		loadPieces(pictDescId, ptr->pieces[i], ptr->piecesCount[i]);
 
 		width = _vm->_inter->load16();
 		height = _vm->_inter->load16();
@@ -224,8 +216,7 @@ void Scenery::freeStatic(int16 index) {
 		return;
 
 	for (int i = 0; i < _staticPictCount[index]; i++) {
-		if (_statics[index].piecesFromExt[i])
-			delete[] _statics[index].pieces[i];
+		delete[] _statics[index].pieces[i];
 
 		spr = _staticPictToSprite[index * 7 + i];
 		_spriteRefs[spr]--;
@@ -239,7 +230,7 @@ void Scenery::freeStatic(int16 index) {
 		delete[] _statics[index].layers[i].planes;
 	delete[] _statics[index].layers;
 	delete[] _statics[index].pieces;
-	delete[] _statics[index].piecesFromExt;
+	delete[] _statics[index].piecesCount;
 
 	_statics[index].layersCount = 0;
 	_staticPictCount[index] = -1;
@@ -253,8 +244,8 @@ void Scenery::renderStatic(int16 scenery, int16 layer) {
 	int16 order;
 	int16 plane;
 
-	int16 pieceIndex;
-	int16 pictIndex;
+	uint16 pieceIndex;
+	uint16 pictIndex;
 
 	int16 left;
 	int16 right;
@@ -286,15 +277,21 @@ void Scenery::renderStatic(int16 scenery, int16 layer) {
 			pieceIndex = planePtr->pieceIndex;
 			pictIndex = planePtr->pictIndex - 1;
 
-			if ((pictIndex >= _staticPictCount[scenery]) || (!ptr->pieces))
+			if (pictIndex >= _staticPictCount[scenery])
+				continue;
+
+			if (!ptr->pieces || !ptr->pieces[pictIndex])
+				continue;
+
+			if (pieceIndex >= ptr->piecesCount[pictIndex])
 				continue;
 
 			_vm->_draw->_destSpriteX = planePtr->destX;
 			_vm->_draw->_destSpriteY = planePtr->destY;
-			left = FROM_LE_16(ptr->pieces[pictIndex][pieceIndex].left);
-			right = FROM_LE_16(ptr->pieces[pictIndex][pieceIndex].right);
-			top = FROM_LE_16(ptr->pieces[pictIndex][pieceIndex].top);
-			bottom = FROM_LE_16(ptr->pieces[pictIndex][pieceIndex].bottom);
+			left = ptr->pieces[pictIndex][pieceIndex].left;
+			right = ptr->pieces[pictIndex][pieceIndex].right;
+			top = ptr->pieces[pictIndex][pieceIndex].top;
+			bottom = ptr->pieces[pictIndex][pieceIndex].bottom;
 
 			_vm->_draw->_sourceSurface =
 			    _staticPictToSprite[scenery * 7 + pictIndex];
@@ -316,8 +313,8 @@ void Scenery::updateStatic(int16 orderFrom, byte index, byte layer) {
 	int16 planeCount;
 	int16 order;
 	int16 plane;
-	int16 pieceIndex;
-	int16 pictIndex;
+	uint16 pieceIndex;
+	uint16 pictIndex;
 
 	int16 left;
 	int16 right;
@@ -341,16 +338,22 @@ void Scenery::updateStatic(int16 orderFrom, byte index, byte layer) {
 			pieceIndex = planePtr->pieceIndex;
 			pictIndex = planePtr->pictIndex - 1;
 
-			if ((pictIndex >= _staticPictCount[index]) || (!pictPtr[pictIndex]))
+			if (pictIndex >= _staticPictCount[index])
+				continue;
+
+			if (!pictPtr || !pictPtr[pictIndex])
+				continue;
+
+			if (pieceIndex >= _statics[index].piecesCount[pictIndex])
 				continue;
 
 			_vm->_draw->_destSpriteX = planePtr->destX;
 			_vm->_draw->_destSpriteY = planePtr->destY;
 
-			left = FROM_LE_16(pictPtr[pictIndex][pieceIndex].left);
-			right = FROM_LE_16(pictPtr[pictIndex][pieceIndex].right);
-			top = FROM_LE_16(pictPtr[pictIndex][pieceIndex].top);
-			bottom = FROM_LE_16(pictPtr[pictIndex][pieceIndex].bottom);
+			left = pictPtr[pictIndex][pieceIndex].left;
+			right = pictPtr[pictIndex][pieceIndex].right;
+			top = pictPtr[pictIndex][pieceIndex].top;
+			bottom = pictPtr[pictIndex][pieceIndex].bottom;
 
 			if (_vm->_draw->_destSpriteX > _toRedrawRight)
 				continue;
@@ -421,7 +424,6 @@ int16 Scenery::loadAnim(char search) {
 	byte *extData;
 	byte *dataPtr;
 	Animation *ptr;
-	int16 pictDescId;
 	int16 width;
 	int16 height;
 	int16 sprResId;
@@ -460,9 +462,6 @@ int16 Scenery::loadAnim(char search) {
 	dataPtr += 2;
 
 	ptr->layers = new AnimLayer[ptr->layersCount];
-	ptr->pieces = new PieceDesc*[picsCount];
-	ptr->piecesFromExt = new bool[picsCount];
-	ptr->sizes = new uint16[picsCount];
 
 	for (i = 0; i < ptr->layersCount; i++) {
 		int16 offset = READ_LE_UINT16(dataPtr + i * 2);
@@ -498,23 +497,13 @@ int16 Scenery::loadAnim(char search) {
 		}
 	}
 
+	ptr->pieces = new PieceDesc*[picsCount];
+	ptr->piecesCount = new uint32[picsCount];
+
 	for (i = 0; i < picsCount; i++) {
-		pictDescId = _vm->_inter->load16();
-		if (pictDescId >= 30000) {
-			uint32 size;
-
-			ptr->pieces[i] =
-				(PieceDesc *) _vm->_game->loadExtData(pictDescId, 0, 0, &size);
-			ptr->piecesFromExt[i] = true;
-			ptr->sizes[i] = size / 8;
-		} else {
-			int16 size;
+		int16 pictDescId = _vm->_inter->load16();
 
-			ptr->pieces[i] =
-				(PieceDesc *) _vm->_game->loadTotResource(pictDescId, &size);
-			ptr->piecesFromExt[i] = false;
-			ptr->sizes[i] = size / 8;
-		}
+		loadPieces(pictDescId, ptr->pieces[i], ptr->piecesCount[i]);
 
 		width = _vm->_inter->load16();
 		height = _vm->_inter->load16();
@@ -560,8 +549,7 @@ void Scenery::freeAnim(int16 index) {
 		return;
 
 	for (int i = 0; i < _animPictCount[index]; i++) {
-		if (_animations[index].piecesFromExt[i])
-			delete[] _animations[index].pieces[i];
+		delete[] _animations[index].pieces[i];
 
 		spr = _animPictToSprite[index * 7 + i];
 		_spriteRefs[spr]--;
@@ -575,8 +563,7 @@ void Scenery::freeAnim(int16 index) {
 		delete[] _animations[index].layers[i].frames;
 	delete[] _animations[index].layers;
 	delete[] _animations[index].pieces;
-	delete[] _animations[index].piecesFromExt;
-	delete[] _animations[index].sizes;
+	delete[] _animations[index].piecesCount;
 
 	_animPictCount[index] = 0;
 }
@@ -827,20 +814,19 @@ void Scenery::updateAnim(int16 layer, int16 frame, int16 animation, int16 flags,
 
 		pictIndex = (pictIndex & 15) - 1;
 
-		if ((pictIndex == 0xFFFF) || (_animPictCount[animation] <= pictIndex)) {
-			warning("Scenery::updateAnim: pictIndex out of range");
-			return;
-		}
+		if (pictIndex >= _animPictCount[animation])
+			continue;
 
-		if (_animations[animation].sizes[pictIndex] <= pieceIndex) {
-			warning("Scenery::updateAnim: pieceIndex out of range");
+		if (!pictPtr[pictIndex])
 			continue;
-		}
 
-		left = READ_LE_UINT16(&pictPtr[pictIndex][pieceIndex].left);
-		right = READ_LE_UINT16(&pictPtr[pictIndex][pieceIndex].right);
-		top = READ_LE_UINT16(&pictPtr[pictIndex][pieceIndex].top);
-		bottom = READ_LE_UINT16(&pictPtr[pictIndex][pieceIndex].bottom);
+		if (pieceIndex >= _animations[animation].piecesCount[pictIndex])
+			continue;
+
+		left = pictPtr[pictIndex][pieceIndex].left;
+		right = pictPtr[pictIndex][pieceIndex].right;
+		top = pictPtr[pictIndex][pieceIndex].top;
+		bottom = pictPtr[pictIndex][pieceIndex].bottom;
 
 		if (flags & 2) {
 			if (destX < _vm->_mult->_animLeft) {
@@ -961,4 +947,42 @@ Scenery::AnimLayer *Scenery::getAnimLayer(uint16 index, uint16 layer) {
 	return &_animations[index].layers[layer];
 }
 
+void Scenery::loadPieces(int16 pictDescId, PieceDesc *&pieceDesc, uint32 &piecesCount) {
+	byte *data;
+	uint32 size;
+	bool fromExt = false;
+
+	if (pictDescId >= 30000) {
+		fromExt = true;
+
+		uint32 eSize;
+
+		data = _vm->_game->loadExtData(pictDescId, 0, 0, &eSize);
+		size = eSize;
+	} else {
+		int16 tSize;
+
+		data = _vm->_game->loadTotResource(pictDescId, &tSize);
+		size = tSize;
+	}
+
+	if (!data)
+		error("Scenery::loadPieces(): Can't load pictDescId %d", pictDescId);
+
+	piecesCount = size / 8;
+	pieceDesc = new PieceDesc[piecesCount];
+
+	Common::MemoryReadStream pieceData(data, size);
+
+	for (uint32 i = 0; i < piecesCount; i++) {
+		pieceDesc[i].left = (int16) pieceData.readUint16LE();
+		pieceDesc[i].right = (int16) pieceData.readUint16LE();
+		pieceDesc[i].top = (int16) pieceData.readUint16LE();
+		pieceDesc[i].bottom = (int16) pieceData.readUint16LE();
+	}
+
+	if (fromExt)
+		delete[] data;
+}
+
 } // End of namespace Gob
diff --git a/engines/gob/scenery.h b/engines/gob/scenery.h
index 03ef84e16d..21bf29deda 100644
--- a/engines/gob/scenery.h
+++ b/engines/gob/scenery.h
@@ -34,10 +34,10 @@ public:
 #include "common/pack-start.h"	// START STRUCT PACKING
 
 	struct PieceDesc {
-		int16 left;		//NOTE:
-		int16 right;		//These are stored in Little Endian format
-		int16 top;		//And should be converted by client code when accessed
-		int16 bottom;		//i.e. use FROM_LE_16()
+		int16 left;
+		int16 right;
+		int16 top;
+		int16 bottom;
 	} PACKED_STRUCT;
 
 	struct StaticPlane {
@@ -82,19 +82,16 @@ public:
 		int16 layersCount;
 		StaticLayer *layers;
 		PieceDesc **pieces;
-		bool *piecesFromExt;
-		Static() : layersCount(0), layers(0), pieces(0),
-				   piecesFromExt(0) {}
+		uint32 *piecesCount;
+		Static() : layersCount(0), layers(0), pieces(0), piecesCount(0) {}
 	};
 
 	struct Animation {
 		int16 layersCount;
 		AnimLayer *layers;
 		PieceDesc **pieces;
-		bool *piecesFromExt;
-		uint16 *sizes;
-		Animation() : layersCount(0), layers(0), pieces(0),
-			              piecesFromExt(0) {}
+		uint32 *piecesCount;
+		Animation() : layersCount(0), layers(0), pieces(0), piecesCount(0) {}
 	};
 
 	int16 _curStatic;
@@ -151,6 +148,8 @@ protected:
 
 	GobEngine *_vm;
 
+	void loadPieces(int16 pictDescId, PieceDesc *&pieceDesc, uint32 &piecesCount);
+
 	void updateStatic(int16 orderFrom, byte index, byte layer);
 };