Subject: fix for vulnerability CVE-2010-0401 for OpenTTD 0.3.5 - 0.5.0 (Access restriction circumvention, remote crash)
From: OpenTTD developer team <info@openttd.org>
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) {