Skip to content

Fix ROI UAF cluster and bump Emscripten INITIAL_MEMORY to 128 MB#807

Merged
foxtacles merged 1 commit into
isledecomp:masterfrom
foxtacles:roi-uaf-fix
May 17, 2026
Merged

Fix ROI UAF cluster and bump Emscripten INITIAL_MEMORY to 128 MB#807
foxtacles merged 1 commit into
isledecomp:masterfrom
foxtacles:roi-uaf-fix

Conversation

@foxtacles

Copy link
Copy Markdown
Member

Collection of fixes based on crash reports collected from isle.pizza - these aren't easily reproducible as they require a lot of of specific game state to build up first.

Fixes a long-running use-after-free cluster in LegoAnimActorStruct::m_roiMap / LegoROI per-element references, plus adds
-sINITIAL_MEMORY=128mb to the Emscripten link options.

  1. m_roiMap array ownership. LegoAnimActorStruct now deep-copies the ROI map array and deletes copy-assignment, so LegoExtraActor's implicit copy-construction (legoextraactor.cpp:173,185) no longer shares ownership with the source presenter and double-frees at destruction.
  2. Per-element slot back-references. LegoROI tracks every LegoROI** slot that points at it; its destructor nulls each registered slot. LegoAnimActorStruct, LegoAnimPresenter, and LegoLocomotionAnimPresenter::CreateROIAndBuildMap register/unregister around their array lifetimes, so a freed ROI cannot leave a dangling pointer in an outliving array.
  3. Dormant-actor sweep on Suspend. LegoAnimationManager::Suspend now invokes ClearMaps() on every persistent LegoExtraActor, not just those in m_extras. Despawn paths (~line 1543, ~1910) previously null m_extras[i].m_roi without ClearMaps, so dormant actors could retain stale m_AnimTreePtr values into presenters whose LegoAnim* got freed at world teardown. New ClearMaps overrides land on Act2Actor, Act3Cop, Act3Brickster, Act3Shark, LegoRaceCar, and LegoExtraActor to null their cached anim pointers.

ROI UAF fixes:
- LegoAnimActorStruct deep-copies m_roiMap and deletes copy-assignment, so
  LegoExtraActor's copy-construction at legoextraactor.cpp no longer shares
  ownership with the source presenter.
- LegoROI tracks slot back-references; its destructor nulls every registered
  LegoROI** so per-element UAF in arrays like LegoAnimPresenter::m_roiMap is
  impossible. LegoAnimActorStruct / LegoAnimPresenter / LegoLocomotionAnimPresenter
  register and unregister around their array lifetimes.
- LegoAnimationManager::Suspend now invokes ClearMaps on every persistent
  LegoExtraActor (not just those currently in m_extras) so dormant actors
  cannot retain stale m_AnimTreePtr after world teardown. Adds ClearMaps
  overrides on Act2Actor, Act3Cop, Act3Brickster, Act3Shark, LegoRaceCar,
  LegoExtraActor.

INITIAL_MEMORY=128mb is re-added to the Emscripten link options.
@foxtacles foxtacles enabled auto-merge (squash) May 17, 2026 20:02
@foxtacles foxtacles merged commit 9e82c46 into isledecomp:master May 17, 2026
22 checks passed
@foxtacles foxtacles deleted the roi-uaf-fix branch May 17, 2026 20:13
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.

1 participant