Refactor game_info packet handler

game_patch/multi/network.cpp

@@ -72,8 +72,6 @@ static std::unordered_map<uint64_t, AfGiReqSeen> g_af_gi_req_seen;
 static std::unordered_map<uint64_t, RconAccessEntry> g_rcon_access_by_addr;
 
 // Client-side: stashed game_info packet for AF extension parsing
-static std::vector<uint8_t> g_game_info_stashed;
-
 // Client-side: per-server extra data parsed from AF game_info extension
 static std::unordered_map<uint64_t, AFGameInfoExtra> g_server_browser_extra;
 
@@ -138,9 +136,6 @@ class BufferOverflowPatch
 // Note: server browser internal functions use strings safely (see 0x0044DDCA for example)
 // Note: level filename was limited to 64 because of VPP format limits
 std::array g_buffer_overflow_patches{
-    BufferOverflowPatch{0x0047B2D3, 0x0047B2DE, 256}, // process_game_info_packet (server name)
-    BufferOverflowPatch{0x0047B334, 0x0047B33D, 256}, // process_game_info_packet (level name)
-    BufferOverflowPatch{0x0047B38E, 0x0047B397, 256}, // process_game_info_packet (mod name)
     BufferOverflowPatch{0x0047ACF6, 0x0047AD03, 32},  // process_join_req_packet (player name)
     BufferOverflowPatch{0x0047AD4E, 0x0047AD55, 256}, // process_join_req_packet (password)
     BufferOverflowPatch{0x0047A8AE, 0x0047A8B5, 64},  // process_join_accept_packet (level filename)
@@ -519,106 +514,176 @@ static void handle_rcon_request_packet(const uint8_t* pkt, size_t len, const rf:
     }
 }
 
-// Parse AF v2 extension from game_info packet tail using the AFFooter.
-// Tail layout: [stock fields...][af_game_info_ext_v2][level_filename\0][AFFooter]
-// The footer at the end of the payload lets us jump directly to the extension
-// without parsing the stock fields.
-static bool parse_game_info_af_tail(const uint8_t* pkt, size_t pkt_len, AFGameInfoExtra& out)
+std::vector<uint8_t> af_game_info_ext_v2::serialize_to_wire() const
 {
-    if (!pkt || pkt_len < sizeof(RF_GamePacketHeader))
-        return false;
-
-    RF_GamePacketHeader gh;
-    std::memcpy(&gh, pkt, sizeof(gh));
-    const size_t payload_len = gh.size;
-    if (pkt_len < sizeof(gh) + payload_len)
-        return false;
-
-    const uint8_t* payload = pkt + sizeof(gh);
-    const uint8_t* end = payload + payload_len;
-
-    // read footer from the end of the payload
-    if (payload_len < sizeof(AFFooter))
-        return false;
-    AFFooter footer;
-    std::memcpy(&footer, end - sizeof(AFFooter), sizeof(AFFooter));
-    if (footer.magic != AF_FOOTER_MAGIC)
-        return false;
-
-    // locate the extension data using footer.total_len
-    const size_t core_len = footer.total_len;
-    const uint8_t* footer_start = end - sizeof(AFFooter);
-    if (core_len > payload_len - sizeof(AFFooter))
-        return false;
-    const uint8_t* ext_start = footer_start - core_len;
-    if (ext_start < payload)
-        return false;
-
-    // parse af_game_info_ext_v2 from the extension data
-    if (core_len < sizeof(af_game_info_ext_v2))
-        return false;
+    // Wire layout: [sig][ver*4][flags][filename\0][bot_counts*4]
+    std::string fname = level_filename.substr(0, 63);
 
-    const auto* ext = reinterpret_cast<const af_game_info_ext_v2*>(ext_start);
-    if (ext->af_signature != ALPINE_FACTION_SIGNATURE)
-        return false;
+    std::vector<uint8_t> buf;
+    buf.reserve(wire_pre_fname_size + fname.size() + 1 + wire_post_fname_size);
 
-    const uint16_t ext_size = ext->ext_size;
-    if (ext_size < sizeof(af_game_info_ext_v2) || ext_size > core_len)
-        return false;
-
-    out.version_major = ext->version_major;
-    out.version_minor = ext->version_minor;
-    out.version_patch = ext->version_patch;
-    out.version_type = ext->version_type;
-    out.af_flags = ext->af_flags;
-    out.num_bots = ext->num_bots;
-    out.num_human_players = ext->num_human_players;
-    out.num_browsers = ext->num_browsers;
-    out.num_total_clients = ext->num_total_clients;
-
-    // level filename follows the fixed struct
-    const uint8_t* fname_start = ext_start + ext_size;
-    const uint8_t* p = fname_start;
-    while (p < footer_start && *p != '\0') ++p;
-    if (p >= footer_start)
-        return false;
-    out.level_filename.assign(reinterpret_cast<const char*>(fname_start), p - fname_start);
+    auto write = [&](const void* data, size_t n) {
+        const auto* p = static_cast<const uint8_t*>(data);
+        buf.insert(buf.end(), p, p + n);
+    };
 
-    return true;
+    // pre-filename fields
+    write(&af_signature, sizeof(af_signature));
+    write(&version_major, sizeof(version_major));
+    write(&version_minor, sizeof(version_minor));
+    write(&version_patch, sizeof(version_patch));
+    write(&version_type, sizeof(version_type));
+    write(&af_flags, sizeof(af_flags));
+
+    // filename
+    buf.insert(buf.end(), fname.begin(), fname.end());
+    buf.push_back(0);
+
+    // post-filename fields
+    write(&num_bots, sizeof(num_bots));
+    write(&num_human_players, sizeof(num_human_players));
+    write(&num_browsers, sizeof(num_browsers));
+    write(&num_total_clients, sizeof(num_total_clients));
+
+    return buf;
 }
 
+// Fully replaces the stock process_game_info_packet handler.
+// Parses stock fields with bounds checking (fixing buffer overflows in the stock code),
+// updates the server browser entry, and parses the AF extension inline.
 FunHook<MultiIoPacketHandler> process_game_info_packet_hook{
     0x0047B2A0,
     [](char* data, const rf::NetAddr& addr) {
-        process_game_info_packet_hook.call_target(data, addr);
+        const auto* payload = reinterpret_cast<const uint8_t*>(data);
 
-        // If this packet is from the server that we are connected to, use game_info for the netgame name
-        // Useful for joining using protocol handler because when we join we do not have the server name available yet
-        const char* server_name = data + 1;
-        if (addr == rf::netgame.server_addr) {
-            rf::netgame.name = server_name;
-        }
+        // Read payload length from the sub-packet header immediately before the payload
+        uint16_t payload_len;
+        std::memcpy(&payload_len, data - 2, sizeof(payload_len));
+        const uint8_t* end = payload + payload_len;
+
+        if (payload_len < 1)
+            return;
+
+        const uint8_t* r = payload;
+        uint8_t version = *r++;
+
+        // Helper: read a null-terminated string safely into dst, advance r past the terminator
+        auto read_string = [&](char* dst, size_t dst_size) -> bool {
+            const uint8_t* start = r;
+            while (r < end && *r != '\0') ++r;
+            if (r >= end) return false;
+            size_t len = std::min(static_cast<size_t>(r - start), dst_size - 1);
+            std::memcpy(dst, start, len);
+            dst[len] = '\0';
+            ++r; // skip null terminator
+            return true;
+        };
+
+        // Server name is always present
+        char name[256]{};
+        if (!read_string(name, sizeof(name)))
+            return;
 
-        // Parse AF extension from the stashed packet data
-        if (!g_game_info_stashed.empty()) {
-            AFGameInfoExtra extra{};
+        if (version >= 0x89) {
+            // Full field set: game_type(1) + players(1) + max_players(1) + level\0 + mod\0 + flags(1)
+            if (end - r < 3) return;
+            uint8_t game_type = std::clamp<uint8_t>(*r++, 0, 7);
+            uint8_t players = *r++;
+            uint8_t max_players = *r++;
+
+            char level_name[256]{};
+            char mod_name[256]{};
+            if (!read_string(level_name, sizeof(level_name))) return;
+            if (!read_string(mod_name, sizeof(mod_name))) return;
+            if (r >= end) return;
+            uint8_t flags = *r++;
+
+            rf::multi_join_game_add_server(
+                addr.ip_addr, addr.port,
+                name, level_name, mod_name,
+                players, max_players, game_type, flags);
+
+            // Parse AF extension if present: [sig][ver*4][af_flags][filename\0][bot_counts*4]
+            constexpr size_t pre_sz = af_game_info_ext_v2::wire_pre_fname_size;
+            constexpr size_t post_sz = af_game_info_ext_v2::wire_post_fname_size;
             uint64_t key = addr_key(addr);
-            if (parse_game_info_af_tail(g_game_info_stashed.data(), g_game_info_stashed.size(), extra)) {
-                g_server_browser_extra[key] = std::move(extra);
+            bool parsed = false;
+
+            if (static_cast<size_t>(end - r) >= pre_sz + 1 + post_sz) {
+                uint32_t sig;
+                std::memcpy(&sig, r, sizeof(sig));
+
+                // r[4] = 19 matches the deprecated AF 1.3 beta format
+                if (sig == ALPINE_FACTION_SIGNATURE && r[4] != 19) {
+                    AFGameInfoExtra extra{};
+                    const uint8_t* ext_r = r + sizeof(sig);
+                    extra.version_major = *ext_r++;
+                    extra.version_minor = *ext_r++;
+                    extra.version_patch = *ext_r++;
+                    extra.version_type = *ext_r++;
+                    std::memcpy(&extra.af_flags, ext_r, sizeof(extra.af_flags));
+                    ext_r += sizeof(extra.af_flags);
+
+                    const uint8_t* fname_start = ext_r;
+                    const uint8_t* fname_limit = end - post_sz;
+                    while (ext_r < fname_limit && *ext_r != '\0') ++ext_r;
+                    if (ext_r < fname_limit) {
+                        extra.level_filename.assign(
+                            reinterpret_cast<const char*>(fname_start), ext_r - fname_start);
+                        ++ext_r;
+                        extra.num_bots = *ext_r++;
+                        extra.num_human_players = *ext_r++;
+                        extra.num_browsers = *ext_r++;
+                        extra.num_total_clients = *ext_r++;
+                        g_server_browser_extra[key] = std::move(extra);
+                        parsed = true;
+                    }
+                }
             }
-            else {
-                g_server_browser_extra.erase(key);
+
+            // DEPRECATED: AF 1.3 beta compat
+            // Old wire format: [sig][ext_size(2)][ext_version(1)][ver*4][af_flags][bot_counts*4][filename\0][AFFooter]
+            if (!parsed && static_cast<size_t>(end - r) >= 19 + 1 + sizeof(AFFooter)) {
+                uint32_t sig;
+                std::memcpy(&sig, r, sizeof(sig));
+                if (sig == ALPINE_FACTION_SIGNATURE && r[4] == 19) {
+                    AFGameInfoExtra extra{};
+                    const uint8_t* ext_r = r + sizeof(sig);
+                    ext_r += 3; // skip ext_size(2) + ext_version(1)
+                    extra.version_major = *ext_r++;
+                    extra.version_minor = *ext_r++;
+                    extra.version_patch = *ext_r++;
+                    extra.version_type = *ext_r++;
+                    std::memcpy(&extra.af_flags, ext_r, sizeof(extra.af_flags));
+                    ext_r += sizeof(extra.af_flags);
+                    extra.num_bots = *ext_r++;
+                    extra.num_human_players = *ext_r++;
+                    extra.num_browsers = *ext_r++;
+                    extra.num_total_clients = *ext_r++;
+                    const uint8_t* fname_start = ext_r;
+                    while (ext_r < end && *ext_r != '\0') ++ext_r;
+                    if (ext_r < end)
+                        extra.level_filename.assign(
+                            reinterpret_cast<const char*>(fname_start), ext_r - fname_start);
+                    g_server_browser_extra[key] = std::move(extra);
+                    parsed = true;
+                }
             }
-            g_game_info_stashed.clear();
+
+            if (!parsed)
+                g_server_browser_extra.erase(key);
+        }
+        else {
+            // RF 1.0/1.1 (0x87): display as incompatible
+            rf::multi_join_game_add_server(
+                addr.ip_addr, addr.port,
+                rf::strings::incompatible_version, "", "", 0, 0, 0, 0);
         }
-    },
-};
 
-CodeInjection process_game_info_packet_game_type_bounds_patch{
-    0x0047B30B,
-    [](auto& regs) {
-        // Valid game types are members of the NetGameType enum
-        regs.ecx = std::clamp<int>(regs.ecx, 0, 7);
+        // Update netgame name if this is from the connected server
+        if (addr == rf::netgame.server_addr) {
+            rf::netgame.name = name;
+        }
     },
 };
 
@@ -1244,7 +1309,7 @@ CallHook<int(const rf::NetAddr*, std::byte*, size_t)> send_game_info_packet_hook
         }
 
         if (req_ver >= 3) {
-            // v2 extension for modern AF clients: [af_game_info_ext_v2][fname\0]
+            // v2 extension for modern AF clients
             af_game_info_ext_v2 ext{};
             ext.set_flags(g_game_info_server_flags);
 
@@ -1265,25 +1330,10 @@ CallHook<int(const rf::NetAddr*, std::byte*, size_t)> send_game_info_packet_hook
             ext.num_browsers = num_browsers;
             ext.num_total_clients = static_cast<uint8_t>(num_bots + num_human_players + num_browsers);
 
-            // build tail: [ext][fname\0][AFFooter]
-            size_t fname_actual = fname_len ? fname_len : 1;
-            size_t core_len = sizeof(ext) + fname_actual;
-
-            AFFooter footer{};
-            footer.total_len = static_cast<uint16_t>(core_len);
-            footer.magic = AF_FOOTER_MAGIC;
-
-            std::vector<uint8_t> tail;
-            tail.reserve(core_len + sizeof(footer));
-            tail.insert(tail.end(), reinterpret_cast<const uint8_t*>(&ext),
-                reinterpret_cast<const uint8_t*>(&ext) + sizeof(ext));
             if (fname_len)
-                tail.insert(tail.end(), fname, fname + fname_len);
-            else
-                tail.push_back(0);
-            tail.insert(tail.end(), reinterpret_cast<const uint8_t*>(&footer),
-                reinterpret_cast<const uint8_t*>(&footer) + sizeof(footer));
+                ext.level_filename.assign(reinterpret_cast<const char*>(fname), fname_len - 1);
 
+            auto tail = ext.serialize_to_wire();
             auto [buf, new_len] = extend_packet_bytes(data, len, tail.data(), tail.size());
             return send_game_info_packet_hook.call_target(addr, buf.get(), new_len);
         }
@@ -1343,7 +1393,7 @@ CallHook<int(const rf::NetAddr*, std::byte*, size_t)> send_game_info_req_packet_
     0x0047B470,
     [](const rf::NetAddr* addr, std::byte* data, size_t len) {
         AFGameInfoReq core{ALPINE_FACTION_SIGNATURE, 3};
-        // version 3: server sends af_game_info_ext_v2 with bot count and proper versioning
+        // version 3: server sends v2 extension with bot counts and proper versioning
         auto [buf, new_len] = append_af_tail(data, len, &core, sizeof(core));
         return send_game_info_req_packet_hook.call_target(addr, buf.get(), new_len);
     },
@@ -2560,13 +2610,6 @@ CodeInjection multi_io_process_packets_injection{
                 }
             }
         }
-        // client-side: stash game_info packet for AF extension parsing
-        if (!rf::is_server && packet_type == static_cast<int>(RF_GamePacketType::RF_GPT_GAME_INFO)) {
-            const auto* base = static_cast<const uint8_t*>(regs.ecx);
-            const int off = regs.ebp;
-            const int len = regs.edi;
-            g_game_info_stashed.assign(base + off, base + off + len);
-        }
     },
 };
 
@@ -2807,12 +2850,9 @@ void network_init()
     multi_get_obj_handle_from_server_handle_hook.install();
     multi_set_obj_handle_mapping_hook.install();
 
-    // Use server name from game_info packet for netgame name if address matches current server
+    // Replace stock process_game_info_packet with bounds-checked parser + AF extension support
     process_game_info_packet_hook.install();
 
-    // Fix game_type out of bounds vulnerability in game_info packet
-    process_game_info_packet_game_type_bounds_patch.install();
-
     // Fix shape_index out of bounds vulnerability in boolean packet
     process_boolean_packet_validate_shape_index_patch.install();