From 59ec28a5a4bfabf054c0eeb7f288aff52ce793fe Mon Sep 17 00:00:00 2001 From: NinjaOfLU Date: Wed, 13 Apr 2022 01:58:00 +0100 Subject: [PATCH 1/4] Prevent integer underflow in item removal Previously, the only check that the user wasn't trashing more items than they had was clientsided, and this could be bypassed by contacting the server to remove items via a console or the like, and then trashing them before the server could respond, resulting in the count for the items being less than iStackCount. This check prevents that underflow. --- dGame/dGameMessages/GameMessages.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dGame/dGameMessages/GameMessages.cpp b/dGame/dGameMessages/GameMessages.cpp index 1ede443b..35e1a13c 100644 --- a/dGame/dGameMessages/GameMessages.cpp +++ b/dGame/dGameMessages/GameMessages.cpp @@ -5441,7 +5441,7 @@ void GameMessages::HandleRemoveItemFromInventory(RakNet::BitStream* inStream, En } } - item->SetCount(item->GetCount() - iStackCount, true); + item->SetCount(item->GetCount() - std::min(item->GetCount(), iStackCount), true); EntityManager::Instance()->SerializeEntity(entity); auto* missionComponent = entity->GetComponent(); From 1d1f47938769a6a40f0472c6643e68e8813b9227 Mon Sep 17 00:00:00 2001 From: NinjaOfLU Date: Thu, 14 Apr 2022 00:11:35 +0100 Subject: [PATCH 2/4] Ensure items correctly removed from missions Doubt it would have affected anyone, but technically if you had a mission to collect something interactable, and you deleted the items at the same time as interacting with something, this would have counted incorrectly. I'm being defensive because I was an idiot who couldn't read, but in my defence, it was late when I made the first edit, and I'm also a blundering idiot! --- dGame/dGameMessages/GameMessages.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dGame/dGameMessages/GameMessages.cpp b/dGame/dGameMessages/GameMessages.cpp index 35e1a13c..c8c1e7aa 100644 --- a/dGame/dGameMessages/GameMessages.cpp +++ b/dGame/dGameMessages/GameMessages.cpp @@ -5448,7 +5448,7 @@ void GameMessages::HandleRemoveItemFromInventory(RakNet::BitStream* inStream, En if (missionComponent != nullptr) { - missionComponent->Progress(MissionTaskType::MISSION_TASK_TYPE_ITEM_COLLECTION, item->GetLot(), LWOOBJID_EMPTY, "", -iStackCount); + missionComponent->Progress(MissionTaskType::MISSION_TASK_TYPE_ITEM_COLLECTION, item->GetLot(), LWOOBJID_EMPTY, "", -std::min(item->GetCount(), iStackCount); } } } From cdbb42badff3247cec8a2e4e016669d0d676f036 Mon Sep 17 00:00:00 2001 From: NinjaOfLU Date: Thu, 14 Apr 2022 00:15:45 +0100 Subject: [PATCH 3/4] Added a bracket... -_- Maybe I should write and test code on my computer rather than using Git as an IDE and commits as the save button... --- dGame/dGameMessages/GameMessages.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dGame/dGameMessages/GameMessages.cpp b/dGame/dGameMessages/GameMessages.cpp index c8c1e7aa..e1fb21ac 100644 --- a/dGame/dGameMessages/GameMessages.cpp +++ b/dGame/dGameMessages/GameMessages.cpp @@ -5448,7 +5448,7 @@ void GameMessages::HandleRemoveItemFromInventory(RakNet::BitStream* inStream, En if (missionComponent != nullptr) { - missionComponent->Progress(MissionTaskType::MISSION_TASK_TYPE_ITEM_COLLECTION, item->GetLot(), LWOOBJID_EMPTY, "", -std::min(item->GetCount(), iStackCount); + missionComponent->Progress(MissionTaskType::MISSION_TASK_TYPE_ITEM_COLLECTION, item->GetLot(), LWOOBJID_EMPTY, "", -std::min(item->GetCount(), iStackCount)); } } } From 3d6d5e58a789ccf8df667a2683cad297d27a49e5 Mon Sep 17 00:00:00 2001 From: NinjaOfLU Date: Thu, 14 Apr 2022 10:09:40 +0100 Subject: [PATCH 4/4] Update GameMessages.cpp I promise I'm not farming changes. I woke up at like 4AM and realised that I'd screwed up in an obvious way. Note to self: You are ALLOWED to change variables. Wasn't caught in testing because, well, it turns out it's actually impossible to test the edge case this covers, due to the script for the brick console. --- dGame/dGameMessages/GameMessages.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dGame/dGameMessages/GameMessages.cpp b/dGame/dGameMessages/GameMessages.cpp index e1fb21ac..9e68941c 100644 --- a/dGame/dGameMessages/GameMessages.cpp +++ b/dGame/dGameMessages/GameMessages.cpp @@ -5433,6 +5433,8 @@ void GameMessages::HandleRemoveItemFromInventory(RakNet::BitStream* inStream, En return; } + iStackCount = std::min(item->GetCount(), iStackCount); + if (bConfirmed) { for (auto i = 0; i < iStackCount; ++i) { if (eInvType == eInventoryType::MODELS) @@ -5441,14 +5443,14 @@ void GameMessages::HandleRemoveItemFromInventory(RakNet::BitStream* inStream, En } } - item->SetCount(item->GetCount() - std::min(item->GetCount(), iStackCount), true); + item->SetCount(item->GetCount() - iStackCount, true); EntityManager::Instance()->SerializeEntity(entity); auto* missionComponent = entity->GetComponent(); if (missionComponent != nullptr) { - missionComponent->Progress(MissionTaskType::MISSION_TASK_TYPE_ITEM_COLLECTION, item->GetLot(), LWOOBJID_EMPTY, "", -std::min(item->GetCount(), iStackCount)); + missionComponent->Progress(MissionTaskType::MISSION_TASK_TYPE_ITEM_COLLECTION, item->GetLot(), LWOOBJID_EMPTY, "", -iStackCount); } } }