From 70536c50bdb9fc8a04c15b6b6030a718eac3655a Mon Sep 17 00:00:00 2001 From: Anders Jenbo Date: Mon, 19 May 2025 19:02:24 +0200 Subject: [PATCH] Fix remaning UBSAN issues (#115) --- .../src/common/mxcontrolpresenter.cpp | 7 ++++++- .../legoomni/src/paths/legopathboundary.cpp | 20 +++++++++++++++---- .../src/video/legophonemepresenter.cpp | 2 +- LEGO1/omni/src/video/flic.cpp | 5 +++-- LEGO1/omni/src/video/mxflcpresenter.cpp | 7 ++++--- LEGO1/tgl/d3drm/group.cpp | 8 +++++++- 6 files changed, 37 insertions(+), 12 deletions(-) diff --git a/LEGO1/lego/legoomni/src/common/mxcontrolpresenter.cpp b/LEGO1/lego/legoomni/src/common/mxcontrolpresenter.cpp index b17c221e..76c26b91 100644 --- a/LEGO1/lego/legoomni/src/common/mxcontrolpresenter.cpp +++ b/LEGO1/lego/legoomni/src/common/mxcontrolpresenter.cpp @@ -9,6 +9,7 @@ #include "mxtimer.h" #include "mxutilities.h" +#include #include #include @@ -78,7 +79,11 @@ void MxControlPresenter::EndAction() MxBool MxControlPresenter::FUN_10044270(MxS32 p_x, MxS32 p_y, MxPresenter* p_presenter) { assert(p_presenter); - MxStillPresenter* presenter = (MxStillPresenter*) p_presenter; + MxStillPresenter* presenter = dynamic_cast(p_presenter); + if (!presenter) { + SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Invalid presenter"); + return FALSE; + } if (m_unk0x4c == 3) { MxStillPresenter* map = (MxStillPresenter*) m_list.front(); diff --git a/LEGO1/lego/legoomni/src/paths/legopathboundary.cpp b/LEGO1/lego/legoomni/src/paths/legopathboundary.cpp index 8f832630..e4c1282b 100644 --- a/LEGO1/lego/legoomni/src/paths/legopathboundary.cpp +++ b/LEGO1/lego/legoomni/src/paths/legopathboundary.cpp @@ -6,6 +6,8 @@ #include "legopathactor.h" #include "legopathstruct.h" +#include + DECOMP_SIZE_ASSERT(LegoPathBoundary, 0x74) // FUNCTION: LEGO1 0x10056a70 @@ -357,8 +359,12 @@ MxU32 LegoPathBoundary::FUN_10057fe0(LegoAnimPresenter* p_presenter) // TODO: This only seems to match if the type is not the same as the type of the // key value of the set. Figure out which type the set (or parameter) actually uses. // Also see call to .find in LegoPathController::FUN_10046050 - m_presenters.insert(static_cast(p_presenter)); - return 1; + if (auto* locomotionPresenter = dynamic_cast(p_presenter)) { + m_presenters.insert(locomotionPresenter); + return 1; + } + SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Invalid locomotion"); + return 0; } // FUNCTION: LEGO1 0x100586e0 @@ -369,8 +375,14 @@ MxU32 LegoPathBoundary::FUN_100586e0(LegoAnimPresenter* p_presenter) // TODO: This only seems to match if the type is not the same as the type of the // key value of the set. Figure out which type the set (or parameter) actually uses. // Also see call to .find in LegoPathController::FUN_10046050 - if (m_presenters.find(static_cast(p_presenter)) != m_presenters.end()) { - m_presenters.erase(static_cast(p_presenter)); + auto* locomotionPresenter = dynamic_cast(p_presenter); + if (!locomotionPresenter) { + SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Invalid locomotion"); + return 0; + } + auto it = m_presenters.find(locomotionPresenter); + if (it != m_presenters.end()) { + m_presenters.erase(it); return 1; } } diff --git a/LEGO1/lego/legoomni/src/video/legophonemepresenter.cpp b/LEGO1/lego/legoomni/src/video/legophonemepresenter.cpp index 46d1edb8..7de88799 100644 --- a/LEGO1/lego/legoomni/src/video/legophonemepresenter.cpp +++ b/LEGO1/lego/legoomni/src/video/legophonemepresenter.cpp @@ -93,7 +93,7 @@ void LegoPhonemePresenter::LoadFrame(MxStreamChunk* p_chunk) { MxU8* data = p_chunk->GetData(); - m_rectCount = *(MxS32*) data; + m_rectCount = UnalignedRead(data); data += sizeof(MxS32); MxRect32* rects = (MxRect32*) data; diff --git a/LEGO1/omni/src/video/flic.cpp b/LEGO1/omni/src/video/flic.cpp index cfd7541e..feb6308a 100644 --- a/LEGO1/omni/src/video/flic.cpp +++ b/LEGO1/omni/src/video/flic.cpp @@ -159,9 +159,10 @@ void WritePixelPairs( short is_odd = p_count & 1; p_count >>= 1; - WORD* dst = (WORD*) (((p_bitmapHeader->biWidth + 3) & -4) * p_row + p_column + p_pixelData); + BYTE* dst = ((p_bitmapHeader->biWidth + 3) & -4) * p_row + p_column + p_pixelData; while (--p_count >= 0) { - *dst++ = p_pixel; + memcpy(dst, &p_pixel, sizeof(WORD)); + dst += sizeof(WORD); } if (is_odd) { diff --git a/LEGO1/omni/src/video/mxflcpresenter.cpp b/LEGO1/omni/src/video/mxflcpresenter.cpp index eb075357..1bc8462e 100644 --- a/LEGO1/omni/src/video/mxflcpresenter.cpp +++ b/LEGO1/omni/src/video/mxflcpresenter.cpp @@ -49,10 +49,10 @@ void MxFlcPresenter::LoadFrame(MxStreamChunk* p_chunk) { MxU8* data = p_chunk->GetData(); - MxS32 rectCount = *(MxS32*) data; + MxS32 rectCount = UnalignedRead(data); data += sizeof(MxS32); - MxRect32* rects = (MxRect32*) data; + MxU8* rects = data; data += rectCount * sizeof(MxRect32); MxBool decodedColorMap; @@ -69,7 +69,8 @@ void MxFlcPresenter::LoadFrame(MxStreamChunk* p_chunk) } for (MxS32 i = 0; i < rectCount; i++) { - MxRect32 rect(rects[i]); + MxRect32 rect = UnalignedRead(rects); + rects += sizeof(MxRect32); rect += m_location; MVideoManager()->InvalidateRect(rect); } diff --git a/LEGO1/tgl/d3drm/group.cpp b/LEGO1/tgl/d3drm/group.cpp index 686b1540..5e1f855e 100644 --- a/LEGO1/tgl/d3drm/group.cpp +++ b/LEGO1/tgl/d3drm/group.cpp @@ -1,5 +1,7 @@ #include "impl.h" +#include + using namespace TglImpl; // FUNCTION: LEGO1 0x100a31d0 @@ -89,7 +91,11 @@ Result GroupImpl::Add(const Group* pGroup) // FUNCTION: LEGO1 0x100a3430 Result GroupImpl::Add(const MeshBuilder* pMeshBuilder) { - const MeshBuilderImpl* pMeshBuilderImpl = static_cast(pMeshBuilder); + const MeshBuilderImpl* pMeshBuilderImpl = dynamic_cast(pMeshBuilder); + if (!pMeshBuilderImpl) { + SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Invalid mesh builder"); + return Result::Error; + } return ResultVal(m_data->AddVisual(pMeshBuilderImpl->ImplementationData())); }