From d138269a35e9743e78ea651f492fd01cb912ab10 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Fri, 24 Apr 2026 17:48:49 -0700 Subject: [PATCH] fix(movement): reject server teleports to corrupted near-origin positions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server can persist a corrupted near-origin position on map 0 (from a faulty area-trigger destination) across sessions. On re-login it sends the bad position via LOGIN_VERIFY_WORLD; if the player walks into the offending trigger again the server re-teleports there, and our heartbeats reinforce the bad save — creating a permanent teleport loop. Defenses added: - handleTeleportAck rejects MSG_MOVE_TELEPORT to near-origin on map 0 (no position update, no ACK, no world reload) - applyPlayerTransportState rejects player UPDATE_OBJECT MOVEMENT blocks pushing the same bad position - sendMovement blocks heartbeats originating from near-origin so the server cannot persist the bad save - 10-second area-trigger cooldown after teleport / world entry / login (replaces the one-shot suppress flag that re-fired on jitter) - Immediate STOP+HEARTBEAT after teleport ACK / WORLDPORT ACK / login to sync the real position with the server promptly - CMSG_AREATRIGGER firing now logged at WARNING level for diagnosis --- include/game/game_handler.hpp | 2 + src/game/entity_controller.cpp | 30 +++++++++--- src/game/game_handler.cpp | 1 + src/game/game_handler_callbacks.cpp | 9 ++++ src/game/movement_handler.cpp | 75 +++++++++++++++++++++++------ 5 files changed, 97 insertions(+), 20 deletions(-) diff --git a/include/game/game_handler.hpp b/include/game/game_handler.hpp index b59eec47..2f08f6a7 100644 --- a/include/game/game_handler.hpp +++ b/include/game/game_handler.hpp @@ -2332,6 +2332,7 @@ public: auto& areaTriggerMsgsRef() { return areaTriggerMsgs_; } auto& areaTriggersRef() { return areaTriggers_; } auto& areaTriggerSuppressFirstRef() { return areaTriggerSuppressFirst_; } + auto& areaTriggerCooldownRef() { return areaTriggerCooldown_; } // ── Death & Resurrection ───────────────────────────────────────── auto& playerDeadRef() { return playerDead_; } @@ -2944,6 +2945,7 @@ private: std::unordered_set activeAreaTriggers_; // triggers player is currently inside float areaTriggerCheckTimer_ = 0.0f; bool areaTriggerSuppressFirst_ = false; // suppress first check after map transfer + float areaTriggerCooldown_ = 0.0f; // seconds remaining — suppress ALL triggers std::array actionBar{}; std::unordered_map macros_; // client-side macro text (persisted in char config) diff --git a/src/game/entity_controller.cpp b/src/game/entity_controller.cpp index ff4f0d80..66a22db7 100644 --- a/src/game/entity_controller.cpp +++ b/src/game/entity_controller.cpp @@ -611,6 +611,14 @@ void EntityController::applyPlayerTransportState(const UpdateBlock& block, const std::shared_ptr& entity, const glm::vec3& canonicalPos, float oCanonical, bool updateMovementInfoPos) { + // Reject server-pushed position corrections to near-origin on map 0. + // The server stores a corrupted position from a faulty area-trigger + // destination; accepting it lets heartbeats reinforce the bad save. + auto positionIsBad = [&](float x, float y) { + return owner_.getCurrentMapId() == 0 && + std::abs(x) < 1000.0f && std::abs(y) < 1000.0f; + }; + if (block.onTransport) { // Convert transport offset from server → canonical coordinates glm::vec3 serverOffset(block.transportX, block.transportY, block.transportZ); @@ -623,18 +631,28 @@ void EntityController::applyPlayerTransportState(const UpdateBlock& block, owner_.movementInfoRef().y = composed.y; owner_.movementInfoRef().z = composed.z; } else if (updateMovementInfoPos) { - owner_.movementInfoRef().x = canonicalPos.x; - owner_.movementInfoRef().y = canonicalPos.y; - owner_.movementInfoRef().z = canonicalPos.z; + if (positionIsBad(canonicalPos.x, canonicalPos.y)) { + LOG_WARNING("REJECTED player UPDATE_OBJECT to near-origin canonical=(", + canonicalPos.x, ", ", canonicalPos.y, ", ", canonicalPos.z, ")"); + } else { + owner_.movementInfoRef().x = canonicalPos.x; + owner_.movementInfoRef().y = canonicalPos.y; + owner_.movementInfoRef().z = canonicalPos.z; + } } LOG_INFO("Player on transport: 0x", std::hex, owner_.playerTransportGuidRef(), std::dec, " offset=(", owner_.playerTransportOffsetRef().x, ", ", owner_.playerTransportOffsetRef().y, ", ", owner_.playerTransportOffsetRef().z, ")"); } else { if (updateMovementInfoPos) { - owner_.movementInfoRef().x = canonicalPos.x; - owner_.movementInfoRef().y = canonicalPos.y; - owner_.movementInfoRef().z = canonicalPos.z; + if (positionIsBad(canonicalPos.x, canonicalPos.y)) { + LOG_WARNING("REJECTED player UPDATE_OBJECT to near-origin canonical=(", + canonicalPos.x, ", ", canonicalPos.y, ", ", canonicalPos.z, ")"); + } else { + owner_.movementInfoRef().x = canonicalPos.x; + owner_.movementInfoRef().y = canonicalPos.y; + owner_.movementInfoRef().z = canonicalPos.z; + } } // Don't clear client-side M2 transport boarding (trams) — // the server doesn't know about client-detected transport attachment. diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index 1e6bd3b6..5a1ab444 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -867,6 +867,7 @@ void GameHandler::update(float deltaTime) { } // Check area triggers (instance portals, tavern rests, etc.) + if (areaTriggerCooldown_ > 0.0f) areaTriggerCooldown_ -= deltaTime; areaTriggerCheckTimer_ += deltaTime; if (areaTriggerCheckTimer_ >= game::AREA_TRIGGER_CHECK_INTERVAL) { areaTriggerCheckTimer_ = 0.0f; diff --git a/src/game/game_handler_callbacks.cpp b/src/game/game_handler_callbacks.cpp index 53236e9b..1f53f574 100644 --- a/src/game/game_handler_callbacks.cpp +++ b/src/game/game_handler_callbacks.cpp @@ -614,6 +614,7 @@ void GameHandler::handleLoginVerifyWorld(network::Packet& packet) { activeAreaTriggers_.clear(); areaTriggerCheckTimer_ = -5.0f; areaTriggerSuppressFirst_ = true; + areaTriggerCooldown_ = 10.0f; // Notify application to load terrain for this map/position (online mode) if (worldEntryCallback_) { @@ -627,6 +628,14 @@ void GameHandler::handleLoginVerifyWorld(network::Packet& packet) { LOG_INFO("Sent CMSG_SET_ACTIVE_MOVER for player 0x", std::hex, playerGuid, std::dec); } + // Immediately sync our position so the server doesn't keep stale coordinates + // from a previous session or character. Without this, the server can think + // the player is elsewhere and issue a corrective teleport. + if (socket) { + sendMovement(game::Opcode::MSG_MOVE_STOP); + sendMovement(game::Opcode::MSG_MOVE_HEARTBEAT); + } + // Kick the first keepalive immediately on world entry. Classic-like realms // can close the session before our default 30s ping cadence fires. timeSinceLastPing = 0.0f; diff --git a/src/game/movement_handler.cpp b/src/game/movement_handler.cpp index 259f6752..578ea512 100644 --- a/src/game/movement_handler.cpp +++ b/src/game/movement_handler.cpp @@ -613,15 +613,18 @@ void MovementHandler::sendMovement(Opcode opcode) { wireOpcode(opcode), std::dec, (includeTransportInWire ? " ONTRANSPORT" : "")); - // Detect near-origin position on Eastern Kingdoms (map 0) — this would place - // the player near Alterac Mountains and is almost certainly a bug. + // Block heartbeats from a near-origin position on Eastern Kingdoms (map 0). + // These positions are almost certainly bugs (area trigger misfire, corrupted + // save). Sending them tells the server the player is there, which persists + // the bad position across sessions and creates a teleport loop. if (owner_.getCurrentMapId() == 0 && - std::abs(movementInfo.x) < 500.0f && std::abs(movementInfo.y) < 500.0f) { - LOG_WARNING("sendMovement: position near map origin! canonical=(", + std::abs(movementInfo.x) < 1000.0f && std::abs(movementInfo.y) < 1000.0f) { + LOG_WARNING("sendMovement: BLOCKED near-origin heartbeat canonical=(", movementInfo.x, ", ", movementInfo.y, ", ", movementInfo.z, ") onTransport=", owner_.isOnTransport(), " transportGuid=0x", std::hex, owner_.playerTransportGuidRef(), std::dec, " flags=0x", std::hex, movementInfo.flags, std::dec); + return; } // Convert canonical → server coordinates for the wire @@ -1660,6 +1663,21 @@ void MovementHandler::handleTeleportAck(network::Packet& packet) { " currentPos=(", movementInfo.x, ", ", movementInfo.y, ", ", movementInfo.z, ")"); glm::vec3 canonical = core::coords::serverToCanonical(glm::vec3(serverX, serverY, serverZ)); + + // Reject teleports to a near-origin position on Eastern Kingdoms (map 0). + // No legitimate gameplay sends the player there; in this codebase it has + // only ever been an area-trigger-destination misfire on the server side. + // Refusing it (no ACK, no position update, no world reload) keeps the + // client at its current position. Heartbeats from the real position will + // eventually convince the server's anti-cheat to update its record. + if (owner_.getCurrentMapId() == 0 && + std::abs(canonical.x) < 1000.0f && std::abs(canonical.y) < 1000.0f) { + LOG_WARNING("REJECTED MSG_MOVE_TELEPORT to near-origin canonical=(", + canonical.x, ", ", canonical.y, ", ", canonical.z, ")" + " — keeping current position"); + return; + } + movementInfo.x = canonical.x; movementInfo.y = canonical.y; movementInfo.z = canonical.z; @@ -1669,6 +1687,14 @@ void MovementHandler::handleTeleportAck(network::Packet& packet) { // Clear cast bar on teleport — SpellHandler owns the casting_ flag if (owner_.getSpellHandler()) owner_.getSpellHandler()->resetCastState(); + // Suppress area triggers for 10s after teleport. A one-shot flag is not + // enough — the player can leave and re-enter a trigger within seconds and + // get teleported again before the world has finished loading. + owner_.activeAreaTriggersRef().clear(); + owner_.areaTriggerCheckTimerRef() = -5.0f; + owner_.areaTriggerSuppressFirstRef() = true; + owner_.areaTriggerCooldownRef() = 10.0f; + if (owner_.getSocket()) { network::Packet ack(wireOpcode(Opcode::MSG_MOVE_TELEPORT_ACK)); const bool legacyGuidAck = @@ -1682,6 +1708,11 @@ void MovementHandler::handleTeleportAck(network::Packet& packet) { ack.writeUInt32(moveTime); owner_.getSocket()->send(ack); LOG_INFO("Sent MSG_MOVE_TELEPORT_ACK response"); + + // Immediately tell the server our position so it doesn't keep using + // stale pre-teleport coordinates for area-trigger / anti-cheat checks. + sendMovement(Opcode::MSG_MOVE_STOP); + sendMovement(Opcode::MSG_MOVE_HEARTBEAT); } if (owner_.worldEntryCallbackRef()) { @@ -1732,10 +1763,18 @@ void MovementHandler::handleNewWorld(network::Packet& packet) { owner_.tabCycleStaleRef() = true; owner_.resetCastState(); + owner_.activeAreaTriggersRef().clear(); + owner_.areaTriggerCheckTimerRef() = -5.0f; + owner_.areaTriggerSuppressFirstRef() = true; + owner_.areaTriggerCooldownRef() = 10.0f; + if (owner_.getSocket()) { network::Packet ack(wireOpcode(Opcode::MSG_MOVE_WORLDPORT_ACK)); owner_.getSocket()->send(ack); LOG_INFO("Sent MSG_MOVE_WORLDPORT_ACK (resurrection)"); + + sendMovement(Opcode::MSG_MOVE_STOP); + sendMovement(Opcode::MSG_MOVE_HEARTBEAT); } return; } @@ -1794,6 +1833,7 @@ void MovementHandler::handleNewWorld(network::Packet& packet) { owner_.activeAreaTriggersRef().clear(); owner_.areaTriggerCheckTimerRef() = -5.0f; owner_.areaTriggerSuppressFirstRef() = true; + owner_.areaTriggerCooldownRef() = 10.0f; owner_.stopAutoAttack(); owner_.resetCastState(); @@ -1801,6 +1841,10 @@ void MovementHandler::handleNewWorld(network::Packet& packet) { network::Packet ack(wireOpcode(Opcode::MSG_MOVE_WORLDPORT_ACK)); owner_.getSocket()->send(ack); LOG_INFO("Sent MSG_MOVE_WORLDPORT_ACK"); + + // Sync position immediately so the server doesn't keep stale coordinates. + sendMovement(Opcode::MSG_MOVE_STOP); + sendMovement(Opcode::MSG_MOVE_HEARTBEAT); } owner_.timeSinceLastPingRef() = 0.0f; @@ -2549,7 +2593,8 @@ void MovementHandler::checkAreaTriggers() { // Sanity: if position is near map origin on Eastern Kingdoms (map 0), // something has corrupted movementInfo — skip area trigger check to // avoid firing Alterac/Hillsbrad triggers and causing a rogue teleport. - if (owner_.getCurrentMapId() == 0 && std::abs(px) < 500.0f && std::abs(py) < 500.0f) { + if (owner_.getCurrentMapId() == 0 && + std::abs(px) < 1000.0f && std::abs(py) < 1000.0f) { LOG_WARNING("checkAreaTriggers: position near map origin (", px, ", ", py, ", ", pz, ") on map 0 — skipping to avoid rogue teleport. onTransport=", owner_.isOnTransport(), " transportGuid=0x", std::hex, @@ -2557,6 +2602,11 @@ void MovementHandler::checkAreaTriggers() { return; } + // Time-based cooldown after teleport/world entry — suppress ALL trigger + // firing (not just the first check) to prevent re-entry from immediately + // sending us to a wrong destination. + const bool cooldownActive = owner_.areaTriggerCooldownRef() > 0.0f; + // On first check after map transfer, just mark which triggers we're inside // without firing them — prevents exit portal from immediately sending us back bool suppressFirst = owner_.areaTriggerSuppressFirstRef(); @@ -2600,19 +2650,16 @@ void MovementHandler::checkAreaTriggers() { if (owner_.activeAreaTriggersRef().count(at.id) == 0) { owner_.activeAreaTriggersRef().insert(at.id); - if (suppressFirst) { - // After map transfer: mark triggers we're inside of, but don't fire them. - // This prevents the exit portal from immediately sending us back. - LOG_WARNING("AreaTrigger suppressed (post-transfer): AT", at.id); + if (suppressFirst || cooldownActive) { + LOG_WARNING("AreaTrigger suppressed (post-transfer): AT", at.id, + " cooldown=", owner_.areaTriggerCooldownRef()); } else { - // Send CMSG_AREATRIGGER — the player is already inside the - // trigger radius so the server's distance check should pass. - // Do NOT spoof position to the trigger center; that tells the - // server we're somewhere we're not and can cause rogue teleports. network::Packet pkt(wireOpcode(Opcode::CMSG_AREATRIGGER)); pkt.writeUInt32(at.id); owner_.getSocket()->send(pkt); - LOG_DEBUG("Fired CMSG_AREATRIGGER: id=", at.id); + LOG_WARNING("Fired CMSG_AREATRIGGER: id=", at.id, + " pos=(", px, ", ", py, ", ", pz, ")", + " trigger=(", at.x, ", ", at.y, ", ", at.z, ")"); } } } else {