Particle manager accesses particle objects after they are destroyed

[SamVanheer]

The particle manager has a use-after-free bug that results in a particle that is considered visible, but that has been destroyed still being accessed in order to draw it.

The code that causes this works like this (in CMiniMem::ProcessAll):

if (effect->CheckVisibility())
{
	auto player = gEngfuncs.GetLocalPlayer();
	effect->SetPlayerDistance((player->origin - effect->m_vOrigin).LengthSquared());

	m_pVisibleParticles[m_iParticlesDrawn++].pVisibleParticle = effect;
}

if (!IsGamePaused())
{
	effect->Think(time);
}

if (0 != effect->m_flDieTime && time >= effect->m_flDieTime)
{
	effect->Die();

	deleteBlock(particle);
}

The code first checks if the particle is visible, and if it is it is added to the list of visible particles. Then it checks if the particle has reached its death time and is deleted.

The particle is not removed from the list of visible particles. Its destructor is not called and the memory remains allocated (particle memory is allocated from a single large pool of memory). When the particle is drawn later on the object is still valid, though its memory is flagged as free and could be re-allocated if another particle were to create a new particle between the old particle's deletion and the particle being drawn.

To fix this the particle should be checked for deletion before checking the visibility. If the particle is deleted then it should skip checking visibility altogether. The particle should also be updated before being checked for deletion, so that if its death time is changed it will work properly:

if (!IsGamePaused())
{
	effect->Think(time);
}

if (0 != effect->m_flDieTime && time >= effect->m_flDieTime)
{
	effect->Die();

	deleteBlock(particle);
}
else
{
	if (effect->CheckVisibility())
	{
		auto player = gEngfuncs.GetLocalPlayer();
		effect->SetPlayerDistance((player->origin - effect->m_vOrigin).LengthSquared());

		m_pVisibleParticles[m_iParticlesDrawn++].pVisibleParticle = effect;
	}
}