# HG changeset patch
# User Patric Stout <truebrain@openttd.org>
# Date 2021-07-21 19:55:30
# Node ID b48d7959e189112d2d86dddaa86293ec1a4165fc
# Parent  bc69ffde9725ccdb125ec13ba34b9c2cefdf218b

Fix: crash when joining a server again after a TCP disconnect (#9453)

"my_client" wasn't always free'd when a game ended. "my_client"
keeps a reference inside the PT_NCLIENT pool. The rest of the
code assumes that when you are not in a game, it can freely
reset this pool.
In result: several ways to trigger a use-after-free.

diff --git a/src/network/core/tcp_game.cpp b/src/network/core/tcp_game.cpp
--- a/src/network/core/tcp_game.cpp
+++ b/src/network/core/tcp_game.cpp
@@ -48,7 +48,7 @@ NetworkRecvStatus NetworkGameSocketHandl
 		_networking = false;
 		ShowErrorMessage(STR_NETWORK_ERROR_LOSTCONNECTION, INVALID_STRING_ID, WL_CRITICAL);
 
-		return NETWORK_RECV_STATUS_CLIENT_QUIT;
+		return this->CloseConnection(NETWORK_RECV_STATUS_CLIENT_QUIT);
 	}
 
 	return this->CloseConnection(NETWORK_RECV_STATUS_CONNECTION_LOST);
diff --git a/src/network/network_client.cpp b/src/network/network_client.cpp
--- a/src/network/network_client.cpp
+++ b/src/network/network_client.cpp
@@ -160,24 +160,19 @@ ClientNetworkGameSocketHandler::~ClientN
 NetworkRecvStatus ClientNetworkGameSocketHandler::CloseConnection(NetworkRecvStatus status)
 {
 	assert(status != NETWORK_RECV_STATUS_OKAY);
-	/*
-	 * Sending a message just before leaving the game calls cs->SendPackets.
-	 * This might invoke this function, which means that when we close the
-	 * connection after cs->SendPackets we will close an already closed
-	 * connection. This handles that case gracefully without having to make
-	 * that code any more complex or more aware of the validity of the socket.
-	 */
-	if (this->sock == INVALID_SOCKET) return status;
+	assert(this->sock != INVALID_SOCKET);
+
+	if (!this->HasClientQuit()) {
+		Debug(net, 3, "Closed client connection {}", this->client_id);
+
+		this->SendPackets(true);
 
-	Debug(net, 3, "Closed client connection {}", this->client_id);
-
-	this->SendPackets(true);
-
-	/* Wait a number of ticks so our leave message can reach the server.
-	 * This is especially needed for Windows servers as they seem to get
-	 * the "socket is closed" message before receiving our leave message,
-	 * which would trigger the server to close the connection as well. */
-	CSleep(3 * MILLISECONDS_PER_TICK);
+		/* Wait a number of ticks so our leave message can reach the server.
+		* This is especially needed for Windows servers as they seem to get
+		* the "socket is closed" message before receiving our leave message,
+		* which would trigger the server to close the connection as well. */
+		CSleep(3 * MILLISECONDS_PER_TICK);
+	}
 
 	delete this;
 
@@ -256,7 +251,7 @@ void ClientNetworkGameSocketHandler::Cli
 /* static */ void ClientNetworkGameSocketHandler::Send()
 {
 	my_client->SendPackets();
-	my_client->CheckConnection();
+	if (my_client != nullptr) my_client->CheckConnection();
 }
 
 /**