Guida C++ Quest States Core Crash Item Dupe Bug Fix

martysama0134

Utente Platinum
9 Gennaio 2009
3,446
80
1,703
1,107
Per comodità, lascerò il contenuto della mia release in inglese.


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:
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:
Diff:
    ALUA(item_is_available)
    {
        auto item = CQuestManager::instance().GetCurrentItem();
        lua_pushboolean(L, item != nullptr);
        return 1;
    }

...

            { "is_available",        item_is_available    },    // [return lua boolean]


Un modo per proteggersi dalle varie manipolazioni via quest può essere questo:
m3VU6Ay.png


Good Luck.