News:

Congratulations!
 You've won the News Item Lottery! Your prize? Reading this news item! :)

network: never trust wire-supplied our_client_id on server

Started by janry, Yesterday at 03:06:22 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

janry

    The our_client_id field is part of every wire packet and is just bytes
    the peer wrote. nwc_tool_t::clone fed it straight into
    socket_list_t::get_client (network_cmd_ingame.cc:1178 pre-patch) with
    no bounds check, so a TCP-connected peer could either crash the server
    (0xFFFFFFFF, vector_tpl bounds fatal) or impersonate any other client
    (claim that client's slot to inherit its player_unlocked bitmap,
    bypassing the auth check) with one unauthenticated 59-byte NWC_TOOL
    packet. The other two consumer sites (nwc_auth_player_t::execute,
    nwc_chg_player_t::clone) catch the crash with is_valid_client_id but
    not the impersonation.
   
    Root-cause fix: on the server side of read_from_packet, override
    our_client_id with the socket-derived id immediately after parse.
    Wire value becomes informational only; every later consumer reads
    the real slot. Routed through network_cmd_ingame.cc rather than the
    nettool-shared network_cmd.cc so nettool's TU is unaffected.
   
    Defense in depth: nwc_tool_t::clone now bounds-checks our_client_id
    before indexing, matching nwc_chg_player_t::clone — a slot can still
    go inactive between receive and execute even once the wire value is
    no longer in play.