Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cell-change crash caused by use-after-free of BGSAttackDataMap #725

Merged

Conversation

miredirex
Copy link
Contributor

@miredirex miredirex commented Oct 23, 2024

Assigned here during TESNPC::Initialize()

void TESNPC::Initialize() noexcept
{
auto pPlayerBaseForm = Cast<TESNPC>(PlayerCharacter::Get()->baseForm);
// These values are all defaulted, if the other actor did not modify them they won't be loaded, therefore we need to force them before load
attackDataForm.attackDataMap = pPlayerBaseForm->attackDataForm.attackDataMap;
npcClass = pPlayerBaseForm->npcClass;
combatStyle = pPlayerBaseForm->combatStyle;
raceForm.race = pPlayerBaseForm->raceForm.race;
outfits[0] = pPlayerBaseForm->outfits[0];
#if TP_SKYRIM
spellList.Initialize();
#endif
// End defaults
flags |= 0x200000;
}

^ Notice how attackDataMap is taken from the PlayerCharacter singleton and assigned to a remote player's TESNPC. When that TESNPC unloads, the game (supposedly) tries to deallocate its resources, including this map

Changed raw pointer to a NiPointer which uses refcount, saving us from the crash

Partially addresses #723 (0x140443C43 crash site)

@RobbeBryssinck RobbeBryssinck merged commit b850014 into tiltedphoques:dev Oct 25, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants