fix: use after free and uninitialized memory (#1603)

* fix use after free and uninitialized memory

* add if check for packet lengths

* move purge down further

Its used in the if check too...
This commit is contained in:
David Markowitz 2024-05-30 21:53:03 -07:00 committed by GitHub
parent cce5755366
commit 01086d05c8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 31 additions and 22 deletions

View File

@ -179,6 +179,7 @@ int main(int argc, char** argv) {
} }
void HandlePacket(Packet* packet) { void HandlePacket(Packet* packet) {
if (packet->length < 1) return;
if (packet->data[0] == ID_DISCONNECTION_NOTIFICATION || packet->data[0] == ID_CONNECTION_LOST) { if (packet->data[0] == ID_DISCONNECTION_NOTIFICATION || packet->data[0] == ID_CONNECTION_LOST) {
LOG("A server has disconnected, erasing their connected players from the list."); LOG("A server has disconnected, erasing their connected players from the list.");
} else if (packet->data[0] == ID_NEW_INCOMING_CONNECTION) { } else if (packet->data[0] == ID_NEW_INCOMING_CONNECTION) {

View File

@ -201,7 +201,7 @@ void OnTerminate() {
} }
void MakeBacktrace() { void MakeBacktrace() {
struct sigaction sigact; struct sigaction sigact{};
sigact.sa_sigaction = CritErrHdlr; sigact.sa_sigaction = CritErrHdlr;
sigact.sa_flags = SA_RESTART | SA_SIGINFO; sigact.sa_flags = SA_RESTART | SA_SIGINFO;

View File

@ -27,6 +27,8 @@ Character::Character(uint32_t id, User* parentUser) {
m_ID = id; m_ID = id;
m_ParentUser = parentUser; m_ParentUser = parentUser;
m_OurEntity = nullptr; m_OurEntity = nullptr;
m_GMLevel = eGameMasterLevel::CIVILIAN;
m_PermissionMap = static_cast<ePermissionMap>(0);
} }
Character::~Character() { Character::~Character() {

View File

@ -464,22 +464,22 @@ private:
/** /**
* The ID of this character. First 32 bits of the ObjectID. * The ID of this character. First 32 bits of the ObjectID.
*/ */
uint32_t m_ID; uint32_t m_ID{};
/** /**
* The 64-bit unique ID used in the game. * The 64-bit unique ID used in the game.
*/ */
LWOOBJID m_ObjectID; LWOOBJID m_ObjectID{ LWOOBJID_EMPTY };
/** /**
* The user that owns this character. * The user that owns this character.
*/ */
User* m_ParentUser; User* m_ParentUser{};
/** /**
* If the character is in game, this is the entity that it represents, else nullptr. * If the character is in game, this is the entity that it represents, else nullptr.
*/ */
Entity* m_OurEntity; Entity* m_OurEntity{};
/** /**
* 0-9, the Game Master level of this character. * 0-9, the Game Master level of this character.
@ -506,17 +506,17 @@ private:
/** /**
* Whether the custom name of this character is rejected * Whether the custom name of this character is rejected
*/ */
bool m_NameRejected; bool m_NameRejected{};
/** /**
* The current amount of coins of this character * The current amount of coins of this character
*/ */
int64_t m_Coins; int64_t m_Coins{};
/** /**
* Whether the character is building * Whether the character is building
*/ */
bool m_BuildMode; bool m_BuildMode{};
/** /**
* The items equipped by the character on world load * The items equipped by the character on world load
@ -583,7 +583,7 @@ private:
/** /**
* The ID of the properties of this character * The ID of the properties of this character
*/ */
uint32_t m_PropertyCloneID; uint32_t m_PropertyCloneID{};
/** /**
* The XML data for this character, stored as string * The XML data for this character, stored as string
@ -613,7 +613,7 @@ private:
/** /**
* The last time this character logged in * The last time this character logged in
*/ */
uint64_t m_LastLogin; uint64_t m_LastLogin{};
/** /**
* The gameplay flags this character has (not just true values) * The gameplay flags this character has (not just true values)

View File

@ -12,6 +12,7 @@ User::User(const SystemAddress& sysAddr, const std::string& username, const std:
m_AccountID = 0; m_AccountID = 0;
m_Username = ""; m_Username = "";
m_SessionKey = ""; m_SessionKey = "";
m_MuteExpire = 0;
m_MaxGMLevel = eGameMasterLevel::CIVILIAN; //The max GM level this account can assign to it's characters m_MaxGMLevel = eGameMasterLevel::CIVILIAN; //The max GM level this account can assign to it's characters
m_LastCharID = 0; m_LastCharID = 0;

View File

@ -29,7 +29,8 @@
BaseCombatAIComponent::BaseCombatAIComponent(Entity* parent, const uint32_t id): Component(parent) { BaseCombatAIComponent::BaseCombatAIComponent(Entity* parent, const uint32_t id): Component(parent) {
m_Target = LWOOBJID_EMPTY; m_Target = LWOOBJID_EMPTY;
SetAiState(AiState::spawn); m_DirtyStateOrTarget = true;
m_State = AiState::spawn;
m_Timer = 1.0f; m_Timer = 1.0f;
m_StartPosition = parent->GetPosition(); m_StartPosition = parent->GetPosition();
m_MovementAI = nullptr; m_MovementAI = nullptr;

View File

@ -875,8 +875,6 @@ void InventoryComponent::UnEquipItem(Item* item) {
RemoveSlot(item->GetInfo().equipLocation); RemoveSlot(item->GetInfo().equipLocation);
PurgeProxies(item);
UnequipScripts(item); UnequipScripts(item);
Game::entityManager->SerializeEntity(m_Parent); Game::entityManager->SerializeEntity(m_Parent);
@ -886,6 +884,8 @@ void InventoryComponent::UnEquipItem(Item* item) {
PropertyManagementComponent::Instance()->GetParent()->OnZonePropertyModelRemovedWhileEquipped(m_Parent); PropertyManagementComponent::Instance()->GetParent()->OnZonePropertyModelRemovedWhileEquipped(m_Parent);
Game::zoneManager->GetZoneControlObject()->OnZonePropertyModelRemovedWhileEquipped(m_Parent); Game::zoneManager->GetZoneControlObject()->OnZonePropertyModelRemovedWhileEquipped(m_Parent);
} }
PurgeProxies(item);
} }
@ -1505,10 +1505,10 @@ void InventoryComponent::PurgeProxies(Item* item) {
const auto root = item->GetParent(); const auto root = item->GetParent();
if (root != LWOOBJID_EMPTY) { if (root != LWOOBJID_EMPTY) {
item = FindItemById(root); Item* itemRoot = FindItemById(root);
if (item != nullptr) { if (itemRoot != nullptr) {
UnEquipItem(item); UnEquipItem(itemRoot);
} }
return; return;

View File

@ -382,6 +382,7 @@ int main(int argc, char** argv) {
} }
void HandlePacket(Packet* packet) { void HandlePacket(Packet* packet) {
if (packet->length < 1) return;
if (packet->data[0] == ID_DISCONNECTION_NOTIFICATION) { if (packet->data[0] == ID_DISCONNECTION_NOTIFICATION) {
LOG("A server has disconnected"); LOG("A server has disconnected");

View File

@ -529,6 +529,7 @@ int main(int argc, char** argv) {
} }
void HandlePacketChat(Packet* packet) { void HandlePacketChat(Packet* packet) {
if (packet->length < 1) return;
if (packet->data[0] == ID_DISCONNECTION_NOTIFICATION || packet->data[0] == ID_CONNECTION_LOST) { if (packet->data[0] == ID_DISCONNECTION_NOTIFICATION || packet->data[0] == ID_CONNECTION_LOST) {
LOG("Lost our connection to chat, zone(%i), instance(%i)", Game::server->GetZoneID(), Game::server->GetInstanceID()); LOG("Lost our connection to chat, zone(%i), instance(%i)", Game::server->GetZoneID(), Game::server->GetInstanceID());
@ -542,7 +543,7 @@ void HandlePacketChat(Packet* packet) {
chatConnected = true; chatConnected = true;
} }
if (packet->data[0] == ID_USER_PACKET_ENUM) { if (packet->data[0] == ID_USER_PACKET_ENUM && packet->length >= 4) {
if (static_cast<eConnectionType>(packet->data[1]) == eConnectionType::CHAT) { if (static_cast<eConnectionType>(packet->data[1]) == eConnectionType::CHAT) {
switch (static_cast<eChatMessageType>(packet->data[3])) { switch (static_cast<eChatMessageType>(packet->data[3])) {
case eChatMessageType::WORLD_ROUTE_PACKET: { case eChatMessageType::WORLD_ROUTE_PACKET: {
@ -557,8 +558,9 @@ void HandlePacketChat(Packet* packet) {
//Write our stream outwards: //Write our stream outwards:
CBITSTREAM; CBITSTREAM;
for (BitSize_t i = 0; i < inStream.GetNumberOfBytesUsed(); i++) { unsigned char data;
bitStream.Write(packet->data[i + 16]); //16 bytes == header + playerID to skip while (inStream.Read(data)) {
bitStream.Write(data);
} }
SEND_PACKET; //send routed packet to player SEND_PACKET; //send routed packet to player
@ -659,7 +661,7 @@ void HandlePacketChat(Packet* packet) {
} }
void HandleMasterPacket(Packet* packet) { void HandleMasterPacket(Packet* packet) {
if (packet->length < 2) return;
if (static_cast<eConnectionType>(packet->data[1]) != eConnectionType::MASTER || packet->length < 4) return; if (static_cast<eConnectionType>(packet->data[1]) != eConnectionType::MASTER || packet->length < 4) return;
switch (static_cast<eMasterMessageType>(packet->data[3])) { switch (static_cast<eMasterMessageType>(packet->data[3])) {
case eMasterMessageType::REQUEST_PERSISTENT_ID_RESPONSE: { case eMasterMessageType::REQUEST_PERSISTENT_ID_RESPONSE: {
@ -785,6 +787,7 @@ void HandleMasterPacket(Packet* packet) {
} }
void HandlePacket(Packet* packet) { void HandlePacket(Packet* packet) {
if (packet->length < 1) return;
if (packet->data[0] == ID_DISCONNECTION_NOTIFICATION || packet->data[0] == ID_CONNECTION_LOST) { if (packet->data[0] == ID_DISCONNECTION_NOTIFICATION || packet->data[0] == ID_CONNECTION_LOST) {
auto user = UserManager::Instance()->GetUser(packet->systemAddress); auto user = UserManager::Instance()->GetUser(packet->systemAddress);
if (!user) return; if (!user) return;
@ -1207,8 +1210,8 @@ void HandlePacket(Packet* packet) {
//Now write the rest of the data: //Now write the rest of the data:
auto data = inStream.GetData(); auto data = inStream.GetData();
for (uint32_t i = 0; i < size; ++i) { for (uint32_t i = 23; i - 23 < size && i < packet->length; ++i) {
bitStream.Write(data[i + 23]); bitStream.Write(data[i]);
} }
Game::chatServer->Send(&bitStream, SYSTEM_PRIORITY, RELIABLE_ORDERED, 0, Game::chatSysAddr, false); Game::chatServer->Send(&bitStream, SYSTEM_PRIORITY, RELIABLE_ORDERED, 0, Game::chatSysAddr, false);