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

NUVIE: Fix Ultima 6 crash on loading save #4808

Merged

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
1 check passed
@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
2 participants