Subject: fix for vulnerability CVE-2011-3341 for OpenTTD 1.0.1 - 1.0.5 (Denial of service via improperly validated commands)
From: OpenTTD developer team <info@openttd.org>
Origin: backport, http://vcs.openttd.org/svn/changeset/22845 
Bug: http://bugs.openttd.org/task/4745 

In multiple places in-game commands are not properly validated that allow remote 
attackers to cause a denial of service (crash) and possibly execute arbitrary 
code via unspecified vectors.

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.

The major cause of these bugs are off-by-one errors in the validation of the 
sent commands from the clients to the server, and from the server to the client. 
One could therefore, in theory, affect both the server and the clients of that 
server.

Two of the cases (since 0.7.0) are known to make the game state invalid, which 
causes an eventual crash of the application via an "abort()". Two cases cause a 
read beyond the boundaries of a (static) table (resp. since 0.3.5 and 1.0.0). 
The last case allows changes to the game state of others that might trigger 
invalid reads for other players if they had the autoreplace window opened (since 
0.6.0).

Index: src/order_cmd.cpp
===================================================================
--- src/order_cmd.cpp	(revision 22703)
+++ src/order_cmd.cpp	(working copy)
@@ -585,10 +585,10 @@
 		case OT_CONDITIONAL: {
 			VehicleOrderID skip_to = new_order.GetConditionSkipToOrder();
 			if (skip_to != 0 && skip_to >= v->GetNumOrders()) return CMD_ERROR; // Always allow jumping to the first (even when there is no order).
-			if (new_order.GetConditionVariable() > OCV_END) return CMD_ERROR;
+			if (new_order.GetConditionVariable() >= OCV_END) return CMD_ERROR;
 
 			OrderConditionComparator occ = new_order.GetConditionComparator();
-			if (occ > OCC_END) return CMD_ERROR;
+			if (occ >= OCC_END) return CMD_ERROR;
 			switch (new_order.GetConditionVariable()) {
 				case OCV_REQUIRES_SERVICE:
 					if (occ != OCC_IS_TRUE && occ != OCC_IS_FALSE) return CMD_ERROR;
Index: src/company_cmd.cpp
===================================================================
--- src/company_cmd.cpp	(revision 22703)
+++ src/company_cmd.cpp	(working copy)
@@ -686,7 +686,7 @@
 	GroupID id_g = GB(p1, 16, 16);
 	CommandCost cost;
 
-	if (!Group::IsValidID(id_g) && !IsAllGroupID(id_g) && !IsDefaultGroupID(id_g)) return CMD_ERROR;
+	if (Group::IsValidID(id_g) ? Group::Get(id_g)->owner != _current_company : !IsAllGroupID(id_g) && !IsDefaultGroupID(id_g)) return CMD_ERROR;
 	if (!Engine::IsValidID(old_engine_type)) return CMD_ERROR;
 
 	if (new_engine_type != INVALID_ENGINE) {
Index: src/vehicle_cmd.cpp
===================================================================
--- src/vehicle_cmd.cpp	(revision 22703)
+++ src/vehicle_cmd.cpp	(working copy)
@@ -181,10 +181,11 @@
 
 	CommandCost cost(EXPENSES_NEW_VEHICLES);
 	VehicleType vehicle_type = Extract<VehicleType, 0, 3>(p1);
-	uint sell_command = GetCmdSellVeh(vehicle_type);
 
 	if (!IsCompanyBuildableVehicleType(vehicle_type)) return CMD_ERROR;
 
+	uint sell_command = GetCmdSellVeh(vehicle_type);
+
 	/* Get the list of vehicles in the depot */
 	BuildDepotVehicleList(vehicle_type, tile, &list, &list);
 
Index: src/network/network_command.cpp
===================================================================
--- src/network/network_command.cpp	(revision 22703)
+++ src/network/network_command.cpp	(working copy)
@@ -208,7 +208,7 @@
 	if (!IsValidCommand(cp->cmd))               return "invalid command";
 	if (GetCommandFlags(cp->cmd) & CMD_OFFLINE) return "offline only command";
 	if ((cp->cmd & CMD_FLAGS_MASK) != 0)        return "invalid command flag";
-	if (callback > lengthof(_callback_table))   return "invalid callback";
+	if (callback >= lengthof(_callback_table))  return "invalid callback";
 
 	cp->callback = _callback_table[callback];
 	return NULL;