SCI: implementing boundary checking for GfxPalette::createFromData(), sq5 has a broken picture 0 resource, which would result in either crash or at least bad read from memory depending on whats read there

svn-id: r50178

5 files changed, 27 insertions, 10 deletions

diff --git a/engines/sci/graphics/palette.cpp b/engines/sci/graphics/palette.cpp
index 208bc6751b..ef20aafc82 100644
--- a/engines/sci/graphics/palette.cpp
+++ b/engines/sci/graphics/palette.cpp
@@ -97,7 +97,7 @@ void GfxPalette::setDefault() {
 #define SCI_PAL_FORMAT_CONSTANT 1
 #define SCI_PAL_FORMAT_VARIABLE 0
 
-void GfxPalette::createFromData(byte *data, Palette *paletteOut) {
+void GfxPalette::createFromData(byte *data, int bytesLeft, Palette *paletteOut) {
 	int palFormat = 0;
 	int palOffset = 0;
 	int palColorStart = 0;
@@ -105,10 +105,16 @@ void GfxPalette::createFromData(byte *data, Palette *paletteOut) {
 	int colorNo = 0;
 
 	memset(paletteOut, 0, sizeof(Palette));
-	// Setup default mapping
+	// Setup 1:1 mapping
 	for (colorNo = 0; colorNo < 256; colorNo++) {
 		paletteOut->mapping[colorNo] = colorNo;
 	}
+	if (bytesLeft < 37) {
+		// This happens when loading palette of picture 0 in sq5 - the resource is broken and doesn't contain a full
+		//  palette
+		warning("GfxPalette::createFromData() - not enough bytes in resource, expected palette header");
+		return;
+	}
 	if ((data[0] == 0 && data[1] == 1) || (data[0] == 0 && data[1] == 0 && READ_LE_UINT16(data + 29) == 0)) {
 		// SCI0/SCI1 palette
 		palFormat = SCI_PAL_FORMAT_VARIABLE; // CONSTANT;
@@ -123,6 +129,11 @@ void GfxPalette::createFromData(byte *data, Palette *paletteOut) {
 	}
 	switch (palFormat) {
 		case SCI_PAL_FORMAT_CONSTANT:
+			// Check, if enough bytes left
+			if (bytesLeft < palOffset + (3 * palColorCount)) {
+				warning("GfxPalette::createFromData() - not enough bytes in resource, expected palette colors");
+				return;
+			}
 			for (colorNo = palColorStart; colorNo < palColorStart + palColorCount; colorNo++) {
 				paletteOut->colors[colorNo].used = 1;
 				paletteOut->colors[colorNo].r = data[palOffset++];
@@ -131,6 +142,10 @@ void GfxPalette::createFromData(byte *data, Palette *paletteOut) {
 			}
 			break;
 		case SCI_PAL_FORMAT_VARIABLE:
+			if (bytesLeft < palOffset + (4 * palColorCount)) {
+				warning("GfxPalette::createFromData() - not enough bytes in resource, expected palette colors");
+				return;
+			}
 			for (colorNo = palColorStart; colorNo < palColorStart + palColorCount; colorNo++) {
 				paletteOut->colors[colorNo].used = data[palOffset++];
 				paletteOut->colors[colorNo].r = data[palOffset++];
@@ -378,7 +393,7 @@ bool GfxPalette::kernelSetFromResource(GuiResourceId resourceId, bool force) {
 	Palette palette;
 
 	if (palResource) {
-		createFromData(palResource->data, &palette);
+		createFromData(palResource->data, palResource->size, &palette);
 		set(&palette, force);
 		return true;
 	}
@@ -526,7 +541,7 @@ bool GfxPalette::palVaryLoadTargetPalette(GuiResourceId resourceId) {
 	Resource *palResource = _resMan->findResource(ResourceId(kResourceTypePalette, resourceId), false);
 	if (palResource) {
 		// Load and initialize destination palette
-		createFromData(palResource->data, &_palVaryTargetPalette);
+		createFromData(palResource->data, palResource->size, &_palVaryTargetPalette);
 		return true;
 	}
 	return false;
@@ -592,7 +607,7 @@ int16 GfxPalette::kernelPalVaryChangeTarget(GuiResourceId resourceId) {
 		Resource *palResource = _resMan->findResource(ResourceId(kResourceTypePalette, resourceId), false);
 		if (palResource) {
 			Palette insertPalette;
-			createFromData(palResource->data, &insertPalette);
+			createFromData(palResource->data, palResource->size, &insertPalette);
 			// insert new palette into target
 			insert(&insertPalette, &_palVaryTargetPalette);
 			// update palette and set on screen
diff --git a/engines/sci/graphics/palette.h b/engines/sci/graphics/palette.h
index 29120daa7d..e640d9b39d 100644
--- a/engines/sci/graphics/palette.h
+++ b/engines/sci/graphics/palette.h
@@ -40,7 +40,7 @@ public:
 	~GfxPalette();
 
 	void setDefault();
-	void createFromData(byte *data, Palette *paletteOut);
+	void createFromData(byte *data, int bytesLeft, Palette *paletteOut);
 	bool setAmiga();
 	void modifyAmigaPalette(byte *data);
 	void setEGA();
diff --git a/engines/sci/graphics/picture.cpp b/engines/sci/graphics/picture.cpp
index 0b62a558ca..b410fc47f1 100644
--- a/engines/sci/graphics/picture.cpp
+++ b/engines/sci/graphics/picture.cpp
@@ -109,7 +109,7 @@ void GfxPicture::drawSci11Vga() {
 	Palette palette;
 
 	// Create palette and set it
-	_palette->createFromData(inbuffer + palette_data_ptr, &palette);
+	_palette->createFromData(inbuffer + palette_data_ptr, size - palette_data_ptr, &palette);
 	_palette->set(&palette, true);
 
 	// display Cel-data
@@ -147,7 +147,7 @@ void GfxPicture::drawSci32Vga(int16 celNo) {
 
 	if ((celNo == -1) || (celNo == 0)) {
 		// Create palette and set it
-		_palette->createFromData(inbuffer + palette_data_ptr, &palette);
+		_palette->createFromData(inbuffer + palette_data_ptr, size - palette_data_ptr, &palette);
 		_palette->set(&palette, true);
 	}
 	if (celNo != -1) {
diff --git a/engines/sci/graphics/view.cpp b/engines/sci/graphics/view.cpp
index 40e2a4fb30..f00d6a652d 100644
--- a/engines/sci/graphics/view.cpp
+++ b/engines/sci/graphics/view.cpp
@@ -62,6 +62,7 @@ void GfxView::initData(GuiResourceId resourceId) {
 		error("view resource %d not found", resourceId);
 	}
 	_resourceData = _resource->data;
+	_resourceSize = _resource->size;
 
 	byte *celData, *loopData;
 	uint16 celOffset;
@@ -114,7 +115,7 @@ void GfxView::initData(GuiResourceId resourceId) {
 			// On the other side: vga sci1 games have this pointing to a VGA palette
 			//  and ega sci1 games have this pointing to a 8x16 byte mapping table that needs to get applied then
 			if (!isEGA) {
-				_palette->createFromData(&_resourceData[palOffset], &_viewPalette);
+				_palette->createFromData(&_resourceData[palOffset], _resourceSize - palOffset, &_viewPalette);
 				_embeddedPal = true;
 			} else {
 				// Only use the EGA-mapping, when being SCI1
@@ -197,7 +198,7 @@ void GfxView::initData(GuiResourceId resourceId) {
 		assert(celSize >= 32);
 
 		if (palOffset) {
-			_palette->createFromData(&_resourceData[palOffset], &_viewPalette);
+			_palette->createFromData(&_resourceData[palOffset], _resourceSize - palOffset, &_viewPalette);
 			_embeddedPal = true;
 		}
 
diff --git a/engines/sci/graphics/view.h b/engines/sci/graphics/view.h
index a2050dc9d5..fe033dfe23 100644
--- a/engines/sci/graphics/view.h
+++ b/engines/sci/graphics/view.h
@@ -86,6 +86,7 @@ private:
 	GuiResourceId _resourceId;
 	Resource *_resource;
 	byte *_resourceData;
+	int _resourceSize;
 
 	uint16 _loopCount;
 	LoopInfo *_loop;