From a78a918d300575d1750dbddcba4099b7299801b2 Mon Sep 17 00:00:00 2001 From: Andrew Bates Date: Mon, 5 Oct 2026 19:40:11 -0500 Subject: [PATCH 1/2] fix(abi): Lobby::EntrantData is 0xD8 bytes, not 0xA0 (#38) echovr.exe indexes the entrant array at [lobby+0x360] with a 0xD8 stride in every site measured in ReVault: CNSLobby::SmiteEntrant 0x14061665d IMUL RAX, RDX, 0xd8 fcn_1406082b0 0x1406082c9 IMUL RDX, RDX, 0xd8 fcn 0x140616920 loop 0x14061698f ADD R8, 0xd8 The object is the same lobby IServerLib::Initialize receives: LoadServerSupport (0x14060bb70) stores the server library at this+0x38 and calls vtable+8 (Initialize) with `this` as the lobby argument, and SmiteEntrant reads this+0x130 (hosting) and this+0x8 (broadcaster) at the offsets Lobby already declares. The field offsets 0x00-0x9F still agree with echovr-reconstruction CServerConfig.h; 0xA0 there is the mapped prefix, not the element size. With the old sizeof, items[i] for i >= 1 read the wrong bytes. It was latent only because ServerContext's entrant copy was empty (#38). Adds the unmapped 0x38-byte tail and moves the static_assert to 0xD8. Co-Authored-By: nevr-runtime --- src/abi/echovr.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/abi/echovr.h b/src/abi/echovr.h index dc89de42..8ba9fefd 100644 --- a/src/abi/echovr.h +++ b/src/abi/echovr.h @@ -371,8 +371,11 @@ enum class NetGameState : INT32 { /// Lobby objects can be local, dedicated, etc. As a game server, this is a dedicated lobby object. /// struct Lobby { - /// Per-player data in the Lobby. Validated against echovr-reconstruction CServerConfig.h. - /// sizeof == 0xA0 (160 bytes) + /// Per-player data in the Lobby. Field offsets 0x00-0x9F match echovr-reconstruction + /// CServerConfig.h. sizeof == 0xD8 (216 bytes): echovr.exe indexes the [lobby+0x360] + /// array with a 0xD8 stride — CNSLobby::SmiteEntrant 0x14061665d `IMUL RAX,RDX,0xd8`, + /// fcn_1406082b0 0x1406082c9 `IMUL RDX,RDX,0xd8`, 0x140616920 loop 0x14061698f + /// `ADD R8,0xd8`. The reconstruction's 0xA0 is the mapped prefix, not the element size. struct EntrantData { XPlatformId userId; // +0x00 SymbolId platformId; // +0x10 @@ -388,6 +391,7 @@ struct Lobby { UINT16 genIndex; // +0x8C UINT16 teamIndex; // +0x8E (0=blue, 1=orange, 2=spec) Json json; // +0x90 (root + cache pointers, 0x10 bytes) + BYTE _unkA0[0x38]; // +0xA0 unmapped tail of the 0xD8-byte element }; /// Validated against echovr-reconstruction CServerConfig.h. sizeof == 0x38 @@ -452,7 +456,7 @@ struct Lobby { }; // --- Lobby sub-struct validation (echovr-reconstruction CServerConfig.h, CLobby.h) --- -static_assert(sizeof(Lobby::EntrantData) == 0xA0, "EntrantData size mismatch with reconstruction"); +static_assert(sizeof(Lobby::EntrantData) == 0xD8, "EntrantData stride mismatch with echovr.exe (0x14061665d)"); static_assert(offsetof(Lobby::EntrantData, userId) == 0x00, "EntrantData::userId offset mismatch"); static_assert(offsetof(Lobby::EntrantData, platformId) == 0x10, "EntrantData::platformId offset mismatch"); static_assert(offsetof(Lobby::EntrantData, uniqueName) == 0x18, "EntrantData::uniqueName offset mismatch"); From 981cf085be6f48f4fc529aa1b9f838040d5d5b93 Mon Sep 17 00:00:00 2001 From: Andrew Bates Date: Mon, 5 Oct 2026 19:44:06 -0500 Subject: [PATCH 2/2] fix(server): read lobby entrants live instead of a copy taken at Initialize (#38) ServerContext copied lobby->entrantData once in Initialize and served GetEntrant/GetEntrantCount from that copy. The game calls IServerLib::Initialize from CNSLobby LoadServerSupport (0x14060bb70) while the server boots, before anyone joins, so the copy was empty or stale for the life of the process. Every reader wanted the current lobby: - kLobbySmiteEntrant resolves an entrant to the slot index it hands the game; a stale copy can never find a player who joined later and could name a slot's previous occupant. - SaveLoadout / CurrentLoadout log the player in the given slot. - GameServerLib::Update scans entrants for the dirty flag. The copy came from 1b323fc ("lobby pointer invalid after Initialize returns"). That premise no longer holds in this code: 2e90317 restored m_lobby, and GetTcpBroadcaster and UnregisterAllCallbacks dereference it after Initialize. 2e90317 also notes FinalizeInitialization was never called before it, so the accessors returned null during 1b323fc's stability run and that run did not exercise entrant reads. Reading on demand is the refresh point the binary itself uses: CNSLobby::SmiteEntrant indexes [this+0x360] at call time, and CNSLobby::Update calls IServerLib::Update (0x1406178c3) before it touches the array, on the same thread. All three readers run on that thread (the ServerDB queue is drained inside Update). GetEntrantCount reports 0 while items is null, and the header documents that the returned pointer must not outlive the callback. The smite path now logs the resolved slot at Info and the entrant count on a miss, so an empty-lobby miss can be told apart from a bad id. Test: test_server_context, wired into test-auth-unit. Before the fix 6/7 failed; after it 7/7 pass. Dropping the null-items guard fails FreedArrayReadsAsEmpty. Co-Authored-By: nevr-runtime --- justfile | 4 +- src/runtime/CMakeLists.txt | 10 ++ src/runtime/server/gameserver.cpp | 8 +- src/runtime/server/server_context.cpp | 30 ++--- src/runtime/server/server_context.h | 11 +- src/runtime/tests/test_server_context.cpp | 145 ++++++++++++++++++++++ 6 files changed, 184 insertions(+), 24 deletions(-) create mode 100644 src/runtime/tests/test_server_context.cpp diff --git a/justfile b/justfile index d5c01ec9..b0590ba4 100644 --- a/justfile +++ b/justfile @@ -279,7 +279,7 @@ test-auth-unit: unset VCPKG_ROOT cmake --preset {{ preset }} -DBUILD_TESTING=ON > /dev/null 2>&1 \ || cmake --preset {{ preset }} -DBUILD_TESTING=ON - cmake --build --preset {{ preset }} --target test_xpid_patch --target test_parse_endpoint --target test_behavioral --target test_token_auth --target test_messages --target test_crash_recovery --target test_nevr_config --target test_service_map --target test_social_facade --target test_scenario_early_quit --target test_early_quit_lockout --target test_schannel_cred_guard --target test_hooking --target test_plugin_load_plan --target test_system_module_loader --target test_login_redirect_override --target test_websocket_frame --target test_protobuf_transport --target test_url_diagnostics --target test_callback_unregistration --target test_session_unregister --target test_mic_lifecycle --target test_telemetry_snapshot_store --target test_coop_ai_trace + cmake --build --preset {{ preset }} --target test_xpid_patch --target test_parse_endpoint --target test_behavioral --target test_token_auth --target test_messages --target test_crash_recovery --target test_nevr_config --target test_service_map --target test_social_facade --target test_scenario_early_quit --target test_early_quit_lockout --target test_schannel_cred_guard --target test_hooking --target test_plugin_load_plan --target test_system_module_loader --target test_login_redirect_override --target test_websocket_frame --target test_protobuf_transport --target test_url_diagnostics --target test_callback_unregistration --target test_server_context --target test_session_unregister --target test_mic_lifecycle --target test_telemetry_snapshot_store --target test_coop_ai_trace cmake --build --preset {{ preset }} --target test_mic_dsp cmake --build --preset {{ preset }} --target test_game_image_guard bin="build/{{ preset }}/bin/test_xpid_patch.exe" @@ -384,7 +384,7 @@ test-auth-unit: exit 1 fi wine "$bin" - for test_name in test_system_module_loader test_login_redirect_override test_websocket_frame test_protobuf_transport test_url_diagnostics test_callback_unregistration test_session_unregister test_mic_lifecycle test_telemetry_snapshot_store test_coop_ai_trace; do + for test_name in test_system_module_loader test_login_redirect_override test_websocket_frame test_protobuf_transport test_url_diagnostics test_callback_unregistration test_server_context test_session_unregister test_mic_lifecycle test_telemetry_snapshot_store test_coop_ai_trace; do bin="build/{{ preset }}/bin/${test_name}.exe" if [[ ! -f "$bin" ]]; then echo "ERROR: GTest binary not found: $bin" >&2 diff --git a/src/runtime/CMakeLists.txt b/src/runtime/CMakeLists.txt index 8fe66a30..e98c54f6 100644 --- a/src/runtime/CMakeLists.txt +++ b/src/runtime/CMakeLists.txt @@ -324,6 +324,16 @@ if(BUILD_TESTING) target_link_libraries(test_callback_unregistration PRIVATE GTest::gtest GTest::gtest_main) gtest_discover_tests(test_callback_unregistration DISCOVERY_MODE PRE_TEST) + # Issue #38: entrant accessors read the lobby's live entrant array. + add_executable(test_server_context + tests/test_server_context.cpp + server/server_context.cpp) + set_source_files_properties(tests/test_server_context.cpp + PROPERTIES SKIP_PRECOMPILE_HEADERS ON) + target_include_directories(test_server_context PRIVATE ${CMAKE_SOURCE_DIR}/src) + target_link_libraries(test_server_context PRIVATE GTest::gtest GTest::gtest_main) + gtest_discover_tests(test_server_context DISCOVERY_MODE PRE_TEST) + add_executable(test_session_unregister tests/test_session_unregister.cpp server/session_unregister.cpp diff --git a/src/runtime/server/gameserver.cpp b/src/runtime/server/gameserver.cpp index 53e06f86..b41ea31b 100644 --- a/src/runtime/server/gameserver.cpp +++ b/src/runtime/server/gameserver.cpp @@ -417,11 +417,15 @@ void OnTcpMsgProtobuf(GameServerLib* self, VOID*, EchoVR::TcpPeer, VOID* msg, VO } if (!found) { - Log(EchoVR::LogLevel::Warning, "[NEVR.GAMESERVER] Smite entrant not found in lobby: %s", - smite.entrant_id().c_str()); + Log(EchoVR::LogLevel::Warning, "[NEVR.GAMESERVER] Smite entrant not found in lobby: %s entrants=%llu", + smite.entrant_id().c_str(), static_cast(entrantCount)); break; } + Log(EchoVR::LogLevel::Info, "[NEVR.GAMESERVER] Smite entrant resolved: entrant=%s slot=%llu entrants=%llu", + smite.entrant_id().c_str(), static_cast(slotIndex), + static_cast(entrantCount)); + if (broadcaster) { auto encoded = EncodeLobbySmiteEntrant(slotIndex); EchoVR::BroadcasterReceiveLocalEvent(broadcaster, Sym::LobbySmiteEntrant, "SNSLobbySmiteEntrant", diff --git a/src/runtime/server/server_context.cpp b/src/runtime/server/server_context.cpp index 6b3d9740..5d920a05 100644 --- a/src/runtime/server/server_context.cpp +++ b/src/runtime/server/server_context.cpp @@ -43,14 +43,6 @@ void SessionState::Reset() { void ServerContext::Initialize(EchoVR::Lobby* lobby, EchoVR::Broadcaster* broadcaster) { std::unique_lock lock(m_stateMutex); - if (lobby && lobby->entrantData.items) { - m_cachedEntrants.clear(); - m_cachedEntrants.reserve(lobby->entrantData.count); - for (uint64_t i = 0; i < lobby->entrantData.count; ++i) { - m_cachedEntrants.push_back(lobby->entrantData.items[i]); - } - } - m_lobby = lobby; m_broadcaster = broadcaster; @@ -75,7 +67,6 @@ void ServerContext::Terminate() { m_state = ServerState::Terminated; m_lobby = nullptr; m_broadcaster = nullptr; - m_cachedEntrants.clear(); m_serverDbPeer = EchoVR::TcpPeer_InvalidPeer; { @@ -179,28 +170,39 @@ EchoVR::TcpBroadcasterData* ServerContext::GetTcpBroadcaster() const { return nullptr; } +// Entrants are read from the lobby's live array on every call (issue #38). The +// game hands us the lobby in IServerLib::Initialize, from CNSLobby +// LoadServerSupport (0x14060bb70) during boot, before anyone has joined, so a +// copy taken there stays empty. CNSLobby::Update (0x140617890) calls +// IServerLib::Update first (0x1406178c3) and then mutates [lobby+0x360] itself, +// on the same thread, so a read made inside one of our callbacks sees a +// consistent array. EchoVR::Lobby::EntrantData* ServerContext::GetEntrant(uint32_t index) const { std::shared_lock lock(m_stateMutex); - if (m_state == ServerState::Uninitialized || m_state == ServerState::Terminated) { + if (m_state == ServerState::Uninitialized || m_state == ServerState::Terminated || !m_lobby) { return nullptr; } - if (index >= m_cachedEntrants.size()) { + const auto& entrants = m_lobby->entrantData; + if (!entrants.items || index >= entrants.count) { return nullptr; } - return const_cast(&m_cachedEntrants[index]); + return &entrants.items[index]; } uint64_t ServerContext::GetEntrantCount() const { std::shared_lock lock(m_stateMutex); - if (m_state == ServerState::Uninitialized || m_state == ServerState::Terminated) { + if (m_state == ServerState::Uninitialized || m_state == ServerState::Terminated || !m_lobby) { return 0; } - return m_cachedEntrants.size(); + // Never report a count we could not index (echovr-reconstruction + // CNSLobby.cpp:473 frees the array and zeroes items and count together). + const auto& entrants = m_lobby->entrantData; + return entrants.items ? entrants.count : 0; } void ServerContext::SetServerDbPeer(const EchoVR::TcpPeer& peer) { diff --git a/src/runtime/server/server_context.h b/src/runtime/server/server_context.h index ef2b377e..671d3737 100644 --- a/src/runtime/server/server_context.h +++ b/src/runtime/server/server_context.h @@ -3,7 +3,6 @@ #include #include #include -#include #include "abi/echovr.h" #include "core/pch.h" @@ -100,8 +99,11 @@ class ServerContext { EchoVR::Broadcaster* GetBroadcaster() const; EchoVR::TcpBroadcasterData* GetTcpBroadcaster() const; - // Safe entrant access with bounds checking (shared lock) - // Returns nullptr if index out of bounds or not initialized + // Entrant access, read live from the lobby's entrant array on every call + // (issue #38). Returns nullptr if index is out of bounds or not initialized. + // The pointer is into game memory: use it within the current game-thread + // callback and never store it — the game may free or rewrite the array on + // its next tick. EchoVR::Lobby::EntrantData* GetEntrant(uint32_t index) const; uint64_t GetEntrantCount() const; @@ -130,9 +132,6 @@ class ServerContext { EchoVR::Lobby* m_lobby = nullptr; EchoVR::Broadcaster* m_broadcaster = nullptr; - // Cached entrant data (owns the data, not pointers from game) - std::vector m_cachedEntrants; - // ServerDB connection EchoVR::TcpPeer m_serverDbPeer = EchoVR::TcpPeer_InvalidPeer; diff --git a/src/runtime/tests/test_server_context.cpp b/src/runtime/tests/test_server_context.cpp new file mode 100644 index 00000000..5d8186fb --- /dev/null +++ b/src/runtime/tests/test_server_context.cpp @@ -0,0 +1,145 @@ +// ServerContext entrant access (issue #38). +// +// The entrant accessors must answer from the lobby's live entrant array, not +// from a copy taken at IServerLib::Initialize. The game calls Initialize from +// CNSLobby LoadServerSupport (echovr.exe 0x14060bb70) while the server is still +// booting, before any player has joined, so a copy taken there is empty for +// the life of the process. Every test below mutates the fake lobby AFTER +// Initialize and asserts the accessors see the mutation. +#include "runtime/server/server_context.h" + +#include + +#include +#include +#include + +namespace { + +using Entrant = EchoVR::Lobby::EntrantData; + +// Points the fake lobby's entrant HeapArray at `entrants`. +void Attach(EchoVR::Lobby& lobby, std::vector& entrants) { + lobby.entrantData.items = entrants.empty() ? nullptr : entrants.data(); + lobby.entrantData.count = entrants.size(); +} + +Entrant MakeEntrant(uint64_t accountId) { + Entrant entrant{}; + entrant.userId.accountId = accountId; + return entrant; +} + +} // namespace + +TEST(ServerContextEntrants, JoinAfterInitializeIsVisible) { + EchoVR::Lobby lobby{}; + GameServer::ServerContext context; + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + ASSERT_EQ(context.GetEntrantCount(), 0U); + + std::vector entrants = {MakeEntrant(11), MakeEntrant(22), MakeEntrant(33)}; + Attach(lobby, entrants); + + EXPECT_EQ(context.GetEntrantCount(), 3U); + ASSERT_NE(context.GetEntrant(2), nullptr); + EXPECT_EQ(context.GetEntrant(2)->userId.accountId, 33U); +} + +TEST(ServerContextEntrants, LeaveAfterInitializeIsVisible) { + EchoVR::Lobby lobby{}; + std::vector entrants = {MakeEntrant(11), MakeEntrant(22), MakeEntrant(33)}; + Attach(lobby, entrants); + GameServer::ServerContext context; + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + ASSERT_EQ(context.GetEntrantCount(), 3U); + + lobby.entrantData.count = 1; + + EXPECT_EQ(context.GetEntrantCount(), 1U); + EXPECT_EQ(context.GetEntrant(1), nullptr); + EXPECT_EQ(context.GetEntrant(2), nullptr); +} + +TEST(ServerContextEntrants, SlotReusedByAnotherPlayerReportsTheNewPlayer) { + // The smite path resolves an entrant to a slot index and hands that index to + // the game. A stale copy here would name the slot's previous occupant. + EchoVR::Lobby lobby{}; + std::vector entrants = {MakeEntrant(11)}; + Attach(lobby, entrants); + GameServer::ServerContext context; + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + + entrants[0].userId.accountId = 99; + + ASSERT_NE(context.GetEntrant(0), nullptr); + EXPECT_EQ(context.GetEntrant(0)->userId.accountId, 99U); +} + +TEST(ServerContextEntrants, ReturnsTheGameElementNotACopy) { + EchoVR::Lobby lobby{}; + std::vector entrants = {MakeEntrant(11), MakeEntrant(22)}; + Attach(lobby, entrants); + GameServer::ServerContext context; + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + + EXPECT_EQ(context.GetEntrant(0), &entrants[0]); + EXPECT_EQ(context.GetEntrant(1), &entrants[1]); +} + +TEST(ServerContextEntrants, ElementStrideMatchesTheBinary) { + // echovr.exe indexes [lobby+0x360] with a 0xD8 stride: CNSLobby::SmiteEntrant + // 0x14061665d IMUL RAX,RDX,0xd8; fcn_1406082b0 0x1406082c9 IMUL RDX,RDX,0xd8; + // 0x14061698f ADD R8,0xd8. + EchoVR::Lobby lobby{}; + std::vector entrants = {MakeEntrant(1), MakeEntrant(2)}; + Attach(lobby, entrants); + GameServer::ServerContext context; + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + + const auto* first = reinterpret_cast(context.GetEntrant(0)); + const auto* second = reinterpret_cast(context.GetEntrant(1)); + ASSERT_NE(first, nullptr); + ASSERT_NE(second, nullptr); + EXPECT_EQ(second - first, 0xD8); +} + +TEST(ServerContextEntrants, FreedArrayReadsAsEmpty) { + // The game frees the array and zeroes items/count together (CNSLobby.cpp:473 + // in echovr-reconstruction). A null items pointer must never be indexed even + // if count is stale. + EchoVR::Lobby lobby{}; + std::vector entrants = {MakeEntrant(11)}; + Attach(lobby, entrants); + GameServer::ServerContext context; + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + + lobby.entrantData.items = nullptr; + + EXPECT_EQ(context.GetEntrantCount(), 0U); + EXPECT_EQ(context.GetEntrant(0), nullptr); +} + +TEST(ServerContextEntrants, UninitializedAndTerminatedReadAsEmpty) { + EchoVR::Lobby lobby{}; + std::vector entrants = {MakeEntrant(11)}; + Attach(lobby, entrants); + GameServer::ServerContext context; + + EXPECT_EQ(context.GetEntrantCount(), 0U); + EXPECT_EQ(context.GetEntrant(0), nullptr); + + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + EXPECT_EQ(context.GetEntrantCount(), 1U); + + context.Terminate(); + EXPECT_EQ(context.GetEntrantCount(), 0U); + EXPECT_EQ(context.GetEntrant(0), nullptr); +}