mirror of
https://github.com/Kelsidavis/WoWee.git
synced 2026-04-26 21:13:51 +00:00
fix(movement): reject server teleports to corrupted near-origin positions
Some checks failed
Build / Build (arm64) (push) Has been cancelled
Build / Build (x86-64) (push) Has been cancelled
Build / Build (macOS arm64) (push) Has been cancelled
Build / Build (windows-arm64) (push) Has been cancelled
Build / Build (windows-x86-64) (push) Has been cancelled
Security / CodeQL (C/C++) (push) Has been cancelled
Security / Semgrep (push) Has been cancelled
Security / Sanitizer Build (ASan/UBSan) (push) Has been cancelled
Some checks failed
Build / Build (arm64) (push) Has been cancelled
Build / Build (x86-64) (push) Has been cancelled
Build / Build (macOS arm64) (push) Has been cancelled
Build / Build (windows-arm64) (push) Has been cancelled
Build / Build (windows-x86-64) (push) Has been cancelled
Security / CodeQL (C/C++) (push) Has been cancelled
Security / Semgrep (push) Has been cancelled
Security / Sanitizer Build (ASan/UBSan) (push) Has been cancelled
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
This commit is contained in:
parent
f9f02569d6
commit
d138269a35
5 changed files with 97 additions and 20 deletions
|
|
@ -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<uint32_t> 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<ActionBarSlot, ACTION_BAR_SLOTS> actionBar{};
|
||||
std::unordered_map<uint32_t, std::string> macros_; // client-side macro text (persisted in char config)
|
||||
|
|
|
|||
|
|
@ -611,6 +611,14 @@ void EntityController::applyPlayerTransportState(const UpdateBlock& block,
|
|||
const std::shared_ptr<Entity>& 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.
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue