Subject: fix for vulnerability CVE-2011-3343 for OpenTTD 1.1.0 - 1.1.2 (Multiple buffer overflows in validation of external data)
From: OpenTTD developer team <info@openttd.org>
Origin: backport, http://vcs.openttd.org/svn/changeset/22871 http://vcs.openttd.org/svn/changeset/22873 http://vcs.openttd.org/svn/changeset/22874 
Bug: http://bugs.openttd.org/task/4746 http://bugs.openttd.org/task/4747 

In multiple places external data isn't properly checked before allocating
memory, which could lead to buffer overflows and arbitrary code execution.

These bugs are only exploitable locally by providing OpenTTD with
invalid/manipulated images, sounds or fonts. This means an attacker
either needs local access or has to trick an user into loading a
manipulated image into OpenTTD. This is especially a concern with
BMP files loaded as heightmaps.

All except one vulnerability are caused by improper validation of input
data prior to allocating memory buffers. It is possible to force allocation
of a too small buffer and thus out-of-bounds writes by causing an integer
overflow. Additionally in RLE-compressed BMP images, it is possible to
write arbitrary data outside the allocated buffer.

No patch for releases before 0.3.1 is provided, as this versions are
unsupported since a long time and would require larger changes not worth
the effort.

diff --git src/bmp.cpp src/bmp.cpp
index 2c85c76..d39f619 100644
--- src/bmp.cpp
+++ src/bmp.cpp
@@ -143,6 +143,7 @@ static inline bool BmpRead4Rle(BmpBuffer *buffer, BmpInfo *info, BmpData *data)
 			switch (c) {
 			case 0: // end of line
 				x = 0;
+				if (y == 0) return false;
 				pixel = &data->bitmap[--y * info->width];
 				break;
 			case 1: // end of bitmap
@@ -153,7 +154,7 @@ static inline bool BmpRead4Rle(BmpBuffer *buffer, BmpInfo *info, BmpData *data)
 			case 2: // delta
 				x += ReadByte(buffer);
 				i = ReadByte(buffer);
-				if (x >= info->width || (y == 0 && i > 0)) return false;
+				if (x >= info->width || i > y) return false;
 				y -= i;
 				pixel = &data->bitmap[y * info->width + x];
 				break;
@@ -226,6 +227,7 @@ static inline bool BmpRead8Rle(BmpBuffer *buffer, BmpInfo *info, BmpData *data)
 			switch (c) {
 			case 0: // end of line
 				x = 0;
+				if (y == 0) return false;
 				pixel = &data->bitmap[--y * info->width];
 				break;
 			case 1: // end of bitmap
@@ -236,13 +238,16 @@ static inline bool BmpRead8Rle(BmpBuffer *buffer, BmpInfo *info, BmpData *data)
 			case 2: // delta
 				x += ReadByte(buffer);
 				i = ReadByte(buffer);
-				if (x >= info->width || (y == 0 && i > 0)) return false;
+				if (x >= info->width || i > y) return false;
 				y -= i;
 				pixel = &data->bitmap[y * info->width + x];
 				break;
 			default: // uncompressed
-				if ((x += c) > info->width) return false;
-				for (i = 0; i < c; i++) *pixel++ = ReadByte(buffer);
+				for (i = 0; i < c; i++) {
+					if (EndOfBuffer(buffer) || x >= info->width) return false;
+					*pixel++ = ReadByte(buffer);
+					x++;
+				}
 				/* Padding for 16 bit align */
 				SkipBytes(buffer, c % 2);
 				break;
diff --git src/fontcache.cpp src/fontcache.cpp
index b279c34..616c54a 100644
--- src/fontcache.cpp
+++ src/fontcache.cpp
@@ -1034,6 +1034,9 @@ static bool GetFontAAState(FontSize size)
 	width  = max(1, slot->bitmap.width + (size == FS_NORMAL));
 	height = max(1, slot->bitmap.rows  + (size == FS_NORMAL));
 
+	/* Limit glyph size to prevent overflows later on. */
+	if (width > 256 || height > 256) usererror("Font glyph is too large");
+
 	/* FreeType has rendered the glyph, now we allocate a sprite and copy the image into it */
 	sprite.AllocateData(width * height);
 	sprite.width = width;
diff --git src/heightmap.cpp src/heightmap.cpp
index 07b1e5e..b85bcd9 100644
--- src/heightmap.cpp
+++ src/heightmap.cpp
@@ -142,13 +142,24 @@ static bool ReadHeightmapPNG(char *filename, uint *x, uint *y, byte **map)
 		return false;
 	}
 
+	uint width = png_get_image_width(png_ptr, info_ptr);
+	uint height = png_get_image_height(png_ptr, info_ptr);
+
+	/* Check if image dimensions don't overflow a size_t to avoid memory corruption. */
+	if ((uint64)width * height >= (size_t)-1) {
+		ShowErrorMessage(STR_ERROR_PNGMAP, STR_ERROR_HEIGHTMAP_TOO_LARGE, WL_ERROR);
+		fclose(fp);
+		png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
+		return false;
+	}
+
 	if (map != NULL) {
-		*map = MallocT<byte>(png_get_image_width(png_ptr, info_ptr) * png_get_image_height(png_ptr, info_ptr));
+		*map = MallocT<byte>(width * height);
 		ReadHeightmapPNGImageData(*map, png_ptr, info_ptr);
 	}
 
-	*x = png_get_image_width(png_ptr, info_ptr);
-	*y = png_get_image_height(png_ptr, info_ptr);
+	*x = width;
+	*y = height;
 
 	fclose(fp);
 	png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
@@ -243,6 +254,14 @@ static bool ReadHeightmapBMP(char *filename, uint *x, uint *y, byte **map)
 		return false;
 	}
 
+	/* Check if image dimensions don't overflow a size_t to avoid memory corruption. */
+	if ((uint64)info.width * info.height >= (size_t)-1 / (info.bpp == 24 ? 3 : 1)) {
+		ShowErrorMessage(STR_ERROR_BMPMAP, STR_ERROR_HEIGHTMAP_TOO_LARGE, WL_ERROR);
+		fclose(f);
+		BmpDestroyData(&data);
+		return false;
+	}
+
 	if (map != NULL) {
 		if (!BmpReadBitmap(&buffer, &info, &data)) {
 			ShowErrorMessage(STR_ERROR_BMPMAP, STR_ERROR_BMPMAP_IMAGE_TYPE, WL_ERROR);
diff --git src/lang/english.txt src/lang/english.txt
index f8b341a..e2b778c 100644
--- src/lang/english.txt
+++ src/lang/english.txt
@@ -3452,6 +3452,8 @@ STR_ERROR_PNGMAP_MISC                                           :{WHITE}... some
 STR_ERROR_BMPMAP                                                :{WHITE}Can't load landscape from BMP...
 STR_ERROR_BMPMAP_IMAGE_TYPE                                     :{WHITE}... could not convert image type
 
+STR_ERROR_HEIGHTMAP_TOO_LARGE                                   :{WHITE}... image is too large
+
 STR_WARNING_HEIGHTMAP_SCALE_CAPTION                             :{WHITE}Scale warning
 STR_WARNING_HEIGHTMAP_SCALE_MESSAGE                             :{YELLOW}Resizing source map too much is not recommended. Continue with the generation?
 
diff --git src/openttd.cpp src/openttd.cpp
index 5cd5eba..af1f77f 100644
--- src/openttd.cpp
+++ src/openttd.cpp
@@ -596,11 +596,12 @@ int ttd_main(int argc, char *argv[])
 
 	/*
 	 * The width and height must be at least 1 pixel and width times
-	 * height must still fit within a 32 bits integer, this way all
-	 * internal drawing routines work correctly.
+	 * height times bytes per pixel must still fit within a 32 bits
+	 * integer, even for 32 bpp video modes. This way all internal
+	 * drawing routines work correctly.
 	 */
-	_cur_resolution.width  = ClampU(_cur_resolution.width,  1, UINT16_MAX);
-	_cur_resolution.height = ClampU(_cur_resolution.height, 1, UINT16_MAX);
+	_cur_resolution.width  = ClampU(_cur_resolution.width,  1, UINT16_MAX / 2);
+	_cur_resolution.height = ClampU(_cur_resolution.height, 1, UINT16_MAX / 2);
 
 	/* enumerate language files */
 	InitializeLanguagePacks();
diff --git src/script/squirrel_helper.hpp src/script/squirrel_helper.hpp
index a7d0bf7..babdf74 100644
--- src/script/squirrel_helper.hpp
+++ src/script/squirrel_helper.hpp
@@ -118,6 +118,9 @@
 
 	template <> inline Array      *GetParam(ForceType<Array *>,      HSQUIRRELVM vm, int index, SQAutoFreePointers *ptr)
 	{
+		/* Sanity check of the size. */
+		if (sq_getsize(vm, index) > UINT16_MAX) throw sq_throwerror(vm, _SC("an array used as parameter to a function is too large"));
+
 		SQObject obj;
 		sq_getstackobj(vm, index, &obj);
 		sq_pushobject(vm, obj);
diff --git src/sound.cpp src/sound.cpp
index 2834078..89d2224 100644
--- src/sound.cpp
+++ src/sound.cpp
@@ -110,7 +110,8 @@ static bool SetBankSource(MixerChannel *mc, const SoundEntry *sound)
 {
 	assert(sound != NULL);
 
-	if (sound->file_size == 0) return false;
+	/* Check for valid sound size. */
+	if (sound->file_size == 0 || sound->file_size > ((size_t)-1) - 2) return false;
 
 	int8 *mem = MallocT<int8>(sound->file_size + 2);
 	/* Add two extra bytes so rate conversion can read these
diff --git src/sound/win32_s.cpp src/sound/win32_s.cpp
index c0e5da5..ef3f98f 100644
--- src/sound/win32_s.cpp
+++ src/sound/win32_s.cpp
@@ -63,7 +63,9 @@ static DWORD WINAPI SoundThread(LPVOID arg)
 	wfex.nBlockAlign = (wfex.nChannels * wfex.wBitsPerSample) / 8;
 	wfex.nAvgBytesPerSec = wfex.nSamplesPerSec * wfex.nBlockAlign;
 
+	/* Limit buffer size to prevent overflows. */
 	_bufsize = GetDriverParamInt(parm, "bufsize", (GB(GetVersion(), 0, 8) > 5) ? 8192 : 4096);
+	_bufsize = min(_bufsize, UINT16_MAX);
 
 	try {
 		if (NULL == (_event = CreateEvent(NULL, FALSE, FALSE, NULL))) throw "Failed to create event";
diff --git src/spriteloader/png.cpp src/spriteloader/png.cpp
index e72f440..4378c7f 100644
--- src/spriteloader/png.cpp
+++ src/spriteloader/png.cpp
@@ -106,9 +106,21 @@ static bool LoadPNG(SpriteLoader::Sprite *sprite, const char *filename, uint32 i
 			if (strcmp("y_offs", text_ptr[i].key) == 0) sprite->y_offs = strtol(text_ptr[i].text, NULL, 0);
 		}
 
-		sprite->height = png_get_image_height(png_ptr, info_ptr);
-		sprite->width  = png_get_image_width(png_ptr, info_ptr);
+		uint height = png_get_image_height(png_ptr, info_ptr);
+		uint width = png_get_image_width(png_ptr, info_ptr);
+		/* Check if sprite dimensions aren't larger than what is allowed in GRF-files. */
+		if (height > UINT8_MAX || width > UINT16_MAX) {
+			png_destroy_read_struct(&png_ptr, &info_ptr, &end_info);
+			return false;
+		}
+		sprite->height = height;
+		sprite->width  = width;
 		sprite->AllocateData(sprite->width * sprite->height);
+	} else if (sprite->height != png_get_image_height(png_ptr, info_ptr) || sprite->width != png_get_image_width(png_ptr, info_ptr)) {
+		/* Make sure the mask image isn't larger than the sprite image. */
+		DEBUG(misc, 0, "Ignoring mask for SpriteID %d as it isn't the same dimension as the masked sprite", id);
+		png_destroy_read_struct(&png_ptr, &info_ptr, &end_info);
+		return true;
 	}
 
 	bit_depth  = png_get_bit_depth(png_ptr, info_ptr);
@@ -116,6 +128,7 @@ static bool LoadPNG(SpriteLoader::Sprite *sprite, const char *filename, uint32 i
 
 	if (mask && (bit_depth != 8 || colour_type != PNG_COLOR_TYPE_PALETTE)) {
 		DEBUG(misc, 0, "Ignoring mask for SpriteID %d as it isn't a 8 bit palette image", id);
+		png_destroy_read_struct(&png_ptr, &info_ptr, &end_info);
 		return true;
 	}