Subject: fix for vulnerability CVE-2011-3343 for OpenTTD 0.5.0 - 0.5.3 (Multiple buffer overflows in validation of external data) From: OpenTTD developer team Origin: backport, https://github.com/OpenTTD/OpenTTD/commit/6c7cbb1 https://github.com/OpenTTD/OpenTTD/commit/65637d8 https://github.com/OpenTTD/OpenTTD/commit/73624ab Bug: https://github.com/OpenTTD/OpenTTD/issues/4746 https://github.com/OpenTTD/OpenTTD/issues/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 bmp.c bmp.c index c2bd912..d9fa3ed 100644 --- bmp.c +++ bmp.c @@ -133,6 +133,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 @@ -143,7 +144,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; @@ -216,6 +217,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 @@ -226,13 +228,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 fontcache.c fontcache.c index 0b38a38..3468df9 100644 --- fontcache.c +++ fontcache.c @@ -394,6 +394,9 @@ static void SetGlyphPtr(FontSize size, WChar key, const GlyphEntry *glyph) 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) error("Font glyph is too large"); + /* FreeType has rendered the glyph, now we allocate a sprite and copy the image into it */ sprite = calloc(width * height + 8, 1); sprite->info = 1; diff --git heightmap.c heightmap.c index adc2a53..145f871 100644 --- heightmap.c +++ heightmap.c @@ -134,6 +134,14 @@ static bool ReadHeightmapPNG(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_ptr->width * info_ptr->height >= (size_t)-1) { + ShowErrorMessage(STR_ERROR_HEIGHTMAP_TOO_LARGE, STR_PNGMAP_ERROR, 0, 0); + fclose(fp); + png_destroy_read_struct(&png_ptr, &info_ptr, NULL); + return false; + } + if (map != NULL) { *map = malloc(info_ptr->width * info_ptr->height * sizeof(byte)); @@ -243,6 +251,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_HEIGHTMAP_TOO_LARGE, STR_BMPMAP_ERROR, 0, 0); + fclose(f); + BmpDestroyData(&data); + return false; + } + if (map != NULL) { if (!BmpReadBitmap(&buffer, &info, &data)) { ShowErrorMessage(STR_BMPMAP_ERR_IMAGE_TYPE, STR_BMPMAP_ERROR, 0, 0); diff --git lang/english.txt lang/english.txt index 47a9425..fb7e07e 100644 --- lang/english.txt +++ lang/english.txt @@ -1499,6 +1499,8 @@ STR_PNGMAP_ERR_MISC :{WHITE}...somet STR_BMPMAP_ERROR :{WHITE}Cannot load landscape from BMP... STR_BMPMAP_ERR_IMAGE_TYPE :{WHITE}...could not convert image type. +STR_ERROR_HEIGHTMAP_TOO_LARGE :{WHITE}... image is too large + ##id 0x0800 STR_0800_COST :{TINYFONT}{RED}Cost: {CURRENCY} STR_0801_COST :{RED}Cost: {CURRENCY} diff --git sound.c sound.c index e4666a1..b19efee 100644 --- sound.c +++ sound.c @@ -107,7 +107,8 @@ static bool SetBankSource(MixerChannel *mc, uint bank) if (bank >= GetNumSounds()) return false; fe = GetSound(bank); - if (fe->file_size == 0) return false; + /* Check for valid sound size. */ + if (fe->file_size == 0 || fe->file_size > ((size_t)-1) - 2) return false; mem = malloc(fe->file_size); if (mem == NULL) return false;