From 49535506a9c8002baf27da3de629326e57982f44 Mon Sep 17 00:00:00 2001
From: Tobias Markmann <tm@ayena.de>
Date: Fri, 19 Sep 2014 22:34:31 +0200
Subject: Fix ASAN-reported heap-use-after-free bug.

ChatListModel used to notify Qt about its changed items via layoutChanged() signals.
However, not only the layout changed but most times also the items and corrosponding
QModelIndex objects and their internal pointers. After layout changed Qt tried to
access now invalid QModelIndex objects and their internal pointers pointed to
freed memory.

Test-Information:
Checked via ASAN. The previously perfect reproducable report by joining a MUC
from the bookmarks went away.

Change-Id: I71d2aa7e66a6b4caf2a9e0f68552ff5174291e1e
License: This patch is BSD-licensed, see Documentation/Licenses/BSD-simplified.txt for details.

diff --git a/Swift/QtUI/ChatList/ChatListModel.cpp b/Swift/QtUI/ChatList/ChatListModel.cpp
index d09b0dd..f08478f 100644
--- a/Swift/QtUI/ChatList/ChatListModel.cpp
+++ b/Swift/QtUI/ChatList/ChatListModel.cpp
@@ -27,6 +27,18 @@ ChatListModel::ChatListModel() : whiteboards_(NULL) {
 
 	root_->addItem(recents_);
 	root_->addItem(mucBookmarks_);
+
+	QModelIndex idx = index(0, 0, QModelIndex());
+	while (idx.isValid()) {
+		if (idx.internalPointer() == mucBookmarks_) {
+			mucBookmarksIndex_ = idx;
+		} else if (idx.internalPointer() == recents_) {
+			recentsIndex_ = idx;
+		} else if (idx.internalPointer() == whiteboards_) {
+			whiteboardsIndex_ = idx;
+		}
+		idx = index(idx.row() + 1, 0, QModelIndex());
+	}
 }
 
 Qt::ItemFlags ChatListModel::flags(const QModelIndex& index) const {
@@ -38,58 +50,57 @@ Qt::ItemFlags ChatListModel::flags(const QModelIndex& index) const {
 }
 
 void ChatListModel::clearBookmarks() {
-	emit layoutAboutToBeChanged();
+	beginRemoveRows(mucBookmarksIndex_, 0, mucBookmarks_->rowCount());
 	mucBookmarks_->clear();
-	emit layoutChanged();
+	endRemoveRows();
 }
 
 void ChatListModel::addMUCBookmark(const Swift::MUCBookmark& bookmark) {
-	emit layoutAboutToBeChanged();
+	beginInsertRows(mucBookmarksIndex_, 0, mucBookmarks_->rowCount());
 	mucBookmarks_->addItem(new ChatListMUCItem(bookmark, mucBookmarks_));
-	emit layoutChanged();
-	//QModelIndex index = createIndex(mucBookmarks_->rowCount() - 1, 0, mucBookmarks_);
-	//emit dataChanged(index, index);
-	//emit dataChanged(parent(index), parent(index));
+	endInsertRows();
 }
 
 void ChatListModel::removeMUCBookmark(const Swift::MUCBookmark& bookmark) {
 	for (int i = 0; i < mucBookmarks_->rowCount(); i++) {
 		ChatListMUCItem* item = dynamic_cast<ChatListMUCItem*>(mucBookmarks_->item(i));
 		if (item->getBookmark() == bookmark) {
-			emit layoutAboutToBeChanged();
+			beginRemoveRows(mucBookmarksIndex_, i, i+1);
 			mucBookmarks_->remove(i);
-			emit layoutChanged();
+			endRemoveRows();
 			break;
 		}
 	}
 }
 
 void ChatListModel::addWhiteboardSession(const ChatListWindow::Chat& chat) {
-	emit layoutAboutToBeChanged();
+	beginInsertRows(whiteboardsIndex_, 0, whiteboards_->rowCount());
 	whiteboards_->addItem(new ChatListWhiteboardItem(chat, whiteboards_));
-	emit layoutChanged();
+	endInsertRows();
 }
 
 void ChatListModel::removeWhiteboardSession(const JID& jid) {
 	for (int i = 0; i < whiteboards_->rowCount(); i++) {
 		ChatListWhiteboardItem* item = dynamic_cast<ChatListWhiteboardItem*>(whiteboards_->item(i));
 		if (item->getChat().jid == jid) {
-			emit layoutAboutToBeChanged();
+			beginRemoveRows(whiteboardsIndex_, i, i+1);
 			whiteboards_->remove(i);
-			emit layoutChanged();
+			endRemoveRows();
 			break;
 		}
 	}
 }
 
 void ChatListModel::setRecents(const std::list<ChatListWindow::Chat>& recents) {
-	emit layoutAboutToBeChanged();
+	beginRemoveRows(recentsIndex_, 0, recents_->rowCount());
 	recents_->clear();
+	endRemoveRows();
+	beginInsertRows(recentsIndex_, 0, recents.size());
 	foreach (const ChatListWindow::Chat chat, recents) {
 		recents_->addItem(new ChatListRecentItem(chat, recents_));
 //whiteboards_->addItem(new ChatListRecentItem(chat, whiteboards_));
 	}
-	emit layoutChanged();
+	endInsertRows();
 }
 
 QMimeData* ChatListModel::mimeData(const QModelIndexList& indexes) const {
diff --git a/Swift/QtUI/ChatList/ChatListModel.h b/Swift/QtUI/ChatList/ChatListModel.h
index a15cbcd..00bd5eb 100644
--- a/Swift/QtUI/ChatList/ChatListModel.h
+++ b/Swift/QtUI/ChatList/ChatListModel.h
@@ -7,7 +7,7 @@
 #pragma once
 
 #include <QAbstractItemModel>
-#include <QList>
+#include <QPersistentModelIndex>
 
 #include <Swiften/MUC/MUCBookmark.h>
 #include <Swift/Controllers/UIInterfaces/ChatListWindow.h>
@@ -38,6 +38,10 @@ namespace Swift {
 			ChatListGroupItem* recents_;
 			ChatListGroupItem* whiteboards_;
 			ChatListGroupItem* root_;
+
+			QPersistentModelIndex mucBookmarksIndex_;
+			QPersistentModelIndex recentsIndex_;
+			QPersistentModelIndex whiteboardsIndex_;
 	};
 
 }
-- 
cgit v0.10.2-6-g49f6