KEMBAR78
NUVIE: Fix Ultima 6 crash on loading save by PushmePullyu · Pull Request #4808 · scummvm/scummvm · GitHub
Skip to content

Conversation

@PushmePullyu
Copy link
Contributor

Fix a use-after-free bug. The chain of events causing it is as follows:

In-game the user changes the current view (e.g. by pressing TAB to go to inventory view). The user next presses ESC to open the GameMenuDialog and then clicks the "Load Game" button. A save slot is selected and then loaded.

The resulting call chain is:

GUI_Button::Activate_button()
  GameMenuDialog::callback()
    GameMenuDialog::close_dialog() <-- This marks GameMenuDialog
                                       for deletion by the GUI
    g_engine->loadGameDialog()
      GUI::AddWidget() <-------------- This deletes GameMenuDialog
                                       and its children, including
                                       the "Load Game" button

Control then returns to GUI_Button::Activate_button() with "this" containing the address of the freed button.

An attempt to call Redraw() finally dereferences the invalid pointer.

This is fixed by calling loadGameDialog() before close_dialog().

Fix a use-after-free bug. The chain of events causing it is as follows:

In-game the user changes the current view (e.g. by pressing TAB to
go to inventory view). The user next presses ESC to open the
GameMenuDialog and then clicks the "Load Game" button. A save slot is
selected and then loaded.

The resulting call chain is:

GUI_Button::Activate_button()
  GameMenuDialog::callback()
    GameMenuDialog::close_dialog() <-- This marks GameMenuDialog
                                       for deletion by the GUI
    g_engine->loadGameDialog()
      GUI::AddWidget() <-------------- This deletes GameMenuDialog
                                       and its children, including
                                       the "Load Game" button

Control then returns to GUI_Button::Activate_button() with "this"
containing the address of the freed button.

An attempt to call Redraw() finally dereferences the invalid pointer.

This is fixed by calling loadGameDialog() before close_dialog().
@dreammaster
Copy link
Member

Nice. Thanks for tracking down and fixing the problem.

@dreammaster dreammaster merged commit 05ae183 into scummvm:master Mar 14, 2023
@PushmePullyu PushmePullyu deleted the fix-u6-crash-on-savegame-load branch March 14, 2023 20:22
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