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

CPlatTrigger could access dangling pointer #1725

Open
SamVanheer opened this issue Jul 17, 2016 · 1 comment
Open

CPlatTrigger could access dangling pointer #1725

SamVanheer opened this issue Jul 17, 2016 · 1 comment

Comments

@SamVanheer
Copy link

CPlatTrigger could access a dangling pointer.

Non-toggled func_plat and derived entities will create a trigger volume so walking onto the platform when it's at its resting position triggers it.

However, if the platform were killtargeted, the trigger would point at a non-existent entity. This could cause a crash. Using an EHANDLE instead of a pointer should fix it.

@SamVanheer
Copy link
Author

These changes are required to fix this:

Change this line:

CFuncPlat *m_pPlatform;

To this:

EHANDLE m_hPlatform;

Change this method:

halflife/dlls/plats.cpp

Lines 351 to 374 in c7240b9

void CPlatTrigger :: SpawnInsideTrigger( CFuncPlat *pPlatform )
{
m_pPlatform = pPlatform;
// Create trigger entity, "point" it at the owning platform, give it a touch method
pev->solid = SOLID_TRIGGER;
pev->movetype = MOVETYPE_NONE;
pev->origin = pPlatform->pev->origin;
// Establish the trigger field's size
Vector vecTMin = m_pPlatform->pev->mins + Vector ( 25 , 25 , 0 );
Vector vecTMax = m_pPlatform->pev->maxs + Vector ( 25 , 25 , 8 );
vecTMin.z = vecTMax.z - ( m_pPlatform->m_vecPosition1.z - m_pPlatform->m_vecPosition2.z + 8 );
if (m_pPlatform->pev->size.x <= 50)
{
vecTMin.x = (m_pPlatform->pev->mins.x + m_pPlatform->pev->maxs.x) / 2;
vecTMax.x = vecTMin.x + 1;
}
if (m_pPlatform->pev->size.y <= 50)
{
vecTMin.y = (m_pPlatform->pev->mins.y + m_pPlatform->pev->maxs.y) / 2;
vecTMax.y = vecTMin.y + 1;
}
UTIL_SetSize ( pev, vecTMin, vecTMax );
}

To this:

void CPlatTrigger :: SpawnInsideTrigger( CFuncPlat *pPlatform )
{
	m_hPlatform = pPlatform;
	// Create trigger entity, "point" it at the owning platform, give it a touch method
	pev->solid		= SOLID_TRIGGER;
	pev->movetype	= MOVETYPE_NONE;
	pev->origin = pPlatform->pev->origin;

	// Establish the trigger field's size
	Vector vecTMin = pPlatform->pev->mins + Vector ( 25 , 25 , 0 );
	Vector vecTMax = pPlatform->pev->maxs + Vector ( 25 , 25 , 8 );
	vecTMin.z = vecTMax.z - (pPlatform->m_vecPosition1.z - pPlatform->m_vecPosition2.z + 8 );
	if (pPlatform->pev->size.x <= 50)
	{
		vecTMin.x = (pPlatform->pev->mins.x + pPlatform->pev->maxs.x) / 2;
		vecTMax.x = vecTMin.x + 1;
	}
	if (pPlatform->pev->size.y <= 50)
	{
		vecTMin.y = (pPlatform->pev->mins.y + pPlatform->pev->maxs.y) / 2;
		vecTMax.y = vecTMin.y + 1;
	}
	UTIL_SetSize ( pev, vecTMin, vecTMax );
}

Change this method:

halflife/dlls/plats.cpp

Lines 380 to 396 in c7240b9

void CPlatTrigger :: Touch( CBaseEntity *pOther )
{
// Ignore touches by non-players
entvars_t* pevToucher = pOther->pev;
if ( !FClassnameIs (pevToucher, "player") )
return;
// Ignore touches by corpses
if (!pOther->IsAlive()||!m_pPlatform||!m_pPlatform->pev)
return;
// Make linked platform go up/down.
if (m_pPlatform->m_toggle_state == TS_AT_BOTTOM)
m_pPlatform->GoUp();
else if (m_pPlatform->m_toggle_state == TS_AT_TOP)
m_pPlatform->pev->nextthink = m_pPlatform->pev->ltime + 1;// delay going down
}

To this:

void CPlatTrigger :: Touch( CBaseEntity *pOther )
{
	//Platform was removed, remove trigger
	if (!m_hPlatform || !m_hPlatform->pev)
	{
		UTIL_Remove(this);
		return;
	}

	// Ignore touches by non-players
	entvars_t*	pevToucher = pOther->pev;
	if ( !FClassnameIs (pevToucher, "player") )
		return;

	// Ignore touches by corpses
	if (!pOther->IsAlive())
		return;

	CFuncPlat* platform = static_cast<CFuncPlat*>(static_cast<CBaseEntity*>(m_hPlatform));
	
	// Make linked platform go up/down.
	if (platform->m_toggle_state == TS_AT_BOTTOM)
		platform->GoUp();
	else if (platform->m_toggle_state == TS_AT_TOP)
		platform->pev->nextthink = platform->pev->ltime + 1;// delay going down
}

This will allow the trigger to detect when the platform has been removed and remove itself in turn when touched once, without accessing the deleted platform entity's memory.

Ideally the platform would keep track of the trigger and delete it on demand but that's not guaranteed to work with old save games.

This change can be made in official games because the trigger isn't saved and restored, it's recreated on load.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants