Skip to content

Commit 0cfff31

Browse files
authored
[ntcore] Fix use-after-free on connection termination (#7177)
The stream can close (e.g. due to an error) while in the middle of writing. The close callback would immediately destroy the connection object, resulting in the writing code having a use-after-free. Fix this by deferring the deletion to the loop main using a single-shot timer.
1 parent a71cee1 commit 0cfff31

File tree

3 files changed

+6
-5
lines changed

3 files changed

+6
-5
lines changed

ntcore/src/main/native/cpp/NetworkServer.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,9 @@ void NetworkServer::ServerConnection::UpdateOutgoingTimer(uint32_t repeatMs) {
125125
void NetworkServer::ServerConnection::ConnectionClosed() {
126126
// don't call back into m_server if it's being destroyed
127127
if (!m_outgoingTimer->IsLoopClosing()) {
128-
m_server.m_serverImpl.RemoveClient(m_clientId);
128+
uv::Timer::SingleShot(m_outgoingTimer->GetLoopRef(), uv::Timer::Time{0},
129+
[client = m_server.m_serverImpl.RemoveClient(
130+
m_clientId)]() mutable { client.reset(); });
129131
m_server.RemoveConnection(this);
130132
}
131133
m_outgoingTimer->Close();

ntcore/src/main/native/cpp/net/ServerImpl.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,7 +1252,7 @@ int ServerImpl::AddClient3(std::string_view connInfo, bool local,
12521252
return index;
12531253
}
12541254

1255-
void ServerImpl::RemoveClient(int clientId) {
1255+
std::shared_ptr<void> ServerImpl::RemoveClient(int clientId) {
12561256
DEBUG3("RemoveClient({})", clientId);
12571257
auto& client = m_clients[clientId];
12581258

@@ -1288,8 +1288,7 @@ void ServerImpl::RemoveClient(int clientId) {
12881288
DeleteTopic(client->m_metaPub);
12891289
DeleteTopic(client->m_metaSub);
12901290

1291-
// delete the client
1292-
client.reset();
1291+
return std::move(client);
12931292
}
12941293

12951294
bool ServerImpl::PersistentChanged() {

ntcore/src/main/native/cpp/net/ServerImpl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class ServerImpl final {
8080
int AddClient3(std::string_view connInfo, bool local,
8181
net3::WireConnection3& wire, Connected3Func connected,
8282
SetPeriodicFunc setPeriodic);
83-
void RemoveClient(int clientId);
83+
std::shared_ptr<void> RemoveClient(int clientId);
8484

8585
void ConnectionsChanged(const std::vector<ConnectionInfo>& conns);
8686

0 commit comments

Comments
 (0)