Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core/Maps entry assert #10259

Closed
Annamaria-CC opened this issue Jan 20, 2022 · 16 comments
Closed

Core/Maps entry assert #10259

Annamaria-CC opened this issue Jan 20, 2022 · 16 comments
Labels
HasBacktrace Priority-Critical Server-breaking bugs. Crashes or Exploits

Comments

@Annamaria-CC
Copy link
Member

Rev: 124ea8a

Modules;
mod-anticheat
mod-autobalance
mod-cfbg
mod-chromie-xp
mod-desertion-warnings
mod-duel-reset
mod-ip-tracker
mod-eluna-lua-engine
mod-low-level-arena
mod-progression-system
mod-pvp-titles
mod-pvpstats-announcer
mod-queue-list-cache
mod-rdf-expansion
mod-server-auto-shutdown
mod-spellsystem-extended

Operating system
Unix

See pastebin
https://pastebin.com/1WzLnT11

@Kitzunu
Copy link
Member

Kitzunu commented Jan 20, 2022

map = FindBaseMap(id);
if (map == nullptr) // pussywizard: check again after acquiring mutex
{
MapEntry const* entry = sMapStore.LookupEntry(id);
ASSERT(entry);

Are you using any custom maps or trying to access maps that don't exist in DBC?

@Kitzunu Kitzunu changed the title CC PTR Crash 20/01/2022 Mapmgr.cpp Core/Maps entry assert Jan 20, 2022
@Kitzunu Kitzunu added the Priority-Critical Server-breaking bugs. Crashes or Exploits label Jan 20, 2022
@Nyeriah
Copy link
Member

Nyeriah commented Jan 20, 2022

andler=0xcfd500 <go_commandscript::HandleGoZoneXYCommand(ChatHandler*, float, floa

Looks like someone tried to port to an obscure place

@Annamaria-CC
Copy link
Member Author

Annamaria-CC commented Jan 20, 2022

No custom maps. Not sure about the second question,

if so, shouldn't result in a crash I suppose?

@Kitzunu
Copy link
Member

Kitzunu commented Jan 20, 2022

you can see in the code above that it will assert if the map entry can't be found in dbc

@Nyeriah
Copy link
Member

Nyeriah commented Jan 20, 2022

The GoZoneXY command needs a check to see if the map theyre porting to is valid

unfortunately it’s a ptr crash so it’s impossible to tell what they tried to do

@Kitzunu
Copy link
Member

Kitzunu commented Jan 20, 2022

The GoZoneXY command needs a check to see if the map theyre porting to is valid

Well that shouldn't be necessary as you don't have an argument to provide map ID. Only for zone id, and if the zone id doesn't exist it will assert within that command

and that is what is throwing me off at the moment because we have an ASSERT for zoneid, and under we call the create map

// update to parent zone if exist (client map show only zones without parents)
AreaTableEntry const* zoneEntry = areaEntry->zone ? sAreaTableStore.LookupEntry(areaEntry->zone) : areaEntry;
ASSERT(zoneEntry);
Map const* map = sMapMgr->CreateBaseMap(zoneEntry->mapid);

So the zone provided is "valid" but has no map?

@Kitzunu
Copy link
Member

Kitzunu commented Jan 20, 2022

and they didnt even provide a zoneid? only coordinates if I read this correctly

0x0000000000cfd6e0 in go_commandscript::HandleGoZoneXYCommand (handler=0x7ffec063cc38, x=40, y=40, areaIdArg=std::optional<Acore::ChatCommands::Variant<Acore::ChatCommands::Hyperlink<Acore::Hyperlinks::LinkTags::area>, unsigned int>> 

So this should not be possible

@Kitzunu
Copy link
Member

Kitzunu commented Jan 20, 2022

0  0x0000000001fe08d3 in Acore::Assert (file=0x23cd708 "/root/env/azerothcore-wotlk/src/server/game/Maps/MapMgr.cpp", line=83, function=0x23cd744 "CreateBaseMap", debugInfo=Python Exception <class 'gdb.error'> There is no member named _M_dataplus.: 
, message=0x23d7a3a "entry") at /root/env/azerothcore-wotlk/src/common/Debugging/Errors.cpp:64
        formattedMessage = <incomplete type>
#1  0x000000000185ac8a in MapMgr::CreateBaseMap (this=0x2e732d8 <MapMgr::instance()::instance>, id=150) at /root/env/azerothcore-wotlk/src/server/game/Maps/MapMgr.cpp:83
        entry = 0x0
        guard = {_M_device = @0x2e732d8}
        map = 0x0
#2  0x0000000000cfd6e0 in go_commandscript::HandleGoZoneXYCommand (handler=0x7ffec063cc38, x=40, y=40, areaIdArg=std::optional<Acore::ChatCommands::Variant<Acore::ChatCommands::Hyperlink<Acore::Hyperlinks::LinkTags::area>, unsigned int>> = {...}) at /root/env/azerothcore-wotlk/src/server/scripts/Commands/cs_go.cpp:282
        player = 0x7fa6d7dd88c0
        areaId = 676
        areaEntry = 0x7fa725371694
        zoneEntry = 0x7fa725371694
        map = 0x54ae98 <std::__detail::__variant::_Move_ctor_base<false, std::monostate, std::basic_string_view<char, std::char_traits<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >::~_Move_ctor_base()+24>
        z = 0

@Nyeriah
Copy link
Member

Nyeriah commented Jan 20, 2022

That assert inside that gm command makes no sense and needs to go away though. The server shouldn't crash if someone decides to try some random value when teleporting

@Kitzunu
Copy link
Member

Kitzunu commented Jan 20, 2022

it is for the zone id if you provide it.

@Kitzunu
Copy link
Member

Kitzunu commented Jan 20, 2022

But, that assert should have hit imo and not the one in createmap. because how can a zone exist but a map doesn't?

@Nyeriah
Copy link
Member

Nyeriah commented Jan 20, 2022

ASSERTs are to be used in very specific situations, code that will irreparably break the core, points of no return that only shutting the entire thing down would make sense. A GM command can just return if something is wrong and print an error instead of shutting the entire server down and no harm will be done.

If I read that thing carefully, the zone falls back if it's null and uses the current area's parent zone, so it passes the zone check, even if the map creation fails a few lines below.

@Kitzunu
Copy link
Member

Kitzunu commented Jan 20, 2022

yea but I am still stuck on how we can have a valid zone but not a valid map

@acidmanifesto
Copy link
Contributor

not sure if relevant but azerothcore seems to be the only core i see with this if statement

if (map == nullptr) // pussywizard: check again after acquiring mutex

where other cores do not.
Trinitycore 335
https://github.com/TrinityCore/TrinityCore/blob/fd7f48545ce76740065860c0963194c2a9e3e0e2/src/server/game/Maps/MapManager.cpp#L74-L76

Cata Preservation, Trinitycore Master and skyfire has it as if there is !map
https://github.com/The-Cataclysm-Preservation-Project/TrinityCore/blob/803bd2c79b058be1128f38e15090985fb59e2f78/src/server/game/Maps/MapManager.cpp#L79-L83
https://github.com/TrinityCore/TrinityCore/blob/aa742c8752d35f3d1a216a4a8ac6a4987cc4da7e/src/server/game/Maps/MapManager.cpp#L79-L82
https://github.com/ProjectSkyfire/SkyFire_548/blob/781ce78b7c43b1e212c1663dffe6f9f1543362a5/src/server/game/Maps/MapManager.cpp#L103-L106

I guess what i am saying, are we certain we need to check with it being a nullptr and not a !map?

@UltraNix
Copy link
Contributor

UltraNix commented Feb 6, 2022

Removing an assert is not a fix.
The only problem is the arg provided here. go zonexy 40 40 676. Area/Zone 676 ("Outland") is invalid.
In my opinin nothing is worng here in the code.

@UltraNix
Copy link
Contributor

Close it as it's not a core problem.
And an assert works great in situations like these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HasBacktrace Priority-Critical Server-breaking bugs. Crashes or Exploits
Projects
None yet
Development

No branches or pull requests

5 participants