[fix 0.12] item dupe exploit

[leak]

What bug does the patch fix? What features does the patch add?

Prevents duping items similar to 7470

For which repository revision was the patch created?

0.12 branch, presumably for master too

Is there a thread in the bug report section or at lighthouse? If yes, please add a link to the thread.

No.

Who has been writing this patch? Please include either forum user names or email addresses.

me

From 0ff8ef674b1195eec6dbd853ad779848a979c34f Mon Sep 17 00:00:00 2001
From: leak <[email protected]>
Date: Wed, 2 Dec 2009 01:54:32 +0100
Subject: [PATCH] release loot in case of bags being moved

---
src/game/Player.cpp |    6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/src/game/Player.cpp b/src/game/Player.cpp
index 3d041cb..e289cfc 100644
--- a/src/game/Player.cpp
+++ b/src/game/Player.cpp
@@ -9267,7 +9267,7 @@ uint8 Player::CanBankItem( uint8 bag, uint8 slot, ItemPosCountVec &dest, Item *p
        return swap ? EQUIP_ERR_ITEMS_CANT_BE_SWAPPED : EQUIP_ERR_ITEM_NOT_FOUND;

    if (pItem->m_lootGenerated)
-        return EQUIP_ERR_ITEM_LOCKED;
+        return EQUIP_ERR_CANT_DO_RIGHT_NOW;

    uint32 count = pItem->GetCount();

@@ -10437,6 +10437,10 @@ void Player::SwapItem( uint16 src, uint16 dst )
        return;
    }

+    // release loot in case a bag is getting moved, prevents exploiting attempts
+    if(pSrcItem->IsBag() && IsBagPos (dst))
+        GetSession()->DoLootRelease(GetLootGUID());
+
    // check unequip potability for equipped items and bank bags
    if(IsEquipmentPos ( src ) || IsBagPos ( src ))
    {
-- 
1.6.5.1.1367.gcd48

Comment:

This whole field might need more research. I'm pretty sure there are other methods to exploit this issue.

I can provide a full exploit guide for this specific issue if any dev is interested. For obvious reason i won't post it in public.

[crashuncle]

Should work when I see the related code here, its really a critical bug in Mangos.

[balrok]

i think this is exactly what i did.. or?

http://github.com/mangos/mangos/commit/5006c3892658cd1ba8afbb4d168434a542d08e73

sorry i added another patch right into the revert to avoid the spamming

if bug was still present you can pm me the guide.. also i have another way for bugging (not duping) with items if you're interested in this field..

[leak]

Modified my patch after checking in with balrok.

I think EQUIP_ERR_CANT_DO_RIGHT_NOW might be a more suitable error msg, since EQUIP_ERR_ITEM_LOCKED usually refers to locked chests, etc. and also it is used in Player::SwapItem() on a similar case.

The second change prevents loot windows staying open if bags are being moved were the loot window is coming from. This might be not 100% retailish, but currently opened boxes are getting unlocked (ungreyed) on client side once you swap the bags they are located in. This behavior opens possibilites for further loot hacks.

[Ranger]

http://www.deathsoft.com/forum/index.php?showtopic=11555

guide?

[crashuncle]

http://www.deathsoft.com/forum/index.php?showtopic=11555

guide?

Yes and I really don't like this other thread "How to crash a Mangos server" I really hope that this forum disappear sometime.

[crashuncle]

New method (With WPE) which is working fine appeared in the same thread!

[leak]

Updated the patch again.

It should also cover that other exploit method.

Note: This patch is just a workaround for now since it is not yet proven to be the way to go.

Ah yea, before i forget:

If someone has a retail account, you could confirm some behavior here and help to create a proper patch for these issues:

Put a box that contains other items like http://www.wowhead.com/?item=35232 in one of your inventory bags.

Open that box, so you get the loot window.

Now try to move that inventory bag which contains the box either

- into an empty bank slot

- or into an empty inventory bag slot

- or switch it with another empty inventory bag

And report results

- Error msgs

- What happens to the loot window

- What happens to the box the loot window comes from

[balrok]

and how does the new method with wpe work now?

i think something like this will be good:

diff --git a/src/game/Player.cpp b/src/game/Player.cpp
index 5e88703..1ba65c3 100644
--- a/src/game/Player.cpp
+++ b/src/game/Player.cpp
@@ -10073,7 +10073,10 @@ uint8 Player::CanBankItem( uint8 bag, uint8 slot, ItemPosCountVec &dest, Item *p
        return swap ? EQUIP_ERR_ITEMS_CANT_BE_SWAPPED : EQUIP_ERR_ITEM_NOT_FOUND;

    if (pItem->m_lootGenerated)
-        return EQUIP_ERR_ITEM_LOCKED;
+    {
+        GetSession()->DoLootRelease(GetLootGUID());
+        return EQUIP_ERR_CANT_DO_RIGHT_NOW;
+    }

    uint32 count = pItem->GetCount();

@@ -11273,6 +11276,12 @@ void Player::SwapItem( uint16 src, uint16 dst )
    if( !pSrcItem )
        return;

+    if (pSrcItem->IsBag() && GetLootGUID())
+    {
+        SendEquipError( EQUIP_ERR_CANT_DO_RIGHT_NOW, pSrcItem, pDstItem );
+        return;
+    }
+
    sLog.outDebug( "STORAGE: SwapItem bag = %u, slot = %u, item = %u", dstbag, dstslot, pSrcItem->GetEntry());

    if(!isAlive() )

it won't fix the describe bug (is already fixed) but i think this is the way it works on retail..

but it will be good if someone can try to reproduce the guide leak posted above me to confirm this..

[balrok]

again an updated version.. player gets the itemloot if the bag closes where his loot is inside, when moving the bags around

so i came to this:

diff --git a/src/game/Player.cpp b/src/game/Player.cpp
index 5e88703..debcd56 100644
--- a/src/game/Player.cpp
+++ b/src/game/Player.cpp
@@ -10073,7 +10073,10 @@ uint8 Player::CanBankItem( uint8 bag, uint8 slot, ItemPosCountVec &dest, Item *p
        return swap ? EQUIP_ERR_ITEMS_CANT_BE_SWAPPED : EQUIP_ERR_ITEM_NOT_FOUND;

    if (pItem->m_lootGenerated)
-        return EQUIP_ERR_ITEM_LOCKED;
+    {
+        GetSession()->DoLootRelease(GetLootGUID());
+        return EQUIP_ERR_OK;
+    }

    uint32 count = pItem->GetCount();

@@ -11493,7 +11496,7 @@ void Player::SwapItem( uint16 src, uint16 dst )
        // bag swap (with items exchange) case
        if(emptyBag && fullBag)
        {
-            ItemPrototype const* emotyProto = emptyBag->GetProto();
+            ItemPrototype const* emptyProto = emptyBag->GetProto();

            uint32 count = 0;

@@ -11504,7 +11507,7 @@ void Player::SwapItem( uint16 src, uint16 dst )
                    continue;

                ItemPrototype const* bagItemProto = bagItem->GetProto();
-                if (!bagItemProto || !ItemCanGoIntoBag(bagItemProto, emotyProto))
+                if (!bagItemProto || !ItemCanGoIntoBag(bagItemProto, emptyProto))
                {
                    // one from items not go to empty target bag
                    SendEquipError( EQUIP_ERR_NONEMPTY_BAG_OVER_OTHER_BAG, pSrcItem, pDstItem );
@@ -11559,6 +11562,38 @@ void Player::SwapItem( uint16 src, uint16 dst )
    else if (IsEquipmentPos(src))
        EquipItem(eDest2, pDstItem, true);

+
+    // if player is moving bags and is looting an item inside this bag
+    // release the loot
+    if (GetLootGUID())
+    {
+        bool released = false;
+        if (IsBagPos(src))
+        {
+            Bag* bag = (Bag*)pSrcItem;
+            for(int i=0; i < bag->GetBagSize(); ++i)
+                if (Item *bagItem = bag->GetItemByPos(i))
+                    if (bagItem->m_lootGenerated)
+                    {
+                        m_session->DoLootRelease(GetLootGUID());
+                        released = true;                    // so we don't need to look at dstBag
+                        break;
+                    }
+        }
+        if (!released && IsBagPos(dst) && pDstItem)
+        {
+            Bag* bag = (Bag*)pDstItem;
+            for(int i=0; i < bag->GetBagSize(); ++i)
+                if (Item *bagItem = bag->GetItemByPos(i))
+                    if (bagItem->m_lootGenerated)
+                    {
+                        m_session->DoLootRelease(GetLootGUID());
+                        released = true;                    // not realy needed here
+                        break;
+                    }
+        }
+    }
+
    AutoUnequipOffhandIfNeed();
}

but (and this problem is very often inside mangos) player will lose his loot with this patch.. but player actually should just get all loot.. somebody knows how to force the looting?

[crashuncle]

They released again a new method where you can even crash any Mangos server with, I guess the crash reports get flooded soon...