From e0ab9aa898fdd813c73ea3c47ffb0f9b5f80e9cf Mon Sep 17 00:00:00 2001
From: xiphon <xiphon@protonmail.com>
Date: Tue, 17 Dec 2019 14:18:54 +0000
Subject: [PATCH] SubaddressAccount: fix use-after-free bug

---
 src/libwalletqt/SubaddressAccount.cpp | 26 +++++++++++++------
 src/libwalletqt/SubaddressAccount.h   |  6 +++--
 src/model/SubaddressAccountModel.cpp  | 36 ++++++++++++++++-----------
 3 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/src/libwalletqt/SubaddressAccount.cpp b/src/libwalletqt/SubaddressAccount.cpp
index 1d83fcbd..940413a7 100644
--- a/src/libwalletqt/SubaddressAccount.cpp
+++ b/src/libwalletqt/SubaddressAccount.cpp
@@ -36,24 +36,34 @@ SubaddressAccount::SubaddressAccount(Monero::SubaddressAccount *subaddressAccoun
     getAll();
 }
 
-QList<Monero::SubaddressAccountRow *> SubaddressAccount::getAll() const
+void SubaddressAccount::getAll() const
 {
     qDebug(__FUNCTION__);
 
     emit refreshStarted();
 
-    m_rows.clear();
-    for (auto &row: m_subaddressAccountImpl->getAll()) {
-        m_rows.append(row);
+    {
+        QWriteLocker locker(&m_lock);
+        m_rows.clear();
+        for (auto &row: m_subaddressAccountImpl->getAll()) {
+            m_rows.append(row);
+        }
     }
 
     emit refreshFinished();
-    return m_rows;
 }
 
-Monero::SubaddressAccountRow * SubaddressAccount::getRow(int index) const
+bool SubaddressAccount::getRow(int index, std::function<void (Monero::SubaddressAccountRow &)> callback) const
 {
-    return m_rows.at(index);
+    QReadLocker locker(&m_lock);
+
+    if (index < 0 || index >= m_rows.size())
+    {
+        return false;
+    }
+
+    callback(*m_rows.value(index));
+    return true;
 }
 
 void SubaddressAccount::addRow(const QString &label) const
@@ -76,5 +86,7 @@ void SubaddressAccount::refresh() const
 
 quint64 SubaddressAccount::count() const
 {
+    QReadLocker locker(&m_lock);
+
     return m_rows.size();
 }
diff --git a/src/libwalletqt/SubaddressAccount.h b/src/libwalletqt/SubaddressAccount.h
index b7979156..2cdaf317 100644
--- a/src/libwalletqt/SubaddressAccount.h
+++ b/src/libwalletqt/SubaddressAccount.h
@@ -31,6 +31,7 @@
 
 #include <wallet/api/wallet2_api.h>
 #include <QObject>
+#include <QReadWriteLock>
 #include <QList>
 #include <QDateTime>
 
@@ -38,8 +39,8 @@ class SubaddressAccount : public QObject
 {
     Q_OBJECT
 public:
-    Q_INVOKABLE QList<Monero::SubaddressAccountRow *> getAll() const;
-    Q_INVOKABLE Monero::SubaddressAccountRow * getRow(int index) const;
+    Q_INVOKABLE void getAll() const;
+    Q_INVOKABLE bool getRow(int index, std::function<void (Monero::SubaddressAccountRow &)> callback) const;
     Q_INVOKABLE void addRow(const QString &label) const;
     Q_INVOKABLE void setLabel(quint32 accountIndex, const QString &label) const;
     Q_INVOKABLE void refresh() const;
@@ -54,6 +55,7 @@ public slots:
 private:
     explicit SubaddressAccount(Monero::SubaddressAccount * subaddressAccountImpl, QObject *parent);
     friend class Wallet;
+    mutable QReadWriteLock m_lock;
     Monero::SubaddressAccount * m_subaddressAccountImpl;
     mutable QList<Monero::SubaddressAccountRow*> m_rows;
 };
diff --git a/src/model/SubaddressAccountModel.cpp b/src/model/SubaddressAccountModel.cpp
index b8258296..c84b4532 100644
--- a/src/model/SubaddressAccountModel.cpp
+++ b/src/model/SubaddressAccountModel.cpp
@@ -59,22 +59,28 @@ QVariant SubaddressAccountModel::data(const QModelIndex &index, int role) const
     if (!index.isValid() || index.row() < 0 || (unsigned)index.row() >= m_subaddressAccount->count())
         return {};
 
-    Monero::SubaddressAccountRow * sr = m_subaddressAccount->getRow(index.row());
+    QVariant result;
 
-    QVariant result = "";
-    switch (role) {
-    case SubaddressAccountAddressRole:
-        result = QString::fromStdString(sr->getAddress());
-        break;
-    case SubaddressAccountLabelRole:
-        result = QString::fromStdString(sr->getLabel());
-        break;
-    case SubaddressAccountBalanceRole:
-        result = QString::fromStdString(sr->getBalance());
-        break;
-    case SubaddressAccountUnlockedBalanceRole:
-        result = QString::fromStdString(sr->getUnlockedBalance());
-        break;
+    bool found = m_subaddressAccount->getRow(index.row(), [&result, &role](const Monero::SubaddressAccountRow &row) {
+        switch (role) {
+        case SubaddressAccountAddressRole:
+            result = QString::fromStdString(row.getAddress());
+            break;
+        case SubaddressAccountLabelRole:
+            result = QString::fromStdString(row.getLabel());
+            break;
+        case SubaddressAccountBalanceRole:
+            result = QString::fromStdString(row.getBalance());
+            break;
+        case SubaddressAccountUnlockedBalanceRole:
+            result = QString::fromStdString(row.getUnlockedBalance());
+            break;
+        default:
+            qCritical() << "Unimplemented role" << role;
+        }
+    });
+    if (!found) {
+        qCritical("%s: internal error: invalid index %d", __FUNCTION__, index.row());
     }
 
     return result;