Skip to content

Commit 0f7294b

Browse files
authored
Fix collision UAF on paired time models (#4864)
#### Summary - Fix timed model collision restore desync that can leave dangling `pColModel` pointers and crash in `CCollision::TestLineSphere` at `0x4174B6`. - Stop using GTA paired propagation in this path and sync paired TIME model collision state explicitly in MTA tracking. #### Motivation - This is a long standing latent bug: after multiple model/COL replacements, reconnect is not enough and collision state can stay corrupted until full game restart. - Blame target for the broken behavior: `34b4a61d40b39e0f21f42ccf031d37439223fa12` (`Resolve remaining collisionless objects (Fixes #927) (PR #2296)`), which used paired `SetColModel(..., true)` without mirrored MTA tracking for the paired model. - Related but incomplete fix: `c1824033c2e56db105730ed1cb0a06d7f720c042` (`Fix timed-object collision crash (#4782)`), which handled one path only. #### Test plan - Repro sequence with two paired TIME models and two resources replacing/restoring COL in different order no longer leaves freed pointers bound. - Crash map/session hop flow that previously reached `gta_sa.exe+0x174B6` no longer crashes in local validation run. - Non TIME model COL replacement/restore behavior stays unchanged. #### Checklist * [x] Your code should follow the [coding guidelines](https://wiki.multitheftauto.com/index.php?title=Coding_guidelines). * [x] Smaller pull requests are easier to review. If your pull request is beefy, your pull request should be reviewable commit-by-commit.
1 parent 9c3737d commit 0f7294b

1 file changed

Lines changed: 72 additions & 3 deletions

File tree

Client/game_sa/CModelInfoSA.cpp

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1594,8 +1594,13 @@ void CModelInfoSA::RestoreOriginalModel()
15941594

15951595
void CModelInfoSA::SetColModel(CColModel* pColModel)
15961596
{
1597+
if (!pColModel)
1598+
return;
1599+
15971600
// Grab the interfaces
15981601
CColModelSAInterface* pColModelInterface = pColModel->GetInterface();
1602+
if (!pColModelInterface)
1603+
return;
15991604

16001605
// Skip setting if already done
16011606
if (m_pCustomColModel == pColModel)
@@ -1619,7 +1624,7 @@ void CModelInfoSA::SetColModel(CColModel* pColModel)
16191624
// Apply some low-level hacks
16201625
pColModelInterface->m_sphere.m_collisionSlot = 0xA9;
16211626

1622-
CBaseModelInfo_SetColModel(m_pInterface, pColModelInterface, true);
1627+
CBaseModelInfo_SetColModel(m_pInterface, pColModelInterface, false);
16231628
CColAccel_addCacheCol(m_dwModelID, pColModelInterface);
16241629

16251630
// SetColModel sets bDoWeOwnTheColModel if the last parameter is truthy
@@ -1645,6 +1650,37 @@ void CModelInfoSA::SetColModel(CColModel* pColModel)
16451650
}
16461651
}
16471652
}
1653+
1654+
// Handle paired time models explicitly so MTA tracking stays in sync.
1655+
if (GetModelType() == eModelInfoType::TIME)
1656+
{
1657+
const short pairedModelId = static_cast<CTimeModelInfoSAInterface*>(m_pInterface)->timeInfo.m_wOtherTimeModel;
1658+
CModelInfo* pairedModel = pairedModelId >= 0 ? pGame->GetModelInfo(pairedModelId) : nullptr;
1659+
auto* pairedModelSA = static_cast<CModelInfoSA*>(pairedModel);
1660+
1661+
if (pairedModelSA && pairedModelSA != this)
1662+
{
1663+
CBaseModelInfoSAInterface* pairedInterface = pairedModelSA->GetInterface();
1664+
1665+
if (pairedInterface)
1666+
{
1667+
if (!pairedModelSA->m_pOriginalColModelInterface)
1668+
{
1669+
pairedModelSA->m_pOriginalColModelInterface = pairedInterface->pColModel;
1670+
pairedModelSA->m_originalFlags = pairedModelSA->GetOriginalFlags();
1671+
}
1672+
1673+
pairedModelSA->m_pCustomColModel = pColModel;
1674+
CBaseModelInfo_SetColModel(pairedInterface, pColModelInterface, false);
1675+
CColAccel_addCacheCol(pairedModelId, pColModelInterface);
1676+
pairedInterface->bDoWeOwnTheColModel = false;
1677+
pairedInterface->bIsColLoaded = false;
1678+
1679+
// Fix random foliage on custom collisions for the paired model
1680+
(reinterpret_cast<void(__cdecl*)(CBaseModelInfoSAInterface*)>(0x5DB650))(pairedInterface);
1681+
}
1682+
}
1683+
}
16481684
}
16491685
}
16501686

@@ -1655,17 +1691,50 @@ void CModelInfoSA::RestoreColModel()
16551691
// Restore original collision model and flags
16561692
if (m_pInterface && m_pOriginalColModelInterface && m_pCustomColModel)
16571693
{
1658-
CBaseModelInfo_SetColModel(m_pInterface, m_pOriginalColModelInterface, true);
1694+
CBaseModelInfo_SetColModel(m_pInterface, m_pOriginalColModelInterface, false);
16591695
CColAccel_addCacheCol(m_dwModelID, m_pInterface->pColModel);
16601696

16611697
m_pInterface->usFlags = m_originalFlags;
16621698

16631699
// Force the game to load the original collision model data, if we applied a custom collision model before
16641700
// there was any object/building, which would've provoked CColStore to request it.
1665-
if (!m_pInterface->pColModel->m_data && m_dwReferences > 1)
1701+
if (m_pInterface->pColModel && !m_pInterface->pColModel->m_data && m_dwReferences > 1)
16661702
{
16671703
pGame->GetStreaming()->RemoveModel(RESOURCE_ID_COL + m_pInterface->pColModel->m_sphere.m_collisionSlot);
16681704
}
1705+
1706+
// Handle paired time models explicitly so MTA tracking stays in sync.
1707+
if (GetModelType() == eModelInfoType::TIME)
1708+
{
1709+
const short pairedModelId = static_cast<CTimeModelInfoSAInterface*>(m_pInterface)->timeInfo.m_wOtherTimeModel;
1710+
CModelInfo* pairedModel = pairedModelId >= 0 ? pGame->GetModelInfo(pairedModelId) : nullptr;
1711+
auto* pairedModelSA = static_cast<CModelInfoSA*>(pairedModel);
1712+
1713+
if (pairedModelSA && pairedModelSA != this)
1714+
{
1715+
bool pairedRestored = false;
1716+
CBaseModelInfoSAInterface* pairedInterface = pairedModelSA->GetInterface();
1717+
if (pairedInterface && pairedModelSA->m_pOriginalColModelInterface && pairedModelSA->m_pCustomColModel)
1718+
{
1719+
CBaseModelInfo_SetColModel(pairedInterface, pairedModelSA->m_pOriginalColModelInterface, false);
1720+
CColAccel_addCacheCol(pairedModelId, pairedInterface->pColModel);
1721+
pairedInterface->usFlags = pairedModelSA->m_originalFlags;
1722+
pairedRestored = true;
1723+
1724+
if (pairedInterface->pColModel && !pairedInterface->pColModel->m_data && pairedModelSA->m_dwReferences > 1)
1725+
{
1726+
pGame->GetStreaming()->RemoveModel(RESOURCE_ID_COL + pairedInterface->pColModel->m_sphere.m_collisionSlot);
1727+
}
1728+
}
1729+
1730+
if (pairedRestored)
1731+
{
1732+
pairedModelSA->m_pCustomColModel = nullptr;
1733+
pairedModelSA->m_pOriginalColModelInterface = nullptr;
1734+
pairedModelSA->m_originalFlags = 0;
1735+
}
1736+
}
1737+
}
16691738
}
16701739

16711740
// We currently have no custom model loaded

0 commit comments

Comments
 (0)