This vulnerability should affect every server. You can duplicate item rewards, and also crash the server through dangling pointers.
The danger of this bug escalates to how many custom systems, and how many crafting quests (for example, the vitality ore quest, not the cube system) you have in your server.
How to trigger it:
Any quest that uses select & wait, and the item lua module after that is vulnerable.
After the server uses select() or wait(), the player's quest state is suspended. After the player replies using the CG packet, the quest state is recovered.
So what's wrong with it? It doesn't verify if the stored quest item ptr expired.
You basically need to destroy the selected item ptr in order to dupe the rewards of the quest. After some tries, you may get a core crash in the game. (dangling pointers often cause crashes only after that memory sector has been rewritten)
In my files, I've checked (since several years ago) if the quest state was suspended for the the default windows such as exchange, cube, shop.
This bug can work very easily on offline shops or other new systems that don't check that.
After the select() or wait() is called, you send the selected item to the (e.g.) offlineshop system window. It will delete the item ptr in the game. Now, you can press "Ok" on the quest, and the quest will proceed as if the item still existed.
The item still exists in the offlineshop, but not the item ptr anymore. The item won't be deleted by the quest even after item.remove() is called.
This is the fix:
- diff --git a/s3ll_server/Srcs/Server/game/src/char.cpp b/s3ll_server/Srcs/Server/game/src/char.cpp
- index 0ea307fa..65b1dd65 100644
- --- a/s3ll_server/Srcs/Server/game/src/char.cpp
- +++ b/s3ll_server/Srcs/Server/game/src/char.cpp
- @@ -303,7 +303,10 @@ void CHARACTER::Initialize()
- m_dwQuestNPCVID = 0;
- m_dwQuestByVnum = 0;
- - m_pQuestItem = NULL;
- + m_dwQuestItemVID = 0;
- m_dwUnderGuildWarInfoMessageTime = get_dword_time()-60000;
- @@ -6123,33 +6126,37 @@ LPCHARACTER CHARACTER::GetQuestNPC() const
- void CHARACTER::SetQuestItemPtr(LPITEM item)
- {
- - m_pQuestItem = item;
- + m_dwQuestItemVID = (item) ? item->GetVID() : 0;
- }
- void CHARACTER::ClearQuestItemPtr()
- {
- - m_pQuestItem = NULL;
- + m_dwQuestItemVID = 0;
- }
- LPITEM CHARACTER::GetQuestItemPtr() const
- {
- - return m_pQuestItem;
- + if (!m_dwQuestItemVID)
- + return nullptr;
- + return ITEM_MANAGER::Instance().FindByVID(m_dwQuestItemVID);
- }
- diff --git a/s3ll_server/Srcs/Server/game/src/char.h b/s3ll_server/Srcs/Server/game/src/char.h
- index cc4da2bb..74b3470e 100644
- --- a/s3ll_server/Srcs/Server/game/src/char.h
- +++ b/s3ll_server/Srcs/Server/game/src/char.h
- @@ -1674,9 +1674,9 @@ class CHARACTER : public CEntity, public CFSM, public CHorseRider
- private:
- DWORD m_dwQuestNPCVID;
- DWORD m_dwQuestByVnum;
- - LPITEM m_pQuestItem;
- + DWORD m_dwQuestItemVID{}; // @fixme304 (LPITEM -> DWORD)
- // Events
Important: after this fix, the item ptr may be nullptr after they press enter, so you need to check if the item ptr is still valid by using this function:
A way to protect your quests via other manipulations can be this:
Bitte melden Sie sich an, um dieses Bild zu sehen.
