Fix: Crash due to use of invalid iterator in ClientNetworkContentSocketHandler

In particular this crash can be observed when using the
bootstrap GUI to download the base graphics.

In ClientNetworkContentSocketHandler::OnReceiveContentInfo
ClientNetworkContentSocketHandler::callbacks is iterated, using an iterator
cb->OnReceiveContentInfo() is called (cb is of type BootstrapAskForDownloadWindow)
This calls new BootstrapContentDownloadStatusWindow()
This inherits from BaseNetworkContentDownloadStatusWindow
The constructor of which calls _network_content_client.AddCallback(this)
This reallocates the std::vector which is being iterated in ClientNetworkContentSocketHandler::OnReceiveContentInfo
This results in iter being invalid, and an assertion failure occurs shortly
afterwards due to its use in the next iteration of cb->OnReceiveContentInfo()

Adjust all locations where ClientNetworkContentSocketHandler::callbacks
is iterated to avoid problematic behaviour

1 files changed, 18 insertions, 15 deletions

diff --git a/src/network/network_content.cpp b/src/network/network_content.cpp
index 19235c8c7..168ecd429 100644
--- a/src/network/network_content.cpp
+++ b/src/network/network_content.cpp
@@ -1029,37 +1029,39 @@ void ClientNetworkContentSocketHandler::Clear()
 
 void ClientNetworkContentSocketHandler::OnConnect(bool success)
 {
-	for (auto iter = this->callbacks.begin(); iter != this->callbacks.end(); /* nothing */) {
-		ContentCallback *cb = *iter;
+	for (size_t i = 0; i < this->callbacks.size(); /* nothing */) {
+		ContentCallback *cb = this->callbacks[i];
+		/* the callback may remove itself from this->callbacks */
 		cb->OnConnect(success);
-		if (iter != this->callbacks.end() && *iter == cb) iter++;
+		if (i != this->callbacks.size() && this->callbacks[i] == cb) i++;
 	}
 }
 
 void ClientNetworkContentSocketHandler::OnDisconnect()
 {
-	for (auto iter = this->callbacks.begin(); iter != this->callbacks.end(); /* nothing */) {
-		ContentCallback *cb = *iter;
+	for (size_t i = 0; i < this->callbacks.size(); /* nothing */) {
+		ContentCallback *cb = this->callbacks[i];
 		cb->OnDisconnect();
-		if (iter != this->callbacks.end() && *iter == cb) iter++;
+		if (i != this->callbacks.size() && this->callbacks[i] == cb) i++;
 	}
 }
 
 void ClientNetworkContentSocketHandler::OnReceiveContentInfo(const ContentInfo *ci)
 {
-	for (auto iter = this->callbacks.begin(); iter != this->callbacks.end(); /* nothing */) {
-		ContentCallback *cb = *iter;
+	for (size_t i = 0; i < this->callbacks.size(); /* nothing */) {
+		ContentCallback *cb = this->callbacks[i];
+		/* the callback may add items and/or remove itself from this->callbacks */
 		cb->OnReceiveContentInfo(ci);
-		if (iter != this->callbacks.end() && *iter == cb) iter++;
+		if (i != this->callbacks.size() && this->callbacks[i] == cb) i++;
 	}
 }
 
 void ClientNetworkContentSocketHandler::OnDownloadProgress(const ContentInfo *ci, int bytes)
 {
-	for (auto iter = this->callbacks.begin(); iter != this->callbacks.end(); /* nothing */) {
-		ContentCallback *cb = *iter;
+	for (size_t i = 0; i < this->callbacks.size(); /* nothing */) {
+		ContentCallback *cb = this->callbacks[i];
 		cb->OnDownloadProgress(ci, bytes);
-		if (iter != this->callbacks.end() && *iter == cb) iter++;
+		if (i != this->callbacks.size() && this->callbacks[i] == cb) i++;
 	}
 }
 
@@ -1070,9 +1072,10 @@ void ClientNetworkContentSocketHandler::OnDownloadComplete(ContentID cid)
 		ci->state = ContentInfo::ALREADY_HERE;
 	}
 
-	for (auto iter = this->callbacks.begin(); iter != this->callbacks.end(); /* nothing */) {
-		ContentCallback *cb = *iter;
+	for (size_t i = 0; i < this->callbacks.size(); /* nothing */) {
+		ContentCallback *cb = this->callbacks[i];
+		/* the callback may remove itself from this->callbacks */
 		cb->OnDownloadComplete(cid);
-		if (iter != this->callbacks.end() && *iter == cb) iter++;
+		if (i != this->callbacks.size() && this->callbacks[i] == cb) i++;
 	}
 }