Subject: fix for vulnerability CVE-2010-0401 for OpenTTD 0.3.5 - 0.5.0 (Access restriction circumvention, remote crash) From: OpenTTD developer team Origin: backport, https://github.com/OpenTTD/OpenTTD/commit/11d6e21 https://github.com/OpenTTD/OpenTTD/commit/df4d8b3 Bug: https://github.com/OpenTTD/OpenTTD/issues/3754 It is possible to circumvent the server password of a network game. It is possible in two cases: 1.you know the company password of one of the companies 1.one of the companies has no password In both cases you send the company password (or “” if there is none) when you receive the request for the server’s password. This fix also prevents reading invalid data and possibly crashing a server when a spectator sends a company password packet. Even though this is technically a different vulnerability, the fix for the access restriction circumvention fixes the other vulnerability too as it will never request and thus accept company password packets from spectators. Note that this is a custom fix for backports as a more elobarate fix with a different network protocol was used for 1.0.1 and trunk (r19607). However, we cannot break the network protocol for older versions thus this version is needed. This version does not change the network protocol, except for a misbehaving client that sends the company password if the server password is expected. In this case the client is disconnected citing a protocol error. Index: network_data.h =================================================================== --- network_data.h (revision 19680) +++ network_data.h (working copy) @@ -47,12 +47,15 @@ typedef enum { STATUS_INACTIVE, + STATUS_AUTH_GAME, ///< The client is authorizing with game (server) password + STATUS_AUTH_COMPANY, ///< The client is authorizing with company password STATUS_AUTH, // This means that the client is authorized STATUS_MAP_WAIT, // This means that the client is put on hold because someone else is getting the map STATUS_MAP, STATUS_DONE_MAP, STATUS_PRE_ACTIVE, STATUS_ACTIVE, + STATUS_END } ClientStatus; typedef enum { Index: console_cmds.c =================================================================== --- console_cmds.c (revision 19680) +++ console_cmds.c (working copy) @@ -512,7 +512,7 @@ DEF_CONSOLE_CMD(ConStatus) { - static const char *stat_str[] = {"inactive", "authorized", "waiting", "loading map", "map done", "ready", "active"}; + static const char *stat_str[] = {"inactive", "authorizing (server)", "authorizing (company)", "authorized", "waiting", "loading map", "map done", "ready", "active"}; const char *status; const NetworkClientState *cs; Index: network_server.c =================================================================== --- network_server.c (revision 19680) +++ network_server.c (working copy) @@ -192,7 +192,14 @@ // uint8: Type of password // - Packet *p = NetworkSend_Init(PACKET_SERVER_NEED_PASSWORD); + Packet *p; + + /* Invalid packet when status is AUTH or higher */ + if (cs->status >= STATUS_AUTH) return; + + cs->status = type == NETWORK_GAME_PASSWORD ? STATUS_AUTH_GAME : STATUS_AUTH_COMPANY; + + p = NetworkSend_Init(PACKET_SERVER_NEED_PASSWORD); NetworkSend_uint8(p, type); NetworkSend_Packet(p, cs); } @@ -662,7 +669,7 @@ type = NetworkRecv_uint8(cs, p); NetworkRecv_string(cs, p, password, sizeof(password)); - if (cs->status == STATUS_INACTIVE && type == NETWORK_GAME_PASSWORD) { + if (cs->status == STATUS_AUTH_GAME && type == NETWORK_GAME_PASSWORD) { // Check game-password if (strncmp(password, _network_game_info.server_password, sizeof(password)) != 0) { // Password is invalid @@ -680,7 +687,7 @@ // Valid password, allow user SEND_COMMAND(PACKET_SERVER_WELCOME)(cs); return; - } else if (cs->status == STATUS_INACTIVE && type == NETWORK_COMPANY_PASSWORD) { + } else if (cs->status == STATUS_AUTH_COMPANY && type == NETWORK_COMPANY_PASSWORD) { ci = DEREF_CLIENT_INFO(cs); if (strncmp(password, _network_player_info[ci->client_playas - 1].password, sizeof(password)) != 0) {