[BUG]Instance exploit/crash - fix

[DrKLO]

bug: if yo go to the dungeon, for example in normal 10 difficulty, and then invite in the raid someone who outside. If this someone change dungeon difficulty and people inside dungeon make logout - server will crash or instance will reset

So do not let this someone to change dungeon difficulty if somebody in this raid\\group in dungeon.

http://paste2.org/p/682412

http://paste2.org/p/683844

[DrKLO]

first post updated, fortgot to add the same check in HandleSetRaidDifficultyOpcode

[zergtmn]

Thank you, seems to work fine.

[DrKLO]

moderators, plz move it to the patch section

[Lightguard]

Well if you take a careful look into Group::ResetInstance you'll surely notice this part:

       bool isEmpty = true;
       // if the map is loaded, reset it
       Map *map = sMapMgr.FindMap(p->GetMapId(), p->GetInstanceId());
       if(map && map->IsDungeon() && !(method == INSTANCE_RESET_GROUP_DISBAND && !p->CanReset()))
           isEmpty = ((InstanceMap*)map)->Reset(method);

Which is supposed to check wether we can reset or not. The bool there wasn't accidently named 'isEmpty' and equals to the return of InstanceMap::Reset.

   if(HavePlayers())
   {
       if(method == INSTANCE_RESET_ALL)
       {
           // notify the players to leave the instance so it can be reset
           for(MapRefManager::iterator itr = m_mapRefManager.begin(); itr != m_mapRefManager.end(); ++itr)
               itr->getSource()->SendResetFailedNotify(GetId());
       }

elseInstanceMap::Reset you'll see a part that checks that if we have players there or not, but then comes a tiny problem, it checks what reset method did we send and if it isn't INSTANCE_RESET_ALL it will invalidate the instance. So as a possible solution i would recommend this:

diff --git a/src/game/Map.cpp b/src/game/Map.cpp
index 7c00f9d..77a459d 100644
--- a/src/game/Map.cpp
+++ b/src/game/Map.cpp
@@ -2582,7 +2582,7 @@ bool InstanceMap::Reset(uint8 method)

    if(HavePlayers())
    {
-        if(method == INSTANCE_RESET_ALL)
+        if(method == INSTANCE_RESET_ALL || method == INSTANCE_RESET_CHANGE_DIFFICULTY)
        {
            // notify the players to leave the instance so it can be reset
            for(MapRefManager::iterator itr = m_mapRefManager.begin(); itr != m_mapRefManager.end(); ++itr)

[Mordaunt]

It seems so easy...Nice find Light

[DrKLO]

to Lightguard, the problem is not only in the instance reset. If someone change difficulty who outside dungeon, and someone who inside make relog in the game - server will crash, because he will enter in the dungeon that not bind to him, and we get assert(false) in the code

sLog.outError("InstanceMap::Add: player %s(%d) is being put in instance %d,%d,%d,%d,%d,%d but he is in group %d and is bound to instance %d,%d,%d,%d,%d,%d!", player->GetName(), player->GetGUIDLow(), mapSave->GetMapId(), mapSave->GetInstanceId(), mapSave->GetDifficulty(), mapSave->GetPlayerCount(), mapSave->GetGroupCount(), mapSave->CanReset(), GUID_LOPART(pGroup->GetLeaderGUID()), playerBind->save->GetMapId(), playerBind->save->GetInstanceId(), playerBind->save->GetDifficulty(), playerBind->save->GetPlayerCount(), playerBind->save->GetGroupCount(), playerBind->save->CanReset());
                       if(groupBind) sLog.outError("InstanceMap::Add: the group is bound to instance %d,%d,%d,%d,%d,%d", groupBind->save->GetMapId(), groupBind->save->GetInstanceId(), groupBind->save->GetDifficulty(), groupBind->save->GetPlayerCount(), groupBind->save->GetGroupCount(), groupBind->save->CanReset());
                       assert(false);

we just prvevent it

i think it's bl1zz like, bacause i got message that i can't change dungeon difficulty, bacause i or some of my party members in the dungeon already

[KAPATEJIb]

to Lightguard, the problem is not only in the instance reset. If someone change difficulty who outside dungeon, and someone who inside make relog in the game - server will crash, because he will enter in the dungeon that not bind to him, and we get assert(false) in the code

i think relogged players should be teleported to instance entrace... this doesn't happens

[alein]

I think this problem still exist.

MaNGOS/0.16.0 (* * Revision 9636 - *) for Linux_x64 (little-endian)

Using script library: ScriptDev2 (for MaNGOS 9603+) Revision [1656] 2010-03-27 11:30:55 (Unix)

Using World DB: PSDB WotLK (r300) for 3.3.2 (Core r9622 / SD2 r1653)

Using creature EventAI: PSDB EventAI & ACID 3.0.3

While someone(player1) is in instance and logout, raid leader outside the dungeon change difficulty and (player1) login again server crash at the moment.

[Morphau]

Do you have crash log ? ...

[alein]

Yes I made it today and exploit still work.

MaNGOS/0.16.0 (* * Revision 9856 - *) for Linux_x64 (little-endian)

Using script library: ScriptDev2 (for MaNGOS 9832+) Revision [1683] 2010-05-04 12:17:06 (Unix)

Using World DB: PSDB WotLK (r307) for 3.3.3a (Core r9826 / SD2 r1683)

Using creature EventAI: PSDB EventAI & ACID 3.0.3

http://files.alein.org/wow/mangos/crashlogs/instance_exploit_crash.txt

http://files.alein.org/wow/mangos/crashlogs/last_console_messages.txt

[Patman128]

For some reason there's a bunch of ASSERTS() there. I'm not sure why it doesn't just kick them from the instance instead.

[darkstalker]

assert's are supposed to do nothing when you compile in release mode

[Patman128]

assert's are supposed to do nothing when you compile in release mode

I know, but they should probably be kicked anyway.

Anyway, this is the problem. It's crashing because it's in debug mode. You need to disable MANGOS_DEBUG if you want it to not crash due to this, or else comment out the ASSERT()s.

[KAPATEJIb]

Yes I made it today and exploit still work.

http://files.alein.org/wow/mangos/crashlogs/instance_exploit_crash.txt

http://files.alein.org/wow/mangos/crashlogs/last_console_messages.txt

I'm using this hack

diff --git a/src/game/Map.cpp b/src/game/Map.cpp
index d8ad165..145b0f4 100644
--- a/src/game/Map.cpp
+++ b/src/game/Map.cpp
@@ -2374,7 +2374,7 @@ bool InstanceMap::Add(Player *player)
                if(playerBind->save != mapSave)
                {
                    sLog.outError("InstanceMap::Add: player %s(%d) is permanently bound to instance %d,%d,%d,%d,%d,%d but he is being put in instance %d,%d,%d,%d,%d,%d", player->GetName(), player->GetGUIDLow(), playerBind->save->GetMapId(), playerBind->save->GetInstanceId(), playerBind->save->GetDifficulty(), playerBind->save->GetPlayerCount(), playerBind->save->GetGroupCount(), playerBind->save->CanReset(), mapSave->GetMapId(), mapSave->GetInstanceId(), mapSave->GetDifficulty(), mapSave->GetPlayerCount(), mapSave->GetGroupCount(), mapSave->CanReset());
-                    ASSERT(false);
+                    player->RepopAtGraveyard();
                }
            }
            else
@@ -2407,7 +2407,7 @@ bool InstanceMap::Add(Player *player)
                                sLog.outError("GroupBind save players: %d, group count: %d", groupBind->save->GetPlayerCount(), groupBind->save->GetGroupCount());
                            else
                                sLog.outError("GroupBind save NULL");
-                            ASSERT(false);
+                            player->RepopAtGraveyard();
                        }
                        // if the group/leader is permanently bound to the instance
                        // players also become permanently bound when they enter

seems to stable now, also buggers can't do anything

[alein]

I know how buggers do this. I've made this crash deliberate.

Patch tested and works. Server don't crash.

[Unkle Nuke]

KAPATEJIb's patch is a patch for DrKLO's patch or is it intended to replace DrKLO's patch?

[KAPATEJIb]

KAPATEJIb's patch is a patch for DrKLO's patch or is it intended to replace DrKLO's patch?

these fixes for different bugs. You should use both if wanna get high stability. Anyway my last fix is just a hack.

How it works on offy:

Offline player in instance shouldn't be moved to new (switched by group leader) difficulty of the instance if he doesn't meet instance difficulty requirements - he just stay in old copy of the instance at login (DO NOT delete instance even it has offline group members!!! http://github.com/apostol/mangos333_janu/commit/ca7585b95a95d1628d4d5ce5809f3baedf1be6ed ), other members that meets instance requirements of new instance difficulty goes into new copy (with new difficulty) at login. Also if more that one player doesn't meet instance difficulty requirements - they (who doesn't meet) stay at old instance copy and can see each other.

So players in one group can be in different instance copies.

i was already tried to write the patch for implement last, but nobody is interested in this and won't help...

http://ru-mangos.ru/showpost.php?p=5731&postcount=7

[Auntie Mangos]

Hope to see this in the master.

[zergtmn]

Hope to see this in the master.

It's hack that will never get into master.

[Unkle Nuke]

...nobody is interested in this and won't help...

It's hack that will never get into master.

/me throws hands up in frustration.

So what needs to be done so it is acceptable for the core? Perhaps some sort of checklist of features that need to be reworked would give any volunteers a place to start.

[zergtmn]

/me throws hands up in frustration.

So what needs to be done so it is acceptable for the core? Perhaps some sort of checklist of features that need to be reworked would give any volunteers a place to start.

From what I know:

1. First bug: enter 25-player WOTLK raid instance, restart server, enter game and you will get into 10-player version of that instance. Difficulty is not saved.

2. There are two exploits to bypass permanent instance binding. One can be fixed with patch from first post, but it's non-blizzlike hack.

3. Old exploit to bypass areatrigger requirements is still working.

4. Another bug (or not implemented feature): die in instance, logout, you will be teleported to graveyard. On offy you remain in instance.

Instance system needs some love.

[Unkle Nuke]

Thank you, zergtmn.

Okay, you apes! You got some places to start. Now quit lurking and start submitting!

[The_Game_Master]

Is this still needed? Fixed here by any chance? http://github.com/mangos/mangos/commit/b2b9a8741d044fc2fde14e85cd569d303ebb76e2