From 52928e1e08bb3ad00bcd44d8b4f16643a0ae3557 Mon Sep 17 00:00:00 2001 From: Ted John Date: Sun, 3 Jul 2016 18:02:18 +0100 Subject: [PATCH] protect objects against invalid allocation ids --- src/drawing/Image.cpp | 6 +++++- src/localisation/language.cpp | 9 ++++++--- src/object/BannerObject.cpp | 7 ++++--- src/object/BannerObject.h | 4 ++-- src/object/EntranceObject.cpp | 6 ++++-- src/object/EntranceObject.h | 2 +- src/object/FootpathItemObject.cpp | 6 ++++-- src/object/FootpathItemObject.h | 4 ++-- src/object/FootpathObject.cpp | 7 ++++--- src/object/FootpathObject.h | 2 +- src/object/LargeSceneryObject.cpp | 7 ++++--- src/object/LargeSceneryObject.h | 4 ++-- src/object/ObjectRepository.cpp | 1 + src/object/RideObject.cpp | 7 +++++++ src/object/RideObject.h | 2 +- src/object/SceneryGroupObject.cpp | 6 ++++-- src/object/SceneryGroupObject.h | 6 +++--- src/object/SmallSceneryObject.cpp | 12 ++++-------- src/object/SmallSceneryObject.h | 4 ++-- src/object/StexObject.cpp | 8 +++++--- src/object/StexObject.h | 2 +- src/object/WallObject.cpp | 7 ++++--- src/object/WallObject.h | 4 ++-- src/object/WaterObject.cpp | 5 +++-- src/object/WaterObject.h | 2 +- src/rct2/S6Exporter.cpp | 2 +- 26 files changed, 78 insertions(+), 54 deletions(-) diff --git a/src/drawing/Image.cpp b/src/drawing/Image.cpp index 8e026686dd..e4d88b8967 100644 --- a/src/drawing/Image.cpp +++ b/src/drawing/Image.cpp @@ -66,6 +66,7 @@ static uint32 AllocateImageList(uint32 count) static void FreeImageList(uint32 baseImageId, uint32 count) { Guard::Assert(_initialised); + Guard::Assert(baseImageId >= BASE_IMAGE_ID); // TODO validate that this was an allocated list _freeLists.push_back({ baseImageId, count }); @@ -95,6 +96,9 @@ extern "C" void gfx_object_free_images(uint32 baseImageId, uint32 count) { - FreeImageList(baseImageId, count); + if (baseImageId != 0) + { + FreeImageList(baseImageId, count); + } } } diff --git a/src/localisation/language.cpp b/src/localisation/language.cpp index b3c8f75d01..f4dc609711 100644 --- a/src/localisation/language.cpp +++ b/src/localisation/language.cpp @@ -355,11 +355,14 @@ rct_string_id language_allocate_object_string(const utf8 * target) void language_free_object_string(rct_string_id stringId) { - if (_languageCurrent != nullptr) + if (stringId != 0) { - _languageCurrent->SetString(stringId, nullptr); + if (_languageCurrent != nullptr) + { + _languageCurrent->SetString(stringId, nullptr); + } + _availableObjectStringIds.push(stringId); } - _availableObjectStringIds.push(stringId); } } diff --git a/src/object/BannerObject.cpp b/src/object/BannerObject.cpp index c4cba5beaf..99fcf16ec3 100644 --- a/src/object/BannerObject.cpp +++ b/src/object/BannerObject.cpp @@ -31,9 +31,7 @@ enum OBJ_STRING_ID void BannerObject::ReadLegacy(IReadObjectContext * context, IStream * stream) { - _legacyType.name = stream->ReadValue(); - _legacyType.image = stream->ReadValue(); - + stream->Seek(6, STREAM_SEEK_CURRENT); _legacyType.banner.scrolling_mode = stream->ReadValue(); _legacyType.banner.flags = stream->ReadValue(); _legacyType.banner.price = stream->ReadValue(); @@ -72,6 +70,9 @@ void BannerObject::Unload() { language_free_object_string(_legacyType.name); gfx_object_free_images(_legacyType.image, GetImageTable()->GetCount()); + + _legacyType.name = 0; + _legacyType.image = 0; } void BannerObject::DrawPreview(rct_drawpixelinfo * dpi) const diff --git a/src/object/BannerObject.h b/src/object/BannerObject.h index 933816c3b9..a7dc09d218 100644 --- a/src/object/BannerObject.h +++ b/src/object/BannerObject.h @@ -26,8 +26,8 @@ extern "C" class BannerObject : public Object { private: - rct_scenery_entry _legacyType; - rct_object_entry _sceneryTabEntry; + rct_scenery_entry _legacyType = { 0 }; + rct_object_entry _sceneryTabEntry = { 0 }; public: explicit BannerObject(const rct_object_entry &entry) : Object(entry) { }; diff --git a/src/object/EntranceObject.cpp b/src/object/EntranceObject.cpp index b302d1fb82..da81343e1b 100644 --- a/src/object/EntranceObject.cpp +++ b/src/object/EntranceObject.cpp @@ -30,8 +30,7 @@ enum OBJ_STRING_ID void EntranceObject::ReadLegacy(IReadObjectContext * context, IStream * stream) { - _legacyType.string_idx = stream->ReadValue(); - _legacyType.image_id = stream->ReadValue(); + stream->Seek(6, STREAM_SEEK_CURRENT); _legacyType.scrolling_mode = stream->ReadValue(); _legacyType.text_height = stream->ReadValue(); @@ -49,6 +48,9 @@ void EntranceObject::Unload() { language_free_object_string(_legacyType.string_idx); gfx_object_free_images(_legacyType.image_id, GetImageTable()->GetCount()); + + _legacyType.string_idx = 0; + _legacyType.image_id = 0; } void EntranceObject::DrawPreview(rct_drawpixelinfo * dpi) const diff --git a/src/object/EntranceObject.h b/src/object/EntranceObject.h index be843883b8..cb370e8c0d 100644 --- a/src/object/EntranceObject.h +++ b/src/object/EntranceObject.h @@ -26,7 +26,7 @@ extern "C" class EntranceObject : public Object { private: - rct_entrance_type _legacyType; + rct_entrance_type _legacyType = { 0 }; public: explicit EntranceObject(const rct_object_entry &entry) : Object(entry) { }; diff --git a/src/object/FootpathItemObject.cpp b/src/object/FootpathItemObject.cpp index 48f2916424..0852ed1a49 100644 --- a/src/object/FootpathItemObject.cpp +++ b/src/object/FootpathItemObject.cpp @@ -30,8 +30,7 @@ enum OBJ_STRING_ID void FootpathItemObject::ReadLegacy(IReadObjectContext * context, IStream * stream) { - _legacyType.name = stream->ReadValue(); - _legacyType.image = stream->ReadValue(); + stream->Seek(6, STREAM_SEEK_CURRENT); _legacyType.path_bit.flags = stream->ReadValue(); _legacyType.path_bit.draw_type = stream->ReadValue(); _legacyType.path_bit.tool_id = stream->ReadValue(); @@ -71,6 +70,9 @@ void FootpathItemObject::Unload() { language_free_object_string(_legacyType.name); gfx_object_free_images(_legacyType.image, GetImageTable()->GetCount()); + + _legacyType.name = 0; + _legacyType.image = 0; } void FootpathItemObject::DrawPreview(rct_drawpixelinfo * dpi) const diff --git a/src/object/FootpathItemObject.h b/src/object/FootpathItemObject.h index 5fe3956359..6aabe8b58c 100644 --- a/src/object/FootpathItemObject.h +++ b/src/object/FootpathItemObject.h @@ -26,8 +26,8 @@ extern "C" class FootpathItemObject : public Object { private: - rct_scenery_entry _legacyType; - rct_object_entry _sceneryTabEntry; + rct_scenery_entry _legacyType = { 0 }; + rct_object_entry _sceneryTabEntry = { 0 }; public: explicit FootpathItemObject(const rct_object_entry &entry) : Object(entry) { }; diff --git a/src/object/FootpathObject.cpp b/src/object/FootpathObject.cpp index 2580f42f01..b7bfe4c928 100644 --- a/src/object/FootpathObject.cpp +++ b/src/object/FootpathObject.cpp @@ -31,9 +31,7 @@ enum OBJ_STRING_ID void FootpathObject::ReadLegacy(IReadObjectContext * context, IStream * stream) { - _legacyType.string_idx = stream->ReadValue(); - _legacyType.image = stream->ReadValue(); - _legacyType.bridge_image = stream->ReadValue(); + stream->Seek(10, STREAM_SEEK_CURRENT); _legacyType.var_0A = stream->ReadValue(); _legacyType.flags = stream->ReadValue(); _legacyType.scrolling_mode = stream->ReadValue(); @@ -60,6 +58,9 @@ void FootpathObject::Unload() { language_free_object_string(_legacyType.string_idx); gfx_object_free_images(_legacyType.image, GetImageTable()->GetCount()); + + _legacyType.string_idx = 0; + _legacyType.image = 0; } void FootpathObject::DrawPreview(rct_drawpixelinfo * dpi) const diff --git a/src/object/FootpathObject.h b/src/object/FootpathObject.h index ef4d3638a0..6bf997e6e1 100644 --- a/src/object/FootpathObject.h +++ b/src/object/FootpathObject.h @@ -26,7 +26,7 @@ extern "C" class FootpathObject : public Object { private: - rct_footpath_entry _legacyType; + rct_footpath_entry _legacyType = { 0 }; public: explicit FootpathObject(const rct_object_entry &entry) : Object(entry) { }; diff --git a/src/object/LargeSceneryObject.cpp b/src/object/LargeSceneryObject.cpp index cefb6828da..7a76701fba 100644 --- a/src/object/LargeSceneryObject.cpp +++ b/src/object/LargeSceneryObject.cpp @@ -37,9 +37,7 @@ LargeSceneryObject::~LargeSceneryObject() void LargeSceneryObject::ReadLegacy(IReadObjectContext * context, IStream * stream) { - _legacyType.name = stream->ReadValue(); - _legacyType.image = stream->ReadValue(); - + stream->Seek(6, STREAM_SEEK_CURRENT); _legacyType.large_scenery.tool_id = stream->ReadValue(); _legacyType.large_scenery.flags = stream->ReadValue(); _legacyType.large_scenery.price = stream->ReadValue(); @@ -115,6 +113,9 @@ void LargeSceneryObject::Unload() { language_free_object_string(_legacyType.name); gfx_object_free_images(_legacyType.image, GetImageTable()->GetCount()); + + _legacyType.name = 0; + _legacyType.image = 0; } void LargeSceneryObject::DrawPreview(rct_drawpixelinfo * dpi) const diff --git a/src/object/LargeSceneryObject.h b/src/object/LargeSceneryObject.h index 708b9f09c4..1034243988 100644 --- a/src/object/LargeSceneryObject.h +++ b/src/object/LargeSceneryObject.h @@ -26,8 +26,8 @@ extern "C" class LargeSceneryObject : public Object { private: - rct_scenery_entry _legacyType; - rct_object_entry _sceneryTabEntry; + rct_scenery_entry _legacyType = { 0 }; + rct_object_entry _sceneryTabEntry = { 0 }; rct_large_scenery_text * _3dFont = nullptr; rct_large_scenery_tile * _tiles = nullptr; diff --git a/src/object/ObjectRepository.cpp b/src/object/ObjectRepository.cpp index d337356692..9e1c9ce72a 100644 --- a/src/object/ObjectRepository.cpp +++ b/src/object/ObjectRepository.cpp @@ -645,6 +645,7 @@ extern "C" Object * object = _loadedObjects[i]; if (object != nullptr) { + object->Unload(); object->Load(); } } diff --git a/src/object/RideObject.cpp b/src/object/RideObject.cpp index e092b292a3..22929f6ae0 100644 --- a/src/object/RideObject.cpp +++ b/src/object/RideObject.cpp @@ -46,6 +46,9 @@ RideObject::~RideObject() void RideObject::ReadLegacy(IReadObjectContext * context, IStream * stream) { stream->Read(&_legacyType); + _legacyType.name = 0; + _legacyType.description = 0; + _legacyType.images_offset = 0; GetStringTable()->Read(context, stream, OBJ_STRING_ID_NAME); GetStringTable()->Read(context, stream, OBJ_STRING_ID_DESCRIPTION); @@ -311,6 +314,10 @@ void RideObject::Unload() language_free_object_string(_legacyType.name); language_free_object_string(_legacyType.description); gfx_object_free_images(_legacyType.images_offset, GetImageTable()->GetCount()); + + _legacyType.name = 0; + _legacyType.description = 0; + _legacyType.images_offset = 0; } void RideObject::DrawPreview(rct_drawpixelinfo * dpi) const diff --git a/src/object/RideObject.h b/src/object/RideObject.h index d53b9901df..2f8d0f097a 100644 --- a/src/object/RideObject.h +++ b/src/object/RideObject.h @@ -26,7 +26,7 @@ extern "C" class RideObject : public Object { private: - rct_ride_entry _legacyType; + rct_ride_entry _legacyType = { 0 }; vehicle_colour_preset_list _presetColours = { 0 }; sint8 * _peepLoadingPositions[4] = { nullptr }; diff --git a/src/object/SceneryGroupObject.cpp b/src/object/SceneryGroupObject.cpp index c2d1094867..74b2df8148 100644 --- a/src/object/SceneryGroupObject.cpp +++ b/src/object/SceneryGroupObject.cpp @@ -37,8 +37,7 @@ SceneryGroupObject::~SceneryGroupObject() void SceneryGroupObject::ReadLegacy(IReadObjectContext * context, IStream * stream) { - _legacyType.name = stream->ReadValue(); - _legacyType.image = stream->ReadValue(); + stream->Seek(6, STREAM_SEEK_CURRENT); stream->Seek(0x80 * 2, STREAM_SEEK_CURRENT); _legacyType.entry_count = stream->ReadValue(); _legacyType.var_107 = stream->ReadValue(); @@ -84,6 +83,9 @@ void SceneryGroupObject::Unload() { language_free_object_string(_legacyType.name); gfx_object_free_images(_legacyType.image, GetImageTable()->GetCount()); + + _legacyType.name = 0; + _legacyType.image = 0; } void SceneryGroupObject::DrawPreview(rct_drawpixelinfo * dpi) const diff --git a/src/object/SceneryGroupObject.h b/src/object/SceneryGroupObject.h index f2867bdc1b..a8715fc309 100644 --- a/src/object/SceneryGroupObject.h +++ b/src/object/SceneryGroupObject.h @@ -28,9 +28,9 @@ struct ObjectRepositoryItem; class SceneryGroupObject : public Object { private: - rct_scenery_set_entry _legacyType; - uint32 _numItems; - rct_object_entry * _items; + rct_scenery_set_entry _legacyType = { 0 }; + uint32 _numItems = 0; + rct_object_entry * _items = nullptr; public: explicit SceneryGroupObject(const rct_object_entry &entry) : Object(entry) { }; diff --git a/src/object/SmallSceneryObject.cpp b/src/object/SmallSceneryObject.cpp index 13133cf1d2..9c97b08bd5 100644 --- a/src/object/SmallSceneryObject.cpp +++ b/src/object/SmallSceneryObject.cpp @@ -37,9 +37,7 @@ SmallSceneryObject::~SmallSceneryObject() void SmallSceneryObject::ReadLegacy(IReadObjectContext * context, IStream * stream) { - _legacyType.name = stream->ReadValue(); - _legacyType.image = stream->ReadValue(); - + stream->Seek(6, STREAM_SEEK_CURRENT); _legacyType.small_scenery.flags = stream->ReadValue(); _legacyType.small_scenery.height = stream->ReadValue(); _legacyType.small_scenery.tool_id = stream->ReadValue(); @@ -103,15 +101,13 @@ void SmallSceneryObject::Unload() { language_free_object_string(_legacyType.name); gfx_object_free_images(_legacyType.image, GetImageTable()->GetCount()); + + _legacyType.name = 0; + _legacyType.image = 0; } void SmallSceneryObject::DrawPreview(rct_drawpixelinfo * dpi) const { - // rct_drawpixelinfo clipDPI; - // if (!clip_drawpixelinfo(&clipDPI, dpi, x - 56, y - 56, 112, 112)) { - // return; - // } - uint32 flags = _legacyType.small_scenery.flags; uint32 imageId = _legacyType.image; if (flags & SMALL_SCENERY_FLAG_HAS_PRIMARY_COLOUR) diff --git a/src/object/SmallSceneryObject.h b/src/object/SmallSceneryObject.h index 9e87b3a2f9..173b0025fb 100644 --- a/src/object/SmallSceneryObject.h +++ b/src/object/SmallSceneryObject.h @@ -26,8 +26,8 @@ extern "C" class SmallSceneryObject : public Object { private: - rct_scenery_entry _legacyType; - rct_object_entry _sceneryTabEntry; + rct_scenery_entry _legacyType = { 0 }; + rct_object_entry _sceneryTabEntry = { 0 }; uint8 * _var10data = nullptr; public: diff --git a/src/object/StexObject.cpp b/src/object/StexObject.cpp index 8023b25b46..9b83b4515f 100644 --- a/src/object/StexObject.cpp +++ b/src/object/StexObject.cpp @@ -31,9 +31,7 @@ enum OBJ_STRING_ID void StexObject::ReadLegacy(IReadObjectContext * context, IStream * stream) { - _legacyType.scenario_name = stream->ReadValue(); - _legacyType.park_name = stream->ReadValue(); - _legacyType.details = stream->ReadValue(); + stream->Seek(6, STREAM_SEEK_CURRENT); _legacyType.var_06 = stream->ReadValue(); stream->Seek(1, STREAM_SEEK_CURRENT); @@ -54,6 +52,10 @@ void StexObject::Unload() language_free_object_string(_legacyType.scenario_name); language_free_object_string(_legacyType.park_name); language_free_object_string(_legacyType.details); + + _legacyType.scenario_name = 0; + _legacyType.park_name = 0; + _legacyType.details = 0; } void StexObject::DrawPreview(rct_drawpixelinfo * dpi) const diff --git a/src/object/StexObject.h b/src/object/StexObject.h index 6032ef5de7..ba09077ebd 100644 --- a/src/object/StexObject.h +++ b/src/object/StexObject.h @@ -26,7 +26,7 @@ extern "C" class StexObject : public Object { private: - rct_stex_entry _legacyType; + rct_stex_entry _legacyType = { 0 }; public: explicit StexObject(const rct_object_entry &entry) : Object(entry) { }; diff --git a/src/object/WallObject.cpp b/src/object/WallObject.cpp index 1182551b5d..5a7276aac4 100644 --- a/src/object/WallObject.cpp +++ b/src/object/WallObject.cpp @@ -31,9 +31,7 @@ enum OBJ_STRING_ID void WallObject::ReadLegacy(IReadObjectContext * context, IStream * stream) { - _legacyType.name = stream->ReadValue(); - _legacyType.image = stream->ReadValue(); - + stream->Seek(6, STREAM_SEEK_CURRENT); _legacyType.wall.tool_id = stream->ReadValue(); _legacyType.wall.flags = stream->ReadValue(); _legacyType.wall.height = stream->ReadValue(); @@ -75,6 +73,9 @@ void WallObject::Unload() { language_free_object_string(_legacyType.name); gfx_object_free_images(_legacyType.image, GetImageTable()->GetCount()); + + _legacyType.name = 0; + _legacyType.image = 0; } void WallObject::DrawPreview(rct_drawpixelinfo * dpi) const diff --git a/src/object/WallObject.h b/src/object/WallObject.h index 51aed22c32..5dce93e816 100644 --- a/src/object/WallObject.h +++ b/src/object/WallObject.h @@ -26,8 +26,8 @@ extern "C" class WallObject : public Object { private: - rct_scenery_entry _legacyType; - rct_object_entry _sceneryTabEntry; + rct_scenery_entry _legacyType = { 0 }; + rct_object_entry _sceneryTabEntry = { 0 }; public: explicit WallObject(const rct_object_entry &entry) : Object(entry) { }; diff --git a/src/object/WaterObject.cpp b/src/object/WaterObject.cpp index 4ad0616341..a35d45121a 100644 --- a/src/object/WaterObject.cpp +++ b/src/object/WaterObject.cpp @@ -30,8 +30,7 @@ enum OBJ_STRING_ID void WaterObject::ReadLegacy(IReadObjectContext * context, IStream * stream) { - _legacyType.string_idx = stream->ReadValue(); - _legacyType.image_id = stream->ReadValue(); + stream->Seek(6, STREAM_SEEK_CURRENT); _legacyType.var_06 = stream->ReadValue(); _legacyType.var_0A = stream->ReadValue(); _legacyType.var_0E = stream->ReadValue(); @@ -54,6 +53,8 @@ void WaterObject::Load() void WaterObject::Unload() { language_free_object_string(_legacyType.string_idx); + + _legacyType.string_idx = 0; } void WaterObject::DrawPreview(rct_drawpixelinfo * dpi) const diff --git a/src/object/WaterObject.h b/src/object/WaterObject.h index 2bc4deabbc..b0531ac4ae 100644 --- a/src/object/WaterObject.h +++ b/src/object/WaterObject.h @@ -26,7 +26,7 @@ extern "C" class WaterObject : public Object { private: - rct_water_type _legacyType; + rct_water_type _legacyType = { 0 }; public: explicit WaterObject(const rct_object_entry &entry) : Object(entry) { }; diff --git a/src/rct2/S6Exporter.cpp b/src/rct2/S6Exporter.cpp index ec9e6e8a5d..be20a8e3b2 100644 --- a/src/rct2/S6Exporter.cpp +++ b/src/rct2/S6Exporter.cpp @@ -507,7 +507,7 @@ extern "C" } delete s6exporter; - // reset_loaded_objects(); + reset_loaded_objects(); gfx_invalidate_screen(); if (result && !(flags & S6_SAVE_FLAG_AUTOMATIC))