From c54466ec287531a6a123ad702cf6d5f790e66aeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Janiszewski?= Date: Sat, 9 Aug 2025 23:56:12 +0200 Subject: [PATCH] Fix #24904: gracefully handle incomplete CSS files (#24923) * Fix #24904: gracefully handle incomplete CSS files Some CSS files are truncated, possibly due to users overwriting RCT2 assets with parts of RCT1. This causes incorrect access to freed memory and potential double frees. This change verifies CSS data has expected samples and if not, informs user about incorrect assets. The samples that exist are still loaded and available for use. There was no backtrace report generated for this on Windows. * Skip reading of unused variables --- data/language/en-GB.txt | 1 + src/openrct2-ui/audio/AudioContext.cpp | 22 ++++--- src/openrct2-ui/audio/SDLAudioSource.cpp | 75 ++++++++++++------------ src/openrct2/localisation/StringIds.h | 3 + src/openrct2/object/AudioSampleTable.cpp | 14 +++++ src/openrct2/object/Object.cpp | 20 +++++++ 6 files changed, 89 insertions(+), 46 deletions(-) diff --git a/data/language/en-GB.txt b/data/language/en-GB.txt index a39784b3f1..cf801e63a4 100644 --- a/data/language/en-GB.txt +++ b/data/language/en-GB.txt @@ -3841,3 +3841,4 @@ STR_6799 :Mini map STR_7000 :or STR_7001 :Ride name STR_7002 :{STRINGID} {STRINGID} +STR_7003 :Audio file ‘{STRING}’ is truncated. Expected sample {INT32}, but only {INT32} are available. Consider reinstalling RCT2. diff --git a/src/openrct2-ui/audio/AudioContext.cpp b/src/openrct2-ui/audio/AudioContext.cpp index e20ffc5f8a..96e7b2455b 100644 --- a/src/openrct2-ui/audio/AudioContext.cpp +++ b/src/openrct2-ui/audio/AudioContext.cpp @@ -78,22 +78,28 @@ namespace OpenRCT2::Audio return nullptr; } + std::unique_ptr source; try { - auto source = CreateAudioSource(rw, index); - - // Stream will already be in memory, so convert to target format - auto& targetFormat = _audioMixer->GetFormat(); - source = source->ToMemory(targetFormat); - - return AddSource(std::move(source)); + source = CreateAudioSource(rw, index); } catch (const std::exception& e) { - SDL_RWclose(rw); LOG_VERBOSE("Unable to create audio source: %s", e.what()); + } + + SDL_RWclose(rw); + + if (source == nullptr) + { return nullptr; } + + // Stream will already be in memory, so convert to target format + auto& targetFormat = _audioMixer->GetFormat(); + source = source->ToMemory(targetFormat); + + return AddSource(std::move(source)); } IAudioSource* CreateStreamFromWAV(std::unique_ptr stream) override diff --git a/src/openrct2-ui/audio/SDLAudioSource.cpp b/src/openrct2-ui/audio/SDLAudioSource.cpp index 3999d7170e..d7d26f5ff8 100644 --- a/src/openrct2-ui/audio/SDLAudioSource.cpp +++ b/src/openrct2-ui/audio/SDLAudioSource.cpp @@ -116,46 +116,45 @@ std::unique_ptr OpenRCT2::Audio::CreateAudioSource(SDL_RWops* rw std::unique_ptr OpenRCT2::Audio::CreateAudioSource(SDL_RWops* rw, uint32_t cssIndex) { auto numSounds = SDL_ReadLE32(rw); - if (cssIndex < numSounds) + if (cssIndex >= numSounds) { - SDL_RWseek(rw, cssIndex * 4, RW_SEEK_CUR); - - auto pcmOffset = SDL_ReadLE32(rw); - SDL_RWseek(rw, pcmOffset, RW_SEEK_SET); - - auto pcmLength = SDL_ReadLE32(rw); - - AudioFormat format; - [[maybe_unused]] auto encoding = SDL_ReadLE16(rw); - format.channels = SDL_ReadLE16(rw); - format.freq = SDL_ReadLE32(rw); - [[maybe_unused]] auto byterate = SDL_ReadLE32(rw); - [[maybe_unused]] auto blockalign = SDL_ReadLE16(rw); - [[maybe_unused]] auto bitspersample = SDL_ReadLE16(rw); - switch (bitspersample) - { - case 8: - format.format = AUDIO_U8; - break; - case 16: - format.format = AUDIO_S16LSB; - break; - default: - SDL_RWclose(rw); - throw std::runtime_error("Unsupported bits per sample"); - } - [[maybe_unused]] auto extrasize = SDL_ReadLE16(rw); - - std::vector pcmData; - pcmData.resize(pcmLength); - SDL_RWread(rw, pcmData.data(), pcmLength, 1); - - SDL_RWclose(rw); - return CreateMemoryAudioSource(format, format, std::move(pcmData)); + // Not enough sounds, caller is responsible for freeing rw + return nullptr; } - else + + SDL_RWseek(rw, cssIndex * 4, RW_SEEK_CUR); + + auto pcmOffset = SDL_ReadLE32(rw); + SDL_RWseek(rw, pcmOffset, RW_SEEK_SET); + + auto pcmLength = SDL_ReadLE32(rw); + + AudioFormat format; + // encoding, 16 bits + rw->seek(rw, 2, RW_SEEK_CUR); + format.channels = SDL_ReadLE16(rw); + format.freq = SDL_ReadLE32(rw); + // byterate, 32 bits + // blockalign, 16 bits + rw->seek(rw, 6, RW_SEEK_CUR); + auto bitspersample = SDL_ReadLE16(rw); + switch (bitspersample) { - SDL_RWclose(rw); - throw std::runtime_error("CSS does not contain required entry"); + case 8: + format.format = AUDIO_U8; + break; + case 16: + format.format = AUDIO_S16LSB; + break; + default: + throw std::runtime_error("Unsupported bits per sample"); } + // extrasize, 16 bits + rw->seek(rw, 2, RW_SEEK_CUR); + + std::vector pcmData; + pcmData.resize(pcmLength); + SDL_RWread(rw, pcmData.data(), pcmLength, 1); + + return CreateMemoryAudioSource(format, format, std::move(pcmData)); } diff --git a/src/openrct2/localisation/StringIds.h b/src/openrct2/localisation/StringIds.h index c370f33185..a9276c20f3 100644 --- a/src/openrct2/localisation/StringIds.h +++ b/src/openrct2/localisation/StringIds.h @@ -1754,6 +1754,9 @@ enum : StringId STR_GAMEPAD_DEADZONE_TOOLTIP_FORMAT = 6790, STR_GAMEPAD_SENSITIVITY_TOOLTIP_FORMAT = 6791, + // Window: Error + STR_AUDIO_FILE_TRUNCATED = 7003, + // Have to include resource strings (from scenarios and objects) for the time being now that language is partially working /* MAX_STR_COUNT = 32768 */ // MAX_STR_COUNT - upper limit for number of strings, not the current count strings }; diff --git a/src/openrct2/object/AudioSampleTable.cpp b/src/openrct2/object/AudioSampleTable.cpp index 40799b9548..509dd59d7e 100644 --- a/src/openrct2/object/AudioSampleTable.cpp +++ b/src/openrct2/object/AudioSampleTable.cpp @@ -15,6 +15,9 @@ #include "../core/File.h" #include "../core/Json.hpp" #include "../core/Path.hpp" +#include "../localisation/Formatting.h" +#include "../localisation/StringIds.h" +#include "../ui/UiContext.h" #include "Object.h" using namespace OpenRCT2; @@ -160,6 +163,17 @@ IAudioSource* AudioSampleTable::LoadSample(uint32_t index) const auto& audioContext = GetContext()->GetAudioContext(); if (entry.PathIndex) { + auto originalPosition = stream->GetPosition(); + auto numSounds = stream->ReadValue(); + stream->SetPosition(originalPosition); + + if (*entry.PathIndex >= numSounds) + { + auto& ui = GetContext()->GetUiContext(); + ui.ShowMessageBox(FormatStringID( + STR_AUDIO_FILE_TRUNCATED, entry.Asset->GetPath().c_str(), *entry.PathIndex, numSounds)); + } + result = audioContext.CreateStreamFromCSS(std::move(stream), *entry.PathIndex); } else diff --git a/src/openrct2/object/Object.cpp b/src/openrct2/object/Object.cpp index 6082d15586..b4b2fa0d53 100644 --- a/src/openrct2/object/Object.cpp +++ b/src/openrct2/object/Object.cpp @@ -375,6 +375,26 @@ std::unique_ptr ObjectAsset::GetStream() const return {}; } +const std::string& ObjectAsset::GetZipPath() const +{ + return _zipPath; +} + +const std::string& ObjectAsset::GetPath() const +{ + return _path; +} + +size_t ObjectAsset::GetHash() const +{ + // Combine hashes of zipPath and path + std::hash hasher; + auto h1 = hasher(_zipPath); + auto h2 = hasher(_path); + // Combine the hashes based on example from https://en.cppreference.com/w/cpp/utility/hash.html + return h1 ^ (h2 << 1); +} + u8string VersionString(const ObjectVersion& version) { return std::to_string(std::get<0>(version)) + "." + std::to_string(std::get<1>(version)) + "."