Subaddress: fix use-after-free on accessing stale Wallet API data

This commit is contained in:
xiphon 2019-12-17 21:49:58 +00:00
parent 46227bdad0
commit 8c511722e0
3 changed files with 39 additions and 23 deletions

View file

@ -36,49 +36,58 @@ Subaddress::Subaddress(Monero::Subaddress *subaddressImpl, QObject *parent)
getAll(); getAll();
} }
QList<Monero::SubaddressRow*> Subaddress::getAll(bool update) const void Subaddress::getAll() const
{ {
qDebug(__FUNCTION__); qDebug(__FUNCTION__);
emit refreshStarted(); emit refreshStarted();
if(update) {
m_rows.clear(); QWriteLocker locker(&m_lock);
if (m_rows.empty()){ m_rows.clear();
for (auto &row: m_subaddressImpl->getAll()) { for (auto &row: m_subaddressImpl->getAll()) {
m_rows.append(row); m_rows.append(row);
} }
} }
emit refreshFinished(); emit refreshFinished();
return m_rows;
} }
Monero::SubaddressRow * Subaddress::getRow(int index) const bool Subaddress::getRow(int index, std::function<void (Monero::SubaddressRow &row)> 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 Subaddress::addRow(quint32 accountIndex, const QString &label) const void Subaddress::addRow(quint32 accountIndex, const QString &label) const
{ {
m_subaddressImpl->addRow(accountIndex, label.toStdString()); m_subaddressImpl->addRow(accountIndex, label.toStdString());
getAll(true); getAll();
} }
void Subaddress::setLabel(quint32 accountIndex, quint32 addressIndex, const QString &label) const void Subaddress::setLabel(quint32 accountIndex, quint32 addressIndex, const QString &label) const
{ {
m_subaddressImpl->setLabel(accountIndex, addressIndex, label.toStdString()); m_subaddressImpl->setLabel(accountIndex, addressIndex, label.toStdString());
getAll(true); getAll();
} }
void Subaddress::refresh(quint32 accountIndex) const void Subaddress::refresh(quint32 accountIndex) const
{ {
m_subaddressImpl->refresh(accountIndex); m_subaddressImpl->refresh(accountIndex);
getAll(true); getAll();
} }
quint64 Subaddress::count() const quint64 Subaddress::count() const
{ {
QReadLocker locker(&m_lock);
return m_rows.size(); return m_rows.size();
} }

View file

@ -30,6 +30,7 @@
#define SUBADDRESS_H #define SUBADDRESS_H
#include <wallet/api/wallet2_api.h> #include <wallet/api/wallet2_api.h>
#include <QReadWriteLock>
#include <QObject> #include <QObject>
#include <QList> #include <QList>
#include <QDateTime> #include <QDateTime>
@ -38,8 +39,8 @@ class Subaddress : public QObject
{ {
Q_OBJECT Q_OBJECT
public: public:
Q_INVOKABLE QList<Monero::SubaddressRow*> getAll(bool update = false) const; Q_INVOKABLE void getAll() const;
Q_INVOKABLE Monero::SubaddressRow * getRow(int index) const; Q_INVOKABLE bool getRow(int index, std::function<void (Monero::SubaddressRow &row)> callback) const;
Q_INVOKABLE void addRow(quint32 accountIndex, const QString &label) const; Q_INVOKABLE void addRow(quint32 accountIndex, const QString &label) const;
Q_INVOKABLE void setLabel(quint32 accountIndex, quint32 addressIndex, const QString &label) const; Q_INVOKABLE void setLabel(quint32 accountIndex, quint32 addressIndex, const QString &label) const;
Q_INVOKABLE void refresh(quint32 accountIndex) const; Q_INVOKABLE void refresh(quint32 accountIndex) const;
@ -54,6 +55,7 @@ public slots:
private: private:
explicit Subaddress(Monero::Subaddress * subaddressImpl, QObject *parent); explicit Subaddress(Monero::Subaddress * subaddressImpl, QObject *parent);
friend class Wallet; friend class Wallet;
mutable QReadWriteLock m_lock;
Monero::Subaddress * m_subaddressImpl; Monero::Subaddress * m_subaddressImpl;
mutable QList<Monero::SubaddressRow*> m_rows; mutable QList<Monero::SubaddressRow*> m_rows;
}; };

View file

@ -60,18 +60,23 @@ QVariant SubaddressModel::data(const QModelIndex &index, int role) const
if (!index.isValid() || index.row() < 0 || (unsigned)index.row() >= m_subaddress->count()) if (!index.isValid() || index.row() < 0 || (unsigned)index.row() >= m_subaddress->count())
return {}; return {};
Monero::SubaddressRow * sr = m_subaddress->getRow(index.row()); QVariant result;
if (!sr)
return {};
QVariant result = ""; bool found = m_subaddress->getRow(index.row(), [&index, &result, &role](const Monero::SubaddressRow &subaddress) {
switch (role) { switch (role) {
case SubaddressAddressRole: case SubaddressAddressRole:
result = QString::fromStdString(sr->getAddress()); result = QString::fromStdString(subaddress.getAddress());
break; break;
case SubaddressLabelRole: case SubaddressLabelRole:
result = index.row() == 0 ? tr("Primary address") : QString::fromStdString(sr->getLabel()); result = index.row() == 0 ? tr("Primary address") : QString::fromStdString(subaddress.getLabel());
break; break;
default:
qCritical() << "Unimplemented role" << role;
}
});
if (!found)
{
qCritical("%s: internal error: invalid index %d", __FUNCTION__, index.row());
} }
return result; return result;