summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorrubidium <rubidium@openttd.org>2008-11-23 13:42:05 +0000
committerrubidium <rubidium@openttd.org>2008-11-23 13:42:05 +0000
commit2277a1ff9ccd2e5a5acca1e815f628f054c84491 (patch)
treed015540e51590b8c79083cc3dfec59135bb47baf
parentca5ed4b2cc6e18e73b1ac2e5a7d964531a58e81d (diff)
downloadopenttd-2277a1ff9ccd2e5a5acca1e815f628f054c84491.tar.xz
(svn r14610) -Fix [FS#2415]: possible stack corruption when reading corrupted sprites.
-Change: harden the sprite reading routine against corrupt sprites.
-rw-r--r--src/lang/english.txt1
-rw-r--r--src/spritecache.cpp5
-rw-r--r--src/spriteloader/grf.cpp53
3 files changed, 56 insertions, 3 deletions
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<SpriteLoader::CommonPixel>(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++) {