Duel Zone Crashes

[image]

Pointers are not checked and people can send in bogus data. I added in bold what needs to be changed.

client_packet.cpp

Old Section:

	Entity* entity = entity_list.GetID(ds->target_id);
	Entity* initiator = entity_list.GetID(ds->entity_id);
	if(!entity->IsClient() || !initiator->IsClient())
		return;

void Client::Handle_OP_DuelResponse(const EQApplicationPacket *app)
{
	if(app->size != sizeof(DuelResponse_Struct))
		return;
	DuelResponse_Struct* ds = (DuelResponse_Struct*) app->pBuffer;


	Client* entity = entity_list.GetClientByID(ds->target_id);
	Client* initiator = entity_list.GetClientByID(ds->entity_id);
	
	if ( !entity )
	{
		LogFile->write(EQEMuLog::Debug, "Handle_OP_DuelResponse had a bad entity passed by %s.", GetName());
		return;
	}
	else if ( !initiator )
	{
		LogFile->write(EQEMuLog::Debug, "Handle_OP_DuelResponse had a bad initiator passed by %s.", GetName());
		return;
	}

	entity->CastToClient()->SetDuelTarget(0);
	entity->CastToClient()->SetDueling(false);
	initiator->CastToClient()->SetDuelTarget(0);
	initiator->CastToClient()->SetDueling(false);
	if(GetID() == initiator->GetID())
		entity->CastToClient()->Message_StringID(10,DUEL_DECLINE,initiator->GetName());
	else
		initiator->CastToClient()->Message_StringID(10,DUEL_DECLINE,entity->GetName());
	return;
}

[image]

This function required more of a rewrite

New Code:

void Client::Handle_OP_RequestDuel(const EQApplicationPacket *app)
{
	if(app->size != sizeof(Duel_Struct))
		return;

	EQApplicationPacket* outapp = app->Copy();
	Duel_Struct* ds = (Duel_Struct*) outapp->pBuffer;
	int32 duel = ds->duel_initiator;
	ds->duel_initiator = ds->duel_target;
	ds->duel_target = duel;
	Client* entity = entity_list.GetClientByID(ds->duel_target);

	// Kings & Bandits - null checks for duels
	if ( !entity )
	{
		LogFile->write(EQEMuLog::Debug, "Handle_OP_RequestDuel had a bad entity passed by %s.", GetName());
		return;
	}
	// Kings & Bandits - duelrequest - check if they are already planning to duel someone and ignore it, if we are the duel target jump down below and just delete the outapp
	else if((entity->CastToClient()->GetDuelTarget() != 0 && entity->CastToClient()->GetDuelTarget() != this->GetID())) {
		Message_StringID(10,DUEL_CONSIDERING,entity->GetName());
		return;
	}
	else if ( entity->CastToClient()->IsDueling() )
	{
		Message_StringID(10,DUEL_INPROGRESS,entity->GetName());
	}
	else if(IsDueling()) {
		Message_StringID(10,DUEL_INPROGRESS);
		return;
	}
	

	if(GetID() != ds->duel_target && entity->IsClient() && GetDuelTarget() == 0 && !IsDueling() && !entity->CastToClient()->IsDueling() && entity->CastToClient()->GetDuelTarget() == 0) {
		SetDuelTarget(ds->duel_target);
		entity->CastToClient()->SetDuelTarget(GetID());
		ds->duel_target = ds->duel_initiator;
		entity->CastToClient()->FastQueuePacket(&outapp);
		entity->CastToClient()->SetDueling(false);
		SetDueling(false);
	}
	else
		safe_delete(outapp);
	return;
}

Old Code:

void Client::Handle_OP_RequestDuel(const EQApplicationPacket *app)
{
	if(app->size != sizeof(Duel_Struct))
		return;

	EQApplicationPacket* outapp = app->Copy();
	Duel_Struct* ds = (Duel_Struct*) outapp->pBuffer;
	int32 duel = ds->duel_initiator;
	ds->duel_initiator = ds->duel_target;
	ds->duel_target = duel;
	Entity* entity = entity_list.GetID(ds->duel_target);
	if(GetID() != ds->duel_target && entity->IsClient() && (entity->CastToClient()->IsDueling() && entity->CastToClient()->GetDuelTarget() != 0)) {
		Message_StringID(10,DUEL_CONSIDERING,entity->GetName());
		return;
	}
	if(IsDueling()) {
		Message_StringID(10,DUEL_INPROGRESS);
		return;
	}

	if(GetID() != ds->duel_target && entity->IsClient() && GetDuelTarget() == 0 && !IsDueling() && !entity->CastToClient()->IsDueling() && entity->CastToClient()->GetDuelTarget() == 0) {
		SetDuelTarget(ds->duel_target);
		entity->CastToClient()->SetDuelTarget(GetID());
		ds->duel_target = ds->duel_initiator;
		entity->CastToClient()->FastQueuePacket(&outapp);
		entity->CastToClient()->SetDueling(false);
		SetDueling(false);
	}
	else
		safe_delete(outapp);
	return;
}

[image]

the RequestDuel should have safe_delete(outapp); on its returns too - forgot that they originally copied the packet instead of using it.