cpp/openttd-patchpack/source Changeset - r25944:76ed289c5cb6

Changeset - r25944:76ed289c5cb6

Parent rev.

Child rev.

[Not reviewed]

master

0 4 0

Patric Stout - 3 years ago 2021-09-05 16:17:39
truebrain@openttd.org

Fix: use-after-free after ClientNetworkCoordinatorSocketHandler::CloseAllConnections() (#9534)

The function clears all stun-handlers. This causes all of those
objects to be destroyed.
A handler can have a pending connecter, which was only killed in
case CloseConnection() was called. This is never the case when
the object is destroyed. In result, the connecter could finish
and cause a use-after-free by calling into the (now deleted)
handler.

4 files changed with 20 insertions and 6 deletions:

src/network/network_stun.cpp

src/network/network_stun.h

src/network/network_turn.cpp

src/network/network_turn.h

0 comments (0 inline, 0 general)

src/network/network_stun.cpp

➞

Show inline comments

@@ @@ -100,9 +100,7 @@ NetworkRecvStatus ClientNetworkStunSocke @@
+{
 	NetworkStunSocketHandler::CloseConnection(error);
 	/* If our connecter is still pending, shut it down too. Otherwise the
 	 * callback of the connecter can call into us, and our object is most
 	 * likely about to be destroyed. */
 	/* Also make sure any pending connecter is killed ASAP. */
 	if (this->connecter != nullptr) {
 		this->connecter->Kill();
 		this->connecter = nullptr;
@@ @@ -111,6 +109,14 @@ NetworkRecvStatus ClientNetworkStunSocke @@
 	return NETWORK_RECV_STATUS_OKAY;
+}
 ClientNetworkStunSocketHandler::~ClientNetworkStunSocketHandler()
+{
 	if (this->connecter != nullptr) {
 		this->connecter->Kill();
 		this->connecter = nullptr;
+	}
+}
 /**
  * Check whether we received/can send some data from/to the STUN server and
  * when that's the case handle it appropriately.

src/network/network_stun.h

➞

Show inline comments

@@ @@ -24,6 +24,7 @@ public: @@
 	NetworkAddress local_addr;         ///< Local addresses of the socket.
 	NetworkRecvStatus CloseConnection(bool error = true) override;
 	~ClientNetworkStunSocketHandler() override;
 	void SendReceive();
 	void Connect(const std::string &token, uint8 family);

src/network/network_turn.cpp

➞

Show inline comments

@@ @@ -108,9 +108,7 @@ NetworkRecvStatus ClientNetworkTurnSocke @@
+{
 	NetworkTurnSocketHandler::CloseConnection(error);
 	/* If our connecter is still pending, shut it down too. Otherwise the
 	 * callback of the connecter can call into us, and our object is most
 	 * likely about to be destroyed. */
 	/* Also make sure any pending connecter is killed ASAP. */
 	if (this->connecter != nullptr) {
 		this->connecter->Kill();
 		this->connecter = nullptr;
@@ @@ -119,6 +117,14 @@ NetworkRecvStatus ClientNetworkTurnSocke @@
 	return NETWORK_RECV_STATUS_OKAY;
+}
 ClientNetworkTurnSocketHandler::~ClientNetworkTurnSocketHandler()
+{
 	if (this->connecter != nullptr) {
 		this->connecter->Kill();
 		this->connecter = nullptr;
+	}
+}
 /**
  * Check whether we received/can send some data from/to the TURN server and
  * when that's the case handle it appropriately

src/network/network_turn.h

➞

Show inline comments

@@ @@ -30,6 +30,7 @@ public: @@
 	ClientNetworkTurnSocketHandler(const std::string &token, uint8 tracking_number, const std::string &connection_string) : token(token), tracking_number(tracking_number), connection_string(connection_string) {}
 	NetworkRecvStatus CloseConnection(bool error = true) override;
 	~ClientNetworkTurnSocketHandler() override;
 	void SendReceive();
 	void Connect();

0 comments (0 inline, 0 general)