Skip to content

Commit 84fcd5e

Browse files
committed
tweaks: more safe fixes
1 parent ad352a3 commit 84fcd5e

File tree

9 files changed

+185
-63
lines changed

9 files changed

+185
-63
lines changed

Code/server/GameServer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,7 @@ void GameServer::OnDisconnection(const ConnectionId_t aConnectionId, EDisconnect
630630
notify.Username = pPlayer->GetUsername();
631631
SendToPlayers(notify);
632632

633-
entt::entity playerCharacter = pPlayer->GetCharacter().value_or(static_cast<entt::entity>(0));
633+
entt::entity playerCharacter = pPlayer->GetCharacter().value_or(entt::null);
634634

635635
// Cleanup all entities that we own
636636
auto ownerView = m_pWorld->view<OwnerComponent>();

Code/server/Services/ActorValueService.cpp

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,16 @@ void ActorValueService::OnActorValueChanges(const PacketEvent<RequestActorValueC
2424
{
2525
auto& message = acMessage.Packet;
2626

27+
const auto entity = m_world.TryResolveEntity(message.Id);
28+
if (!entity)
29+
{
30+
spdlog::debug("Actor value change requested for unknown entity {:X}", message.Id);
31+
return;
32+
}
33+
2734
auto actorValuesView = m_world.view<ActorValuesComponent, OwnerComponent>();
2835

29-
auto it = actorValuesView.find(static_cast<entt::entity>(message.Id));
36+
auto it = actorValuesView.find(*entity);
3037

3138
if (it != actorValuesView.end())
3239
{
@@ -41,18 +48,24 @@ void ActorValueService::OnActorValueChanges(const PacketEvent<RequestActorValueC
4148
notify.Id = acMessage.Packet.Id;
4249
notify.Values = acMessage.Packet.Values;
4350

44-
const entt::entity cEntity = static_cast<entt::entity>(message.Id);
45-
if (!GameServer::Get()->SendToPlayersInRange(notify, cEntity, acMessage.pPlayer))
51+
if (!GameServer::Get()->SendToPlayersInRange(notify, *entity, acMessage.pPlayer))
4652
spdlog::error("{}: SendToPlayersInRange failed", __FUNCTION__);
4753
}
4854

4955
void ActorValueService::OnActorMaxValueChanges(const PacketEvent<RequestActorMaxValueChanges>& acMessage) const noexcept
5056
{
5157
auto& message = acMessage.Packet;
5258

59+
const auto entity = m_world.TryResolveEntity(message.Id);
60+
if (!entity)
61+
{
62+
spdlog::debug("Actor max value change requested for unknown entity {:X}", message.Id);
63+
return;
64+
}
65+
5366
auto actorValuesView = m_world.view<ActorValuesComponent, OwnerComponent>();
5467

55-
auto it = actorValuesView.find(static_cast<entt::entity>(message.Id));
68+
auto it = actorValuesView.find(*entity);
5669

5770
if (it != actorValuesView.end())
5871
{
@@ -67,19 +80,25 @@ void ActorValueService::OnActorMaxValueChanges(const PacketEvent<RequestActorMax
6780
notify.Id = message.Id;
6881
notify.Values = message.Values;
6982

70-
const entt::entity cEntity = static_cast<entt::entity>(message.Id);
71-
if (!GameServer::Get()->SendToPlayersInRange(notify, cEntity, acMessage.pPlayer))
83+
if (!GameServer::Get()->SendToPlayersInRange(notify, *entity, acMessage.pPlayer))
7284
spdlog::error("{}: SendToPlayersInRange failed", __FUNCTION__);
7385
}
7486

7587
void ActorValueService::OnHealthChangeBroadcast(const PacketEvent<RequestHealthChangeBroadcast>& acMessage) const noexcept
7688
{
7789
auto& message = acMessage.Packet;
7890

91+
const auto entity = m_world.TryResolveEntity(message.Id);
92+
if (!entity)
93+
{
94+
spdlog::debug("Health change broadcast requested for unknown entity {:X}", message.Id);
95+
return;
96+
}
97+
7998
// TODO(cosideci): should server side health not be updated?
8099
auto actorValuesView = m_world.view<ActorValuesComponent, OwnerComponent>();
81100

82-
auto it = actorValuesView.find(static_cast<entt::entity>(message.Id));
101+
auto it = actorValuesView.find(*entity);
83102

84103
if (it != actorValuesView.end())
85104
{
@@ -89,21 +108,27 @@ void ActorValueService::OnHealthChangeBroadcast(const PacketEvent<RequestHealthC
89108
}
90109

91110
NotifyHealthChangeBroadcast notify;
92-
notify.Id = message.Id;
111+
notify.Id = message.Id;
93112
notify.DeltaHealth = message.DeltaHealth;
94113

95-
const entt::entity cEntity = static_cast<entt::entity>(message.Id);
96-
if (!GameServer::Get()->SendToPlayersInRange(notify, cEntity, acMessage.pPlayer))
114+
if (!GameServer::Get()->SendToPlayersInRange(notify, *entity, acMessage.pPlayer))
97115
spdlog::error("{}: SendToPlayersInRange failed", __FUNCTION__);
98116
}
99117

100118
void ActorValueService::OnDeathStateChange(const PacketEvent<RequestDeathStateChange>& acMessage) const noexcept
101119
{
102120
auto& message = acMessage.Packet;
103121

122+
const auto entity = m_world.TryResolveEntity(message.Id);
123+
if (!entity)
124+
{
125+
spdlog::debug("Death state change requested for unknown entity {:X}", message.Id);
126+
return;
127+
}
128+
104129
auto characterView = m_world.view<CharacterComponent, OwnerComponent>();
105130

106-
const auto it = characterView.find(static_cast<entt::entity>(message.Id));
131+
const auto it = characterView.find(*entity);
107132

108133
if (it != characterView.end())
109134
{
@@ -116,7 +141,6 @@ void ActorValueService::OnDeathStateChange(const PacketEvent<RequestDeathStateCh
116141
notify.Id = message.Id;
117142
notify.IsDead = message.IsDead;
118143

119-
const entt::entity cEntity = static_cast<entt::entity>(message.Id);
120-
if (!GameServer::Get()->SendToPlayersInRange(notify, cEntity, acMessage.pPlayer))
144+
if (!GameServer::Get()->SendToPlayersInRange(notify, *entity, acMessage.pPlayer))
121145
spdlog::error("{}: SendToPlayersInRange failed", __FUNCTION__);
122146
}

Code/server/Services/CharacterService.cpp

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -41,20 +41,9 @@
4141
#include <Messages/NotifyRelinquishControl.h>
4242

4343
#include <Setting.h>
44-
#include <optional>
4544
namespace
4645
{
4746
Console::Setting bEnableXpSync{"Gameplay:bEnableXpSync", "Syncs combat XP within the party", true};
48-
49-
[[nodiscard]] std::optional<entt::entity> TryResolveEntity(World& aWorld, const uint32_t aServerId) noexcept
50-
{
51-
const auto entity = static_cast<entt::entity>(aServerId);
52-
53-
if (!aWorld.valid(entity))
54-
return std::nullopt;
55-
56-
return entity;
57-
}
5847
}
5948

6049
CharacterService::CharacterService(World& aWorld, entt::dispatcher& aDispatcher) noexcept
@@ -269,7 +258,7 @@ void CharacterService::OnOwnershipTransferRequest(const PacketEvent<RequestOwner
269258
{
270259
auto& message = acMessage.Packet;
271260

272-
const auto entity = TryResolveEntity(m_world, message.ServerId);
261+
const auto entity = m_world.TryResolveEntity(message.ServerId);
273262
if (!entity)
274263
{
275264
spdlog::warn("Client {:X} requested ownership transfer of an entity that doesn't exist, server id: {:X}", acMessage.pPlayer->GetConnectionId(), message.ServerId);
@@ -361,7 +350,7 @@ void CharacterService::OnOwnershipTransferEvent(const OwnershipTransferEvent& ac
361350

362351
void CharacterService::OnCharacterRemoveEvent(const CharacterRemoveEvent& acEvent) const noexcept
363352
{
364-
const auto entity = TryResolveEntity(m_world, acEvent.ServerId);
353+
const auto entity = m_world.TryResolveEntity(acEvent.ServerId);
365354
if (!entity)
366355
{
367356
spdlog::warn("Character remove event received for unknown entity {:X}", acEvent.ServerId);
@@ -422,7 +411,7 @@ void CharacterService::OnReferencesMoveRequest(const PacketEvent<ClientReference
422411
for (auto& entry : message.Updates)
423412
{
424413
const auto entityId = entry.first;
425-
const auto resolved = TryResolveEntity(m_world, entityId);
414+
const auto resolved = m_world.TryResolveEntity(entityId);
426415
if (!resolved)
427416
{
428417
spdlog::debug("{:X} requested move of {:X} but entity does not exist", acMessage.pPlayer->GetConnectionId(), entityId);
@@ -483,7 +472,7 @@ void CharacterService::OnFactionsChanges(const PacketEvent<RequestFactionsChange
483472

484473
for (auto& [id, factions] : message.Changes)
485474
{
486-
const auto entity = TryResolveEntity(m_world, id);
475+
const auto entity = m_world.TryResolveEntity(id);
487476
if (!entity)
488477
{
489478
spdlog::debug("{:X} requested faction update for unknown entity {:X}", acMessage.pPlayer->GetConnectionId(), id);
@@ -511,7 +500,7 @@ void CharacterService::OnMountRequest(const PacketEvent<MountRequest>& acMessage
511500
notify.RiderId = message.RiderId;
512501
notify.MountId = message.MountId;
513502

514-
const auto entity = TryResolveEntity(m_world, message.MountId);
503+
const auto entity = m_world.TryResolveEntity(message.MountId);
515504
if (!entity)
516505
{
517506
spdlog::debug("{:X} requested mount broadcast for unknown entity {:X}", acMessage.pPlayer->GetConnectionId(), message.MountId);
@@ -530,7 +519,7 @@ void CharacterService::OnNewPackageRequest(const PacketEvent<NewPackageRequest>&
530519
notify.ActorId = message.ActorId;
531520
notify.PackageId = message.PackageId;
532521

533-
const auto entity = TryResolveEntity(m_world, message.ActorId);
522+
const auto entity = m_world.TryResolveEntity(message.ActorId);
534523
if (!entity)
535524
{
536525
spdlog::debug("{:X} requested package update for unknown entity {:X}", acMessage.pPlayer->GetConnectionId(), message.ActorId);
@@ -543,7 +532,7 @@ void CharacterService::OnNewPackageRequest(const PacketEvent<NewPackageRequest>&
543532

544533
void CharacterService::OnRequestRespawn(const PacketEvent<RequestRespawn>& acMessage) const noexcept
545534
{
546-
const auto entity = TryResolveEntity(m_world, acMessage.Packet.ActorId);
535+
const auto entity = m_world.TryResolveEntity(acMessage.Packet.ActorId);
547536
if (!entity)
548537
{
549538
spdlog::warn("Respawn requested for unknown actor id {:X}", acMessage.Packet.ActorId);
@@ -604,7 +593,7 @@ void CharacterService::OnDialogueRequest(const PacketEvent<DialogueRequest>& acM
604593
notify.ServerId = message.ServerId;
605594
notify.SoundFilename = message.SoundFilename;
606595

607-
const auto entity = TryResolveEntity(m_world, message.ServerId);
596+
const auto entity = m_world.TryResolveEntity(message.ServerId);
608597
if (!entity)
609598
{
610599
spdlog::debug("{:X} requested dialogue broadcast for unknown entity {:X}", acMessage.pPlayer->GetConnectionId(), message.ServerId);
@@ -623,7 +612,7 @@ void CharacterService::OnSubtitleRequest(const PacketEvent<SubtitleRequest>& acM
623612
notify.ServerId = message.ServerId;
624613
notify.Text = message.Text;
625614

626-
const auto entity = TryResolveEntity(m_world, message.ServerId);
615+
const auto entity = m_world.TryResolveEntity(message.ServerId);
627616
if (!entity)
628617
{
629618
spdlog::debug("{:X} requested subtitle broadcast for unknown entity {:X}", acMessage.pPlayer->GetConnectionId(), message.ServerId);
@@ -726,7 +715,7 @@ void CharacterService::CreateCharacter(const PacketEvent<AssignCharacterRequest>
726715
void CharacterService::TransferOwnership(Player* apPlayer, const uint32_t acServerId,
727716
const ActorData& acActorData) const noexcept
728717
{
729-
const auto entity = TryResolveEntity(m_world, acServerId);
718+
const auto entity = m_world.TryResolveEntity(acServerId);
730719
if (!entity)
731720
{
732721
spdlog::warn("Client {:X} requested ownership of an entity that doesn't exist ({:X})!", apPlayer->GetConnectionId(), acServerId);

Code/server/Services/CombatService.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,13 @@ void CombatService::OnProjectileLaunchRequest(const PacketEvent<ProjectileLaunch
5050
notify.UnkBool1 = packet.UnkBool1;
5151
notify.UnkBool2 = packet.UnkBool2;
5252

53-
const auto cShooterEntity = static_cast<entt::entity>(packet.ShooterID);
54-
if (!GameServer::Get()->SendToPlayersInRange(notify, cShooterEntity, acMessage.GetSender()))
53+
const auto entity = m_world.TryResolveEntity(packet.ShooterID);
54+
if (!entity)
55+
{
56+
spdlog::debug("Projectile launch from unknown entity {:X}", packet.ShooterID);
57+
return;
58+
}
59+
60+
if (!GameServer::Get()->SendToPlayersInRange(notify, *entity, acMessage.GetSender()))
5561
spdlog::error("{}: SendToPlayersInRange failed", __FUNCTION__);
5662
}

Code/server/Services/InventoryService.cpp

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,24 +29,42 @@ void InventoryService::OnInventoryChanges(const PacketEvent<RequestInventoryChan
2929
{
3030
auto& message = acMessage.Packet;
3131

32-
auto view = m_world.view<InventoryComponent>();
32+
const auto entity = m_world.TryResolveEntity(message.ServerId);
33+
if (!entity)
34+
{
35+
spdlog::warn("Inventory update requested for unknown entity {:X}", message.ServerId);
36+
return;
37+
}
3338

34-
const auto it = view.find(static_cast<entt::entity>(message.ServerId));
39+
auto view = m_world.view<InventoryComponent, OwnerComponent>();
40+
41+
const auto it = view.find(*entity);
3542

3643
if (it != view.end())
3744
{
45+
auto& ownerComponent = view.get<OwnerComponent>(*it);
46+
if (ownerComponent.GetOwner() != acMessage.pPlayer)
47+
{
48+
spdlog::warn("Inventory change denied for {:X}: player {:X} not owner", message.ServerId, acMessage.pPlayer->GetConnectionId());
49+
return;
50+
}
51+
3852
auto& inventoryComponent = view.get<InventoryComponent>(*it);
3953
inventoryComponent.Content.AddOrRemoveEntry(message.Item);
4054
}
55+
else
56+
{
57+
spdlog::warn("Inventory change requested for entity {:X} without InventoryComponent", message.ServerId);
58+
return;
59+
}
4160

4261
if (!message.UpdateClients)
4362
return;
4463

4564
NotifyInventoryChanges notify;
4665
notify.ServerId = message.ServerId;
4766
notify.Item = message.Item;
48-
49-
notify.Drop = false; // bEnableItemDrops ? message.Drop : false;
67+
notify.Drop = message.Drop;
5068
if (message.HasDropInstanceId)
5169
{
5270
notify.HasDropInstanceId = true;
@@ -63,24 +81,42 @@ void InventoryService::OnInventoryChanges(const PacketEvent<RequestInventoryChan
6381
notify.DropRotation = message.DropRotation;
6482
}
6583

66-
const entt::entity cOrigin = static_cast<entt::entity>(message.ServerId);
67-
if (!GameServer::Get()->SendToPlayersInRange(notify, cOrigin, acMessage.GetSender()))
84+
if (!GameServer::Get()->SendToPlayersInRange(notify, *entity, acMessage.GetSender()))
6885
spdlog::error("{}: SendToPlayersInRange failed", __FUNCTION__);
6986
}
7087

7188
void InventoryService::OnEquipmentChanges(const PacketEvent<RequestEquipmentChanges>& acMessage) noexcept
7289
{
7390
auto& message = acMessage.Packet;
7491

75-
auto view = m_world.view<InventoryComponent>();
92+
const auto entity = m_world.TryResolveEntity(message.ServerId);
93+
if (!entity)
94+
{
95+
spdlog::warn("Equipment update requested for unknown entity {:X}", message.ServerId);
96+
return;
97+
}
98+
99+
auto view = m_world.view<InventoryComponent, OwnerComponent>();
76100

77-
const auto it = view.find(static_cast<entt::entity>(message.ServerId));
101+
const auto it = view.find(*entity);
78102

79103
if (it != view.end())
80104
{
105+
auto& ownerComponent = view.get<OwnerComponent>(*it);
106+
if (ownerComponent.GetOwner() != acMessage.pPlayer)
107+
{
108+
spdlog::warn("Equipment change denied for {:X}: player {:X} not owner", message.ServerId, acMessage.pPlayer->GetConnectionId());
109+
return;
110+
}
111+
81112
auto& inventoryComponent = view.get<InventoryComponent>(*it);
82113
inventoryComponent.Content.UpdateEquipment(message.CurrentInventory);
83114
}
115+
else
116+
{
117+
spdlog::warn("Equipment change requested for entity {:X} without InventoryComponent", message.ServerId);
118+
return;
119+
}
84120

85121
NotifyEquipmentChanges notify;
86122
notify.ServerId = message.ServerId;
@@ -91,17 +127,23 @@ void InventoryService::OnEquipmentChanges(const PacketEvent<RequestEquipmentChan
91127
notify.IsSpell = message.IsSpell;
92128
notify.IsShout = message.IsShout;
93129

94-
const entt::entity cOrigin = static_cast<entt::entity>(message.ServerId);
95-
if (!GameServer::Get()->SendToPlayersInRange(notify, cOrigin, acMessage.GetSender()))
130+
if (!GameServer::Get()->SendToPlayersInRange(notify, *entity, acMessage.GetSender()))
96131
spdlog::error("{}: SendToPlayersInRange failed", __FUNCTION__);
97132
}
98133

99134
void InventoryService::OnWeaponDrawnRequest(const PacketEvent<DrawWeaponRequest>& acMessage) noexcept
100135
{
101136
auto& message = acMessage.Packet;
102137

138+
const auto entity = m_world.TryResolveEntity(message.Id);
139+
if (!entity)
140+
{
141+
spdlog::debug("Weapon drawn request for unknown entity {:X}", message.Id);
142+
return;
143+
}
144+
103145
auto characterView = m_world.view<CharacterComponent, OwnerComponent>();
104-
const auto it = characterView.find(static_cast<entt::entity>(message.Id));
146+
const auto it = characterView.find(*entity);
105147

106148
if (it != std::end(characterView) && characterView.get<OwnerComponent>(*it).GetOwner() == acMessage.pPlayer)
107149
{

0 commit comments

Comments
 (0)