Subject: fix for vulnerability CVE-2006-1998 for OpenTTD 0.4.5 - 0.4.7 (Denial of service (server) via invalid error number)
From: OpenTTD developer team <info@openttd.org>
Origin: backport, https://github.com/OpenTTD/OpenTTD/commit/7d232cf https://github.com/OpenTTD/OpenTTD/commit/e9a5ca7 https://github.com/OpenTTD/OpenTTD/commit/1c5c32d
Bug:

Both client and server handle a type of command (PACKET_SERVER_ERROR
and PACKET_CLIENT_ERROR) for the visualization of some pre-built errors
in the console.
The problem happens when an attacker sends an invalid big error number
(8 bit) which forces the program to terminate spontaneously through the
usage of the error() function.
The bug is exploitable only in-game so the attacker must have access to
the server: his IP must not be banned, he must know the password if it
has been set and the server must not be full.


Index: network.c
===================================================================
--- network.c
+++ network.c
@@ -256,6 +256,36 @@
 	_networking = false;
 }

+/** Retrieve a string representation of an internal error number
+ * @param buf buffer where the error message will be stored
+ * @param err NetworkErrorCode (integer)
+ * @return returns a pointer to the error message (buf) */
+char *GetNetworkErrorMsg(char *buf, NetworkErrorCode err)
+{
+	/* List of possible network errors, used by
+	 * PACKET_SERVER_ERROR and PACKET_CLIENT_ERROR */
+	static const StringID network_error_strings[] = {
+		STR_NETWORK_ERR_CLIENT_GENERAL,
+		STR_NETWORK_ERR_CLIENT_DESYNC,
+		STR_NETWORK_ERR_CLIENT_SAVEGAME,
+		STR_NETWORK_ERR_CLIENT_CONNECTION_LOST,
+		STR_NETWORK_ERR_CLIENT_PROTOCOL_ERROR,
+		STR_NETWORK_ERR_CLIENT_NOT_AUTHORIZED,
+		STR_NETWORK_ERR_CLIENT_NOT_EXPECTED,
+		STR_NETWORK_ERR_CLIENT_WRONG_REVISION,
+		STR_NETWORK_ERR_CLIENT_NAME_IN_USE,
+		STR_NETWORK_ERR_CLIENT_WRONG_PASSWORD,
+		STR_NETWORK_ERR_CLIENT_PLAYER_MISMATCH,
+		STR_NETWORK_ERR_CLIENT_KICKED,
+		STR_NETWORK_ERR_CLIENT_CHEATER,
+		STR_NETWORK_ERR_CLIENT_SERVER_FULL,
+	};
+
+	if (err >= lengthof(network_error_strings)) err = 0;
+
+	return GetString(buf, network_error_strings[err]);
+}
+
 // Find all IP-aliases for this host
 static void NetworkFindIPs(void)
 {
@@ -524,7 +554,7 @@

 		NetworkGetClientName(client_name, sizeof(client_name), cs);

-		GetString(str, STR_NETWORK_ERR_CLIENT_GENERAL + errorno);
+		GetNetworkErrorMsg(str, errorno);

 		NetworkTextMessage(NETWORK_ACTION_LEAVE, 1, false, client_name, "%s", str);

Index: network_data.h
===================================================================
--- network_data.h
+++ network_data.h
@@ -230,6 +230,7 @@
 NetworkClientInfo *NetworkFindClientInfoFromIP(const char *ip);
 NetworkClientState *NetworkFindClientStateFromIndex(uint16 client_index);
 unsigned long NetworkResolveHost(const char *hostname);
+char *GetNetworkErrorMsg(char *buf, NetworkErrorCode err);

 #endif /* ENABLE_NETWORK */

Index: network_client.c
===================================================================
--- network_client.c
+++ network_client.c
@@ -683,16 +683,13 @@

 DEF_CLIENT_RECEIVE_COMMAND(PACKET_SERVER_ERROR_QUIT)
 {
-	int errorno;
 	char str[100];
 	uint16 index;
 	NetworkClientInfo *ci;

 	index = NetworkRecv_uint16(MY_CLIENT, p);
-	errorno = NetworkRecv_uint8(MY_CLIENT, p);
+	GetNetworkErrorMsg(str, NetworkRecv_uint8(MY_CLIENT, p));

-	GetString(str, STR_NETWORK_ERR_CLIENT_GENERAL + errorno);
-
 	ci = NetworkFindClientInfoFromIndex(index);
 	if (ci != NULL) {
 		NetworkTextMessage(NETWORK_ACTION_LEAVE, 1, false, ci->client_name, "%s", str);
Index: network_server.c
===================================================================
--- network_server.c
+++ network_server.c
@@ -29,8 +29,6 @@

 void NetworkPopulateCompanyInfo(void);

-// Is the network enabled?
-
 // **********
 // Sending functions
 //   DEF_SERVER_SEND_COMMAND has parameter: NetworkClientState *cs
@@ -148,7 +146,7 @@
 	NetworkSend_uint8(p, error);
 	NetworkSend_Packet(p, cs);

-	GetString(str, STR_NETWORK_ERR_CLIENT_GENERAL + error);
+	GetNetworkErrorMsg(str, error);

 	// Only send when the current client was in game
 	if (cs->status > STATUS_AUTH) {
@@ -890,8 +889,8 @@
 	// This packets means a client noticed an error and is reporting this
 	//  to us. Display the error and report it to the other clients
 	NetworkClientState *new_cs;
-	byte errorno = NetworkRecv_uint8(cs, p);
 	char str[100];
+	NetworkErrorCode errorno = NetworkRecv_uint8(cs, p);
 	char client_name[NETWORK_CLIENT_NAME_LENGTH];

 	// The client was never joined.. thank the client for the packet, but ignore it
@@ -902,7 +901,7 @@

 	NetworkGetClientName(client_name, sizeof(client_name), cs);

-	GetString(str, STR_NETWORK_ERR_CLIENT_GENERAL + errorno);
+	GetNetworkErrorMsg(str, errorno);

 	DEBUG(net, 2)("[NET] %s reported an error and is closing his connection (%s)", client_name, str);