TransactionHistory: fix use-after-free bugs

pull/2/head
xiphon 4 years ago
parent 46227bdad0
commit 4e1f7349c4

@ -36,16 +36,18 @@
#include <QWriteLocker>
TransactionInfo *TransactionHistory::transaction(int index)
bool TransactionHistory::transaction(int index, std::function<void (TransactionInfo &)> callback)
{
QReadLocker locker(&m_tinfoLock);
QReadLocker locker(&m_lock);
if (index < 0 || index >= m_tinfo.size()) {
qCritical("%s: no transaction info for index %d", __FUNCTION__, index);
qCritical("%s: there's %d transactions in backend", __FUNCTION__, m_pimpl->count());
return nullptr;
return false;
}
return m_tinfo.at(index);
callback(*m_tinfo.value(index));
return true;
}
//// XXX: not sure if this method really needed;
@ -54,7 +56,7 @@ TransactionInfo *TransactionHistory::transaction(int index)
// return nullptr;
//}
QList<TransactionInfo *> TransactionHistory::getAll(quint32 accountIndex) const
void TransactionHistory::refresh(quint32 accountIndex)
{
QDateTime firstDateTime = QDateTime(QDate(2014, 4, 18)); // the genesis block
QDateTime lastDateTime = QDateTime::currentDateTime().addDays(1); // tomorrow (guard against jitter and timezones)
@ -62,23 +64,24 @@ QList<TransactionInfo *> TransactionHistory::getAll(quint32 accountIndex) const
emit refreshStarted();
{
QWriteLocker locker(&m_tinfoLock);
QWriteLocker locker(&m_lock);
// XXX this invalidates previously saved history that might be used by model
qDeleteAll(m_tinfo);
m_tinfo.clear();
quint64 lastTxHeight = 0;
m_locked = false;
m_minutesToUnlock = 0;
TransactionHistory * parent = const_cast<TransactionHistory*>(this);
m_pimpl->refresh();
for (const auto i : m_pimpl->getAll()) {
TransactionInfo * ti = new TransactionInfo(i, parent);
if (ti->subaddrAccount() != accountIndex) {
delete ti;
if (i->subaddrAccount() != accountIndex) {
continue;
}
m_tinfo.append(ti);
m_tinfo.append(new TransactionInfo(i, this));
const TransactionInfo *ti = m_tinfo.back();
// looking for transactions timestamp scope
if (ti->timestamp() >= lastDateTime) {
lastDateTime = ti->timestamp();
@ -94,7 +97,6 @@ QList<TransactionInfo *> TransactionHistory::getAll(quint32 accountIndex) const
m_minutesToUnlock = (requiredConfirmations - ti->confirmations()) * 2;
m_locked = true;
}
}
}
@ -108,21 +110,11 @@ QList<TransactionInfo *> TransactionHistory::getAll(quint32 accountIndex) const
m_lastDateTime = lastDateTime;
emit lastDateTimeChanged();
}
return m_tinfo;
}
void TransactionHistory::refresh(quint32 accountIndex)
{
// rebuilding transaction list in wallet_api;
m_pimpl->refresh();
// copying list here and keep track on every item to avoid memleaks
getAll(accountIndex);
}
quint64 TransactionHistory::count() const
{
QReadLocker locker(&m_tinfoLock);
QReadLocker locker(&m_lock);
return m_tinfo.count();
}
@ -157,11 +149,6 @@ TransactionHistory::TransactionHistory(Monero::TransactionHistory *pimpl, QObjec
QString TransactionHistory::writeCSV(quint32 accountIndex, QString out)
{
QList<TransactionInfo *> history = this->getAll(accountIndex);
if(history.count() < 1){
return QString("");
}
// construct filename
qint64 now = QDateTime::currentDateTime().currentMSecsSinceEpoch();
QString fn = QString(QString("%1/monero-txs_%2.csv").arg(out, QString::number(now / 1000)));
@ -176,15 +163,21 @@ QString TransactionHistory::writeCSV(quint32 accountIndex, QString out)
QTextStream output(&data);
output << "blockHeight,epoch,date,direction,amount,atomicAmount,fee,txid,label,subaddrAccount,paymentId\n";
foreach(const TransactionInfo *info, history)
{
QReadLocker locker(&m_lock);
for (const auto &tx : m_pimpl->getAll()) {
if (tx->subaddrAccount() != accountIndex) {
continue;
}
TransactionInfo info(tx, this);
// collect column data
double amount = info->amount();
quint64 atomicAmount = info->atomicAmount();
quint32 subaddrAccount = info->subaddrAccount();
QString fee = info->fee();
double amount = info.amount();
quint64 atomicAmount = info.atomicAmount();
quint32 subaddrAccount = info.subaddrAccount();
QString fee = info.fee();
QString direction = QString("");
TransactionInfo::Direction _direction = info->direction();
TransactionInfo::Direction _direction = info.direction();
if(_direction == TransactionInfo::Direction_In)
{
direction = QString("in");
@ -195,14 +188,14 @@ QString TransactionHistory::writeCSV(quint32 accountIndex, QString out)
else {
continue; // skip TransactionInfo::Direction_Both
}
QString label = info->label();
QString label = info.label();
label.remove(QChar('"')); // reserved
quint64 blockHeight = info->blockHeight();
QDateTime timeStamp = info->timestamp();
QString date = info->date() + " " + info->time();
quint64 blockHeight = info.blockHeight();
QDateTime timeStamp = info.timestamp();
QString date = info.date() + " " + info.time();
uint epoch = timeStamp.toTime_t();
QString displayAmount = info->displayAmount();
QString paymentId = info->paymentId();
QString displayAmount = info.displayAmount();
QString paymentId = info.paymentId();
if(paymentId == "0000000000000000"){
paymentId = "";
}
@ -211,7 +204,7 @@ QString TransactionHistory::writeCSV(quint32 accountIndex, QString out)
QString line = QString("%1,%2,%3,%4,%5,%6,%7,%8,\"%9\",%10,%11\n")
.arg(QString::number(blockHeight), QString::number(epoch), date)
.arg(direction, QString::number(amount), QString::number(atomicAmount))
.arg(info->fee(), info->hash(), label, QString::number(subaddrAccount))
.arg(info.fee(), info.hash(), label, QString::number(subaddrAccount))
.arg(paymentId);
output << line;
}

@ -50,9 +50,8 @@ class TransactionHistory : public QObject
Q_PROPERTY(bool locked READ locked)
public:
Q_INVOKABLE TransactionInfo *transaction(int index);
Q_INVOKABLE bool transaction(int index, std::function<void (TransactionInfo &)> callback);
// Q_INVOKABLE TransactionInfo * transaction(const QString &id);
Q_INVOKABLE QList<TransactionInfo*> getAll(quint32 accountIndex) const;
Q_INVOKABLE void refresh(quint32 accountIndex);
Q_INVOKABLE QString writeCSV(quint32 accountIndex, QString out);
quint64 count() const;
@ -75,9 +74,9 @@ private:
private:
friend class Wallet;
mutable QReadWriteLock m_lock;
Monero::TransactionHistory * m_pimpl;
mutable QList<TransactionInfo*> m_tinfo;
mutable QReadWriteLock m_tinfoLock;
mutable QDateTime m_firstDateTime;
mutable QDateTime m_lastDateTime;
mutable int m_minutesToUnlock;

@ -100,8 +100,4 @@ private:
mutable QList<Transfer*> m_transfers;
};
// in order to wrap it to QVariant
Q_DECLARE_METATYPE(TransactionInfo*)
#endif // TRANSACTIONINFO_H

@ -59,106 +59,90 @@ TransactionHistory *TransactionHistoryModel::transactionHistory() const
return m_transactionHistory;
}
QVariant TransactionHistoryModel::data(const QModelIndex &index, int role) const
QVariant TransactionHistoryModel::parseTransactionInfo(const TransactionInfo &tInfo, int role) const
{
if (!m_transactionHistory) {
return QVariant();
}
if (index.row() < 0 || (unsigned)index.row() >= m_transactionHistory->count()) {
return QVariant();
}
TransactionInfo * tInfo = m_transactionHistory->transaction(index.row());
Q_ASSERT(tInfo);
if (!tInfo) {
qCritical("%s: internal error: no transaction info for index %d", __FUNCTION__, index.row());
return QVariant();
}
QVariant result;
switch (role) {
case TransactionRole:
result = QVariant::fromValue(tInfo);
break;
switch (role)
{
case TransactionDirectionRole:
result = QVariant::fromValue(tInfo->direction());
break;
return QVariant::fromValue(tInfo.direction());
case TransactionPendingRole:
result = tInfo->isPending();
break;
return tInfo.isPending();
case TransactionFailedRole:
result = tInfo->isFailed();
break;
return tInfo.isFailed();
case TransactionAmountRole:
result = tInfo->amount();
break;
return tInfo.amount();
case TransactionDisplayAmountRole:
result = tInfo->displayAmount();
break;
return tInfo.displayAmount();
case TransactionAtomicAmountRole:
result = tInfo->atomicAmount();
break;
return tInfo.atomicAmount();
case TransactionFeeRole:
result = tInfo->fee();
break;
return tInfo.fee();
case TransactionBlockHeightRole:
{
// Use NULL QVariant for transactions without height.
// Forces them to be displayed at top when sorted by blockHeight.
if (tInfo->blockHeight() != 0) {
result = tInfo->blockHeight();
if (tInfo.blockHeight() != 0)
{
return tInfo.blockHeight();
}
break;
return QVariant();
}
case TransactionSubaddrIndexRole:
{
QString str = QString{""};
bool first = true;
for (quint32 i : tInfo.subaddrIndex())
{
QString str = QString{""};
bool first = true;
for (quint32 i : tInfo->subaddrIndex()) {
if (!first)
str += QString{","};
first = false;
str += QString::number(i);
}
result = str;
if (!first)
str += QString{","};
first = false;
str += QString::number(i);
}
break;
return str;
}
case TransactionSubaddrAccountRole:
result = tInfo->subaddrAccount();
break;
return tInfo.subaddrAccount();
case TransactionLabelRole:
result = tInfo->subaddrIndex().size() == 1 && *tInfo->subaddrIndex().begin() == 0 ? tr("Primary address") : tInfo->label();
break;
return tInfo.subaddrIndex().size() == 1 && *tInfo.subaddrIndex().begin() == 0 ? tr("Primary address") : tInfo.label();
case TransactionConfirmationsRole:
result = tInfo->confirmations();
break;
return tInfo.confirmations();
case TransactionConfirmationsRequiredRole:
result = (tInfo->blockHeight() < tInfo->unlockTime()) ? tInfo->unlockTime() - tInfo->blockHeight() : 10;
break;
return (tInfo.blockHeight() < tInfo.unlockTime()) ? tInfo.unlockTime() - tInfo.blockHeight() : 10;
case TransactionHashRole:
result = tInfo->hash();
break;
return tInfo.hash();
case TransactionTimeStampRole:
result = tInfo->timestamp();
break;
return tInfo.timestamp();
case TransactionPaymentIdRole:
result = tInfo->paymentId();
break;
return tInfo.paymentId();
case TransactionIsOutRole:
result = tInfo->direction() == TransactionInfo::Direction_Out;
break;
return tInfo.direction() == TransactionInfo::Direction_Out;
case TransactionDateRole:
result = tInfo->date();
break;
return tInfo.date();
case TransactionTimeRole:
result = tInfo->time();
break;
return tInfo.time();
case TransactionDestinationsRole:
result = tInfo->destinations_formatted();
break;
return tInfo.destinations_formatted();
default:
{
qCritical() << "Unimplemented role" << role;
return QVariant();
}
}
}
QVariant TransactionHistoryModel::data(const QModelIndex &index, int role) const
{
if (!m_transactionHistory) {
return QVariant();
}
QVariant result;
bool found = m_transactionHistory->transaction(index.row(), [this, &result, &role](const TransactionInfo &tInfo) {
result = parseTransactionInfo(tInfo, role);
});
if (!found) {
qCritical("%s: internal error: no transaction info for index %d", __FUNCTION__, index.row());
}
return result;
}
@ -171,7 +155,6 @@ int TransactionHistoryModel::rowCount(const QModelIndex &parent) const
QHash<int, QByteArray> TransactionHistoryModel::roleNames() const
{
QHash<int, QByteArray> roleNames = QAbstractListModel::roleNames();
roleNames.insert(TransactionRole, "transaction");
roleNames.insert(TransactionDirectionRole, "direction");
roleNames.insert(TransactionPendingRole, "isPending");
roleNames.insert(TransactionFailedRole, "isFailed");

@ -45,8 +45,7 @@ class TransactionHistoryModel : public QAbstractListModel
public:
enum TransactionInfoRole {
TransactionRole = Qt::UserRole + 1, // for the TransactionInfo object;
TransactionDirectionRole,
TransactionDirectionRole = Qt::UserRole + 1,
TransactionPendingRole,
TransactionFailedRole,
TransactionAmountRole,
@ -97,6 +96,9 @@ public:
signals:
void transactionHistoryChanged();
private:
QVariant parseTransactionInfo(const TransactionInfo &tInfo, int role) const;
private:
TransactionHistory * m_transactionHistory;
};