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

@@ @@ -97,23 +97,29 @@ std::unique_ptr<ClientNetworkStunSocketH @@
+}
 NetworkRecvStatus ClientNetworkStunSocketHandler::CloseConnection(bool error)
+{
 	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;
+	}
 	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.
  */
 void ClientNetworkStunSocketHandler::SendReceive()
+{

src/network/network_stun.h

➞

Show inline comments

@@ @@ -21,12 +21,13 @@ private: @@
 public:
 	TCPConnecter *connecter = nullptr; ///< Connecter instance.
 	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);
 	static std::unique_ptr<ClientNetworkStunSocketHandler> Stun(const std::string &token, uint8 family);
 };

src/network/network_turn.cpp

➞

Show inline comments

@@ @@ -105,23 +105,29 @@ void ClientNetworkTurnSocketHandler::Con @@
+}
 NetworkRecvStatus ClientNetworkTurnSocketHandler::CloseConnection(bool error)
+{
 	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;
+	}
 	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
  */
 void ClientNetworkTurnSocketHandler::SendReceive()
+{

src/network/network_turn.h

➞

Show inline comments

@@ @@ -27,12 +27,13 @@ public: @@
 	TCPConnecter *connecter = nullptr; ///< Connecter instance.
 	bool connect_started = false;      ///< Whether we started the connection.
 	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();
 	void ConnectFailure();
 	static std::unique_ptr<ClientNetworkTurnSocketHandler> Turn(const std::string &token, uint8 tracking_number, const std::string &ticket, const std::string &connection_string);

0 comments (0 inline, 0 general)