From 20e12dcc3affb2009859cfd47cda65d4bf7f5950 Mon Sep 17 00:00:00 2001
From: Gleb Mazovetskiy <glex.spb@gmail.com>
Date: Mon, 28 Nov 2022 03:45:15 +0000
Subject: [PATCH] selhero: Fix heap-use-after-free

A popup-like error dialog in selhero resulted in a heap-use-after-free:

https://gist.github.com/glebm/f014bd87f066d2b79965b7c48bd8f6d7

This is because the popup's `Deinit()` freed the background art.

The fix is simply to not free the background art.
This is OK because the popup never has a background.

It used to load an empty background in the past just to load the
palette but luckily it no longer does (otherwise this would require more
work).

Also, fixes dialog rendering:

1. Fixes what is rendered behind the dialog.
2. Draws the mouse (if possible) regardless of whether the background is
   present.
3. Clears the screen if the background doesn't cover it completely.

Fixes #4195
---
 Source/DiabloUI/diabloui.cpp |  7 ++++++-
 Source/DiabloUI/diabloui.h   |  5 ++++-
 Source/DiabloUI/dialogs.cpp  | 13 ++++---------
 Source/DiabloUI/selhero.cpp  |  2 +-
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/Source/DiabloUI/diabloui.cpp b/Source/DiabloUI/diabloui.cpp
index 4d1ba55fe..f903dfce0 100644
--- a/Source/DiabloUI/diabloui.cpp
+++ b/Source/DiabloUI/diabloui.cpp
@@ -164,6 +164,11 @@ void UiInitList(void (*fnFocus)(int value), void (*fnSelect)(int value), void (*
 	}
 }
 
+void UiRenderListItems()
+{
+	UiRenderItems(gUiItems);
+}
+
 void UiInitList_clear()
 {
 	SelectedItem = 0;
@@ -772,7 +777,7 @@ void UiPollAndRender(std::optional<tl::function_ref<bool(SDL_Event &)>> eventHan
 		UiHandleEvents(&event);
 	}
 	HandleMenuAction(GetMenuHeldUpDownAction());
-	UiRenderItems(gUiItems);
+	UiRenderListItems();
 	DrawMouse();
 	UiFadeIn();
 
diff --git a/Source/DiabloUI/diabloui.h b/Source/DiabloUI/diabloui.h
index 83f36adc4..5c5e99412 100644
--- a/Source/DiabloUI/diabloui.h
+++ b/Source/DiabloUI/diabloui.h
@@ -106,13 +106,16 @@ void UiAddLogo(std::vector<std::unique_ptr<UiItemBase>> *vecDialog);
 void UiFocusNavigationSelect();
 void UiFocusNavigationEsc();
 void UiFocusNavigationYesNo();
+
 void UiInitList(void (*fnFocus)(int value), void (*fnSelect)(int value), void (*fnEsc)(), const std::vector<std::unique_ptr<UiItemBase>> &items, bool wraps = false, void (*fnFullscreen)() = nullptr, bool (*fnYesNo)() = nullptr, size_t selectedItem = 0);
+void UiRenderListItems();
+void UiInitList_clear();
+
 void UiClearScreen();
 void UiPollAndRender(std::optional<tl::function_ref<bool(SDL_Event &)>> eventHandler = std::nullopt);
 void UiRenderItem(const UiItemBase &item);
 void UiRenderItems(const std::vector<UiItemBase *> &items);
 void UiRenderItems(const std::vector<std::unique_ptr<UiItemBase>> &items);
-void UiInitList_clear();
 ClxSprite UiGetHeroDialogSprite(size_t heroClassIndex);
 
 void mainmenu_restart_repintro();
diff --git a/Source/DiabloUI/dialogs.cpp b/Source/DiabloUI/dialogs.cpp
index 21f35a1b2..91b157e43 100644
--- a/Source/DiabloUI/dialogs.cpp
+++ b/Source/DiabloUI/dialogs.cpp
@@ -101,7 +101,6 @@ void Deinit()
 {
 	ownedDialogSprite = std::nullopt;
 	vecOkDialog.clear();
-	ArtBackground = std::nullopt;
 	FreeDialogButtonGraphics();
 }
 
@@ -130,15 +129,11 @@ void DialogLoop(const std::vector<std::unique_ptr<UiItemBase>> &items, const std
 			UiHandleEvents(&event);
 		}
 
-		if (renderBehind.empty()) {
-			SDL_FillRect(DiabloUiSurface(), nullptr, 0x000000);
-		} else {
-			UiRenderItems(renderBehind);
-		}
+		UiClearScreen();
+		UiRenderItems(renderBehind);
+		UiRenderListItems();
 		UiRenderItems(items);
-		if (ArtBackground) {
-			DrawMouse();
-		}
+		DrawMouse();
 		UiFadeIn();
 	} while (!dialogEnd);
 }
diff --git a/Source/DiabloUI/selhero.cpp b/Source/DiabloUI/selhero.cpp
index 35b7c0304..9b4ce67a1 100644
--- a/Source/DiabloUI/selhero.cpp
+++ b/Source/DiabloUI/selhero.cpp
@@ -310,7 +310,7 @@ void SelheroNameSelect(int /*value*/)
 			SelheroLoadSelect(1);
 			return;
 		}
-		UiErrorOkDialog(_(/* TRANSLATORS: Error Message */ "Unable to create character."), vecSelDlgItems);
+		UiErrorOkDialog(_(/* TRANSLATORS: Error Message */ "Unable to create character."), vecSelHeroDialog);
 	}
 
 	memset(selhero_heroInfo.name, '\0', sizeof(selhero_heroInfo.name));