Skip to content

Commit be0f696

Browse files
authored
[GEN][ZH] Fix heap-use-after-free in ScoreScaleUpTransition::draw and dangling pointers to GameWindow in WindowTransitions (#848)
1 parent 84f902f commit be0f696

File tree

10 files changed

+162
-14
lines changed

10 files changed

+162
-14
lines changed

Generals/Code/GameEngine/Include/GameClient/GameWindow.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
class GameWindow;
6666
class WindowLayout;
6767
class GameFont;
68+
class TransitionWindow;
6869
struct GameWindowEditData;
6970

7071
///////////////////////////////////////////////////////////////////////////////
@@ -240,6 +241,9 @@ friend class GameWindowManager;
240241
/// draw border for this window only, NO child windows or anything
241242
virtual void winDrawBorder( void ) = 0;
242243

244+
void linkTransitionWindow( TransitionWindow* transitionWindow );
245+
void unlinkTransitionWindow( TransitionWindow* transitionWindow );
246+
243247
Int winSetWindowId( Int id ); ///< set id for this window
244248
Int winGetWindowId( void ); ///< return window id for this window
245249
Int winSetSize( Int width, Int height ); ///< set size
@@ -380,6 +384,8 @@ friend class GameWindowManager;
380384
void freeImages( void ) { }
381385
Bool isEnabled( void ); ///< see if we and our parents are enabled
382386

387+
void unlinkFromTransitionWindows( void );
388+
383389
void normalizeWindowRegion( void ); ///< put UL corner in window region.lo
384390

385391
GameWindow *findFirstLeaf( void ); ///< return first leaf of branch
@@ -422,6 +428,9 @@ friend class GameWindowManager;
422428
// game window edit data for the GUIEditor only
423429
GameWindowEditData *m_editData;
424430

431+
// vector of window transitions that have a relation to the current GameWindow
432+
std::vector<TransitionWindow*> m_transitionWindows;
433+
425434
}; // end class GameWindow
426435

427436
// ModalWindow ----------------------------------------------------------------

Generals/Code/GameEngine/Include/GameClient/GameWindowTransitions.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ class Transition
125125

126126
virtual void skip( void ) = 0;
127127

128+
void unlinkGameWindow(GameWindow* win) { if ( m_win == win ) m_win = NULL; }
128129
Bool isFinished( void ) { return m_isFinished; }
129130
Int getFrameLength( void ){ return m_frameLength; }
130131
protected:
@@ -609,7 +610,9 @@ class TransitionWindow
609610
void reverse( Int totalFrames );
610611
Int getTotalFrames( void );
611612
void skip( void );
612-
void draw( void );
613+
void draw( void );
614+
615+
void unlinkGameWindow( GameWindow* win );
613616

614617
// INI parsed vars
615618
AsciiString m_winName;

Generals/Code/GameEngine/Source/GameClient/GUI/GameWindow.cpp

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
#include "GameClient/GadgetStaticText.h"
6262
#include "GameClient/Mouse.h"
6363
#include "GameClient/SelectionXlat.h"
64+
#include "GameClient/GameWindowTransitions.h"
6465
#ifdef RTS_INTERNAL
6566
// for occasional debugging...
6667
//#pragma optimize("", off)
@@ -136,8 +137,51 @@ GameWindow::~GameWindow( void )
136137
delete m_editData;
137138
m_editData = NULL;
138139

140+
unlinkFromTransitionWindows();
141+
139142
} // end ~GameWindow
140143

144+
// GameWindow::linkTransitionWindow ============================================
145+
//=============================================================================
146+
void GameWindow::linkTransitionWindow( TransitionWindow* transitionWindow )
147+
{
148+
149+
m_transitionWindows.push_back(transitionWindow);
150+
151+
} // end linkTransitionWindow
152+
153+
// GameWindow::unlinkTransitionWindow =========================================
154+
//=============================================================================
155+
void GameWindow::unlinkTransitionWindow( TransitionWindow* transitionWindow )
156+
{
157+
158+
std::vector<TransitionWindow*>::iterator it = m_transitionWindows.begin();
159+
while ( it != m_transitionWindows.end() )
160+
{
161+
if ( *it == transitionWindow )
162+
{
163+
*it = m_transitionWindows.back();
164+
m_transitionWindows.pop_back();
165+
return;
166+
}
167+
++it;
168+
}
169+
170+
} // end unlinkTransitionWindow
171+
172+
// GameWindow::unlinkFromTransitionWindows =========================================
173+
//=============================================================================
174+
void GameWindow::unlinkFromTransitionWindows( void )
175+
{
176+
177+
while ( !m_transitionWindows.empty() )
178+
{
179+
m_transitionWindows.back()->unlinkGameWindow(this);
180+
m_transitionWindows.pop_back();
181+
}
182+
183+
} // end unlinkFromTransitionWindows
184+
141185
// GameWindow::normalizeWindowRegion ==========================================
142186
/** Puts the upper left corner in the window's region.lo field */
143187
//=============================================================================

Generals/Code/GameEngine/Source/GameClient/GUI/GameWindowTransitions.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,9 @@ TransitionWindow::TransitionWindow( void )
158158

159159
TransitionWindow::~TransitionWindow( void )
160160
{
161+
if (m_win)
162+
m_win->unlinkTransitionWindow(this);
163+
161164
m_win = NULL;
162165
if(m_transition)
163166
delete m_transition;
@@ -180,6 +183,10 @@ Bool TransitionWindow::init( void )
180183
m_transition = getTransitionForStyle( m_style );
181184
m_transition->init(m_win);
182185

186+
// TheSuperHackers @fix Mauller 15/05/2025 Link TransitionWindow to the GameWindow so the GameWindow can unlink itself when it is destroyed
187+
if(m_win)
188+
m_win->linkTransitionWindow(this);
189+
183190
return TRUE;
184191
}
185192

@@ -218,6 +225,15 @@ void TransitionWindow::draw( void )
218225
m_transition->draw();
219226
}
220227

228+
void TransitionWindow::unlinkGameWindow(GameWindow* win)
229+
{
230+
if (m_win != win)
231+
return;
232+
233+
m_transition->unlinkGameWindow(win);
234+
m_win = NULL;
235+
}
236+
221237
Int TransitionWindow::getTotalFrames( void )
222238
{
223239
if(m_transition)

Generals/Code/GameEngine/Source/GameClient/GUI/GameWindowTransitionsStyles.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,8 @@ void FadeTransition::draw( void )
692692
{
693693
if(!m_win)
694694
return;
695+
if(m_drawState <= FADETRANSITION_START || m_drawState >= FADETRANSITION_END)
696+
return;
695697
const Image *image = m_win->winGetEnabledImage(0);
696698
switch (m_drawState)
697699
{
@@ -854,9 +856,9 @@ void ScaleUpTransition::draw( void )
854856
{
855857
if(!m_win)
856858
return;
857-
const Image *image = m_win->winGetEnabledImage(0);
858859
if(m_drawState <= SCALEUPTRANSITION_START || m_drawState >= SCALEUPTRANSITION_END)
859860
return;
861+
const Image *image = m_win->winGetEnabledImage(0);
860862
Int x = m_centerPos.x - ((m_incrementSize.x * m_drawState) / 2);
861863
Int y = m_centerPos.y - ((m_incrementSize.y * m_drawState) / 2);
862864
Int x1 = x + m_incrementSize.x * m_drawState;
@@ -977,9 +979,9 @@ void ScoreScaleUpTransition::draw( void )
977979
{
978980
if(!m_win)
979981
return;
980-
const Image *image = m_win->winGetEnabledImage(0);
981982
if(m_drawState <= SCORESCALEUPTRANSITION_START || m_drawState >= SCORESCALEUPTRANSITION_END)
982983
return;
984+
const Image *image = m_win->winGetEnabledImage(0);
983985
Int x = m_centerPos.x - ((m_incrementSize.x * m_drawState) / 2);
984986
Int y = m_centerPos.y - ((m_incrementSize.y * m_drawState) / 2);
985987
Int x1 = x + m_incrementSize.x * m_drawState;
@@ -1094,9 +1096,9 @@ void MainMenuScaleUpTransition::draw( void )
10941096
{
10951097
if(!m_win)
10961098
return;
1097-
const Image *image = m_growWin->winGetEnabledImage(0);
10981099
if(m_drawState <= MAINMENUSCALEUPTRANSITION_START || m_drawState >= MAINMENUSCALEUPTRANSITION_END)
10991100
return;
1101+
const Image *image = m_growWin->winGetEnabledImage(0);
11001102
Int x = m_pos.x + ((m_incrementPos.x * m_drawState));
11011103
Int y = m_pos.y + ((m_incrementPos.y * m_drawState));
11021104
Int x1 = x + m_size.x + ((m_incrementSize.x * m_drawState));
@@ -1214,9 +1216,9 @@ void MainMenuMediumScaleUpTransition::draw( void )
12141216
{
12151217
if(!m_win)
12161218
return;
1217-
const Image *image = m_win->winGetEnabledImage(0);
12181219
if(m_drawState <= MAINMENUMEDIUMSCALEUPTRANSITION_START || m_drawState >= MAINMENUMEDIUMSCALEUPTRANSITION_END)
12191220
return;
1221+
const Image *image = m_win->winGetEnabledImage(0);
12201222
Int x = m_pos.x - ((m_incrementSize.x * m_drawState) /2);
12211223
Int y = m_pos.y - ((m_incrementSize.y * m_drawState) / 2);
12221224
Int x1 = m_pos.x + m_size.x + ((m_incrementSize.x * m_drawState) / 2);
@@ -1325,9 +1327,9 @@ void MainMenuSmallScaleDownTransition::draw( void )
13251327
{
13261328
if(!m_win)
13271329
return;
1328-
const Image *image = m_win->winGetEnabledImage(0);
13291330
if(m_drawState <= MAINMENUSMALLSCALEDOWNTRANSITION_START || m_drawState >= MAINMENUSMALLSCALEDOWNTRANSITION_END)
13301331
return;
1332+
const Image *image = m_win->winGetEnabledImage(0);
13311333
Int x = m_pos.x - ((m_incrementSize.x * m_drawState) /2);
13321334
Int y = m_pos.y - ((m_incrementSize.y * m_drawState) / 2);
13331335
Int x1 = m_pos.x + m_size.x + ((m_incrementSize.x * m_drawState) / 2);
@@ -1757,7 +1759,7 @@ void ControlBarArrowTransition::reverse( void )
17571759

17581760
void ControlBarArrowTransition::draw( void )
17591761
{
1760-
if(m_drawState <0)
1762+
if(m_drawState < CONTROLBARARROWTRANSITION_START)
17611763
return;
17621764
if(m_drawState < CONTROLBARARROWTRANSITION_BEGIN_FADE)
17631765
{

GeneralsMD/Code/GameEngine/Include/GameClient/GameWindow.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
class GameWindow;
6666
class WindowLayout;
6767
class GameFont;
68+
class TransitionWindow;
6869
struct GameWindowEditData;
6970

7071
///////////////////////////////////////////////////////////////////////////////
@@ -242,6 +243,9 @@ friend class GameWindowManager;
242243
/// draw border for this window only, NO child windows or anything
243244
virtual void winDrawBorder( void ) = 0;
244245

246+
void linkTransitionWindow( TransitionWindow* transitionWindow );
247+
void unlinkTransitionWindow( TransitionWindow* transitionWindow );
248+
245249
Int winSetWindowId( Int id ); ///< set id for this window
246250
Int winGetWindowId( void ); ///< return window id for this window
247251
Int winSetSize( Int width, Int height ); ///< set size
@@ -382,6 +386,8 @@ friend class GameWindowManager;
382386
void freeImages( void ) { }
383387
Bool isEnabled( void ); ///< see if we and our parents are enabled
384388

389+
void unlinkFromTransitionWindows( void );
390+
385391
void normalizeWindowRegion( void ); ///< put UL corner in window region.lo
386392

387393
GameWindow *findFirstLeaf( void ); ///< return first leaf of branch
@@ -424,6 +430,9 @@ friend class GameWindowManager;
424430
// game window edit data for the GUIEditor only
425431
GameWindowEditData *m_editData;
426432

433+
// vector of window transitions that have a relation to the current GameWindow
434+
std::vector<TransitionWindow*> m_transitionWindows;
435+
427436
}; // end class GameWindow
428437

429438
// ModalWindow ----------------------------------------------------------------

GeneralsMD/Code/GameEngine/Include/GameClient/GameWindowTransitions.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ class Transition
125125

126126
virtual void skip( void ) = 0;
127127

128+
void unlinkGameWindow(GameWindow* win) { if ( m_win == win ) m_win = NULL; }
128129
Bool isFinished( void ) { return m_isFinished; }
129130
Int getFrameLength( void ){ return m_frameLength; }
130131
protected:
@@ -609,7 +610,9 @@ class TransitionWindow
609610
void reverse( Int totalFrames );
610611
Int getTotalFrames( void );
611612
void skip( void );
612-
void draw( void );
613+
void draw( void );
614+
615+
void unlinkGameWindow( GameWindow* win );
613616

614617
// INI parsed vars
615618
AsciiString m_winName;

GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GameWindow.cpp

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
#include "GameClient/GadgetStaticText.h"
6262
#include "GameClient/Mouse.h"
6363
#include "GameClient/SelectionXlat.h"
64+
#include "GameClient/GameWindowTransitions.h"
6465
#ifdef RTS_INTERNAL
6566
// for occasional debugging...
6667
//#pragma optimize("", off)
@@ -136,8 +137,51 @@ GameWindow::~GameWindow( void )
136137
delete m_editData;
137138
m_editData = NULL;
138139

140+
unlinkFromTransitionWindows();
141+
139142
} // end ~GameWindow
140143

144+
// GameWindow::linkTransitionWindow ============================================
145+
//=============================================================================
146+
void GameWindow::linkTransitionWindow( TransitionWindow* transitionWindow )
147+
{
148+
149+
m_transitionWindows.push_back(transitionWindow);
150+
151+
} // end linkTransitionWindow
152+
153+
// GameWindow::unlinkTransitionWindow =========================================
154+
//=============================================================================
155+
void GameWindow::unlinkTransitionWindow( TransitionWindow* transitionWindow )
156+
{
157+
158+
std::vector<TransitionWindow*>::iterator it = m_transitionWindows.begin();
159+
while ( it != m_transitionWindows.end() )
160+
{
161+
if ( *it == transitionWindow )
162+
{
163+
*it = m_transitionWindows.back();
164+
m_transitionWindows.pop_back();
165+
return;
166+
}
167+
++it;
168+
}
169+
170+
} // end unlinkTransitionWindow
171+
172+
// GameWindow::unlinkFromTransitionWindows =========================================
173+
//=============================================================================
174+
void GameWindow::unlinkFromTransitionWindows( void )
175+
{
176+
177+
while ( !m_transitionWindows.empty() )
178+
{
179+
m_transitionWindows.back()->unlinkGameWindow(this);
180+
m_transitionWindows.pop_back();
181+
}
182+
183+
} // end unlinkFromTransitionWindows
184+
141185
// GameWindow::normalizeWindowRegion ==========================================
142186
/** Puts the upper left corner in the window's region.lo field */
143187
//=============================================================================

GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GameWindowTransitions.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,9 @@ TransitionWindow::TransitionWindow( void )
158158

159159
TransitionWindow::~TransitionWindow( void )
160160
{
161+
if (m_win)
162+
m_win->unlinkTransitionWindow(this);
163+
161164
m_win = NULL;
162165
if(m_transition)
163166
delete m_transition;
@@ -180,6 +183,10 @@ Bool TransitionWindow::init( void )
180183
m_transition = getTransitionForStyle( m_style );
181184
m_transition->init(m_win);
182185

186+
// TheSuperHackers @fix Mauller 15/05/2025 Link TransitionWindow to the GameWindow so the GameWindow can unlink itself when it is destroyed
187+
if(m_win)
188+
m_win->linkTransitionWindow(this);
189+
183190
return TRUE;
184191
}
185192

@@ -218,6 +225,15 @@ void TransitionWindow::draw( void )
218225
m_transition->draw();
219226
}
220227

228+
void TransitionWindow::unlinkGameWindow(GameWindow* win)
229+
{
230+
if (m_win != win)
231+
return;
232+
233+
m_transition->unlinkGameWindow(win);
234+
m_win = NULL;
235+
}
236+
221237
Int TransitionWindow::getTotalFrames( void )
222238
{
223239
if(m_transition)

0 commit comments

Comments
 (0)