From 2277a1ff9ccd2e5a5acca1e815f628f054c84491 Mon Sep 17 00:00:00 2001 From: rubidium Date: Sun, 23 Nov 2008 13:42:05 +0000 Subject: (svn r14610) -Fix [FS#2415]: possible stack corruption when reading corrupted sprites. -Change: harden the sprite reading routine against corrupt sprites. --- src/lang/english.txt | 1 + src/spritecache.cpp | 5 ++++- src/spriteloader/grf.cpp | 53 ++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 56 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/lang/english.txt b/src/lang/english.txt index f4341aa84..4a8f8f243 100644 --- a/src/lang/english.txt +++ b/src/lang/english.txt @@ -3234,6 +3234,7 @@ STR_NEWGRF_ERROR_STATIC_GRF_CAUSES_DESYNC :Loading {RAW_ST STR_NEWGRF_ERROR_UNEXPECTED_SPRITE :Unexpected sprite. STR_NEWGRF_ERROR_UNKNOWN_PROPERTY :Unknown Action 0 property. STR_NEWGRF_ERROR_INVALID_ID :Attempt to use invalid ID. +STR_NEWGRF_ERROR_CORRUPT_SPRITE :{YELLOW}{RAW_STRING} contains a corrupt sprite. All corrupt sprites will be shown as a red question mark (?). STR_NEWGRF_PRESET_LIST_TIP :{BLACK}Load the selected preset STR_NEWGRF_PRESET_SAVE :{BLACK}Save preset diff --git a/src/spritecache.cpp b/src/spritecache.cpp index 4de7e076b..a354c65ff 100644 --- a/src/spritecache.cpp +++ b/src/spritecache.cpp @@ -271,7 +271,10 @@ static void* ReadSprite(SpriteCache *sc, SpriteID id, SpriteType sprite_type) sc->type = sprite_type; - if (!sprite_loader.LoadSprite(&sprite, file_slot, file_pos, sprite_type)) return NULL; + if (!sprite_loader.LoadSprite(&sprite, file_slot, file_pos, sprite_type)) { + if (id == SPR_IMG_QUERY) usererror("Okay... something went horribly wrong. I couldn't load the fallback sprite. What should I do?"); + return (void*)GetRawSprite(SPR_IMG_QUERY, ST_NORMAL); + } sc->ptr = BlitterFactoryBase::GetCurrentBlitter()->Encode(&sprite, &AllocSprite); free(sprite.data); diff --git a/src/spriteloader/grf.cpp b/src/spriteloader/grf.cpp index 1ded90b07..d7403ef31 100644 --- a/src/spriteloader/grf.cpp +++ b/src/spriteloader/grf.cpp @@ -7,8 +7,31 @@ #include "../fileio_func.h" #include "../debug.h" #include "../core/alloc_func.hpp" +#include "../strings_func.h" +#include "table/strings.h" +#include "../gui.h" #include "grf.hpp" +/** + * We found a corrupted sprite. This means that the sprite itself + * contains invalid data or is too small for the given dimensions. + * @param file_slot the file the errored sprite is in + * @param file_pos the location in the file of the errored sprite + * @param line the line where the error occurs. + * @return always false (to tell loading the sprite failed) + */ +static bool WarnCorruptSprite(uint8 file_slot, size_t file_pos, int line) +{ + static byte warning_level = 0; + if (warning_level == 0) { + SetDParamStr(0, FioGetFilename(file_slot)); + ShowErrorMessage(INVALID_STRING_ID, STR_NEWGRF_ERROR_CORRUPT_SPRITE, 0, 0); + } + DEBUG(sprite, warning_level, "[%i] Loading corrupted sprite from %s at position %i", line, FioGetFilename(file_slot), (int)file_pos); + warning_level = 6; + return false; +} + bool SpriteLoaderGrf::LoadSprite(SpriteLoader::Sprite *sprite, uint8 file_slot, size_t file_pos, SpriteType sprite_type) { /* Open the right file and go to the correct position */ @@ -32,6 +55,7 @@ bool SpriteLoaderGrf::LoadSprite(SpriteLoader::Sprite *sprite, uint8 file_slot, byte *dest_orig = AllocaM(byte, num); byte *dest = dest_orig; + const int dest_size = num; /* Read the file, which has some kind of compression */ while (num > 0) { @@ -41,6 +65,7 @@ bool SpriteLoaderGrf::LoadSprite(SpriteLoader::Sprite *sprite, uint8 file_slot, /* Plain bytes to read */ int size = (code == 0) ? 0x80 : code; num -= size; + if (num < 0) return WarnCorruptSprite(file_slot, file_pos, __LINE__); for (; size > 0; size--) { *dest = FioReadByte(); dest++; @@ -48,8 +73,10 @@ bool SpriteLoaderGrf::LoadSprite(SpriteLoader::Sprite *sprite, uint8 file_slot, } else { /* Copy bytes from earlier in the sprite */ const uint data_offset = ((code & 7) << 8) | FioReadByte(); + if (dest - data_offset < dest_orig) return WarnCorruptSprite(file_slot, file_pos, __LINE__); int size = -(code >> 3); num -= size; + if (num < 0) return WarnCorruptSprite(file_slot, file_pos, __LINE__); for (; size > 0; size--) { *dest = *(dest - data_offset); dest++; @@ -57,7 +84,7 @@ bool SpriteLoaderGrf::LoadSprite(SpriteLoader::Sprite *sprite, uint8 file_slot, } } - assert(num == 0); + if (num != 0) return WarnCorruptSprite(file_slot, file_pos, __LINE__); sprite->data = CallocT(sprite->width * sprite->height); @@ -67,10 +94,16 @@ bool SpriteLoaderGrf::LoadSprite(SpriteLoader::Sprite *sprite, uint8 file_slot, bool last_item = false; /* Look up in the header-table where the real data is stored for this row */ int offset = (dest_orig[y * 2 + 1] << 8) | dest_orig[y * 2]; + /* Go to that row */ - dest = &dest_orig[offset]; + dest = dest_orig + offset; do { + if (dest + 2 > dest_orig + dest_size) { + free(sprite->data); + return WarnCorruptSprite(file_slot, file_pos, __LINE__); + } + SpriteLoader::CommonPixel *data; /* Read the header: * 0 .. 14 - length @@ -82,6 +115,11 @@ bool SpriteLoaderGrf::LoadSprite(SpriteLoader::Sprite *sprite, uint8 file_slot, data = &sprite->data[y * sprite->width + skip]; + if (skip + length > sprite->width || dest + length > dest_orig + dest_size) { + free(sprite->data); + return WarnCorruptSprite(file_slot, file_pos, __LINE__); + } + for (int x = 0; x < length; x++) { data->m = ((sprite_type == ST_NORMAL && _palette_remap_grf[file_slot]) ? _palette_remap[*dest] : *dest); dest++; @@ -90,6 +128,17 @@ bool SpriteLoaderGrf::LoadSprite(SpriteLoader::Sprite *sprite, uint8 file_slot, } while (!last_item); } } else { + if (dest_size < sprite->width * sprite->height) { + free(sprite->data); + return WarnCorruptSprite(file_slot, file_pos, __LINE__); + } + + if (dest_size > sprite->width * sprite->height) { + static byte warning_level = 0; + DEBUG(sprite, warning_level, "Ignoring %i unused extra bytes from the sprite from %s at position %i", dest_size - sprite->width * sprite->height, FioGetFilename(file_slot), (int)file_pos); + warning_level = 6; + } + dest = dest_orig; for (int i = 0; i < sprite->width * sprite->height; i++) { -- cgit v1.2.3-54-g00ecf