From db0d4d85a600eddec46f1ac3d9b98f133e187ee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=A4rkl?= Date: Sat, 9 Jul 2022 16:26:35 +0200 Subject: [PATCH] Use CutterJson for Signatures and rewrite JsonModel (#2997) This fixes a double-free in getSignatureInfo() when the string was already freed by the intermediate CutterJson. But actually just using this CutterJson makes more sense than the string anyway, so let's do that. The old JsonModel rebuilt the entire structure, defeating the main purpose of a custom model in comparison to the existing QTreeWidget. And since we are holding the json structures ourselves anyway, such a custom model does not have many benefits anyway. --- src/CMakeLists.txt | 2 - src/common/JsonModel.cpp | 149 +++++++-------------------------- src/common/JsonModel.h | 32 +------ src/common/JsonTreeItem.cpp | 104 ----------------------- src/common/JsonTreeItem.h | 39 --------- src/core/Cutter.cpp | 8 +- src/core/Cutter.h | 2 +- src/core/CutterJson.h | 1 - src/widgets/CutterTreeView.cpp | 9 +- src/widgets/CutterTreeView.h | 2 + src/widgets/Dashboard.cpp | 48 +++++------ 11 files changed, 67 insertions(+), 329 deletions(-) delete mode 100644 src/common/JsonTreeItem.cpp delete mode 100644 src/common/JsonTreeItem.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 47357c69..ba6691c9 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -72,7 +72,6 @@ set(SOURCES widgets/CutterTreeWidget.cpp widgets/GraphWidget.cpp widgets/OverviewWidget.cpp - common/JsonTreeItem.cpp common/JsonModel.cpp dialogs/VersionInfoDialog.cpp widgets/FlirtWidget.cpp @@ -222,7 +221,6 @@ set(HEADER_FILES widgets/CutterTreeWidget.h widgets/GraphWidget.h widgets/OverviewWidget.h - common/JsonTreeItem.h common/JsonModel.h dialogs/VersionInfoDialog.h widgets/FlirtWidget.h diff --git a/src/common/JsonModel.cpp b/src/common/JsonModel.cpp index 4b1ef628..4bc01cb4 100644 --- a/src/common/JsonModel.cpp +++ b/src/common/JsonModel.cpp @@ -1,123 +1,38 @@ #include "JsonModel.h" -#include - -JsonModel::JsonModel(QObject *parent) : QAbstractItemModel(parent) +QTreeWidgetItem *Cutter::jsonTreeWidgetItem(const QString &key, const CutterJson &json) { - mRootItem = new JsonTreeItem; - mHeaders.append("key"); - mHeaders.append("value"); -} - -JsonModel::~JsonModel() -{ - delete mRootItem; -} - -bool JsonModel::load(QIODevice *device) -{ - return loadJson(device->readAll()); -} - -bool JsonModel::loadJson(const QByteArray &json) -{ - mDocument = QJsonDocument::fromJson(json); - - if (!mDocument.isNull()) { - beginResetModel(); - delete mRootItem; - if (mDocument.isArray()) { - mRootItem = JsonTreeItem::load(QJsonValue(mDocument.array())); - } else { - mRootItem = JsonTreeItem::load(QJsonValue(mDocument.object())); + QString val; + switch (json.type()) { + case RZ_JSON_STRING: + val = json.toString(); + break; + case RZ_JSON_BOOLEAN: + val = json.toBool() ? "true" : "false"; + break; + case RZ_JSON_DOUBLE: + val = QString::number(json.lowLevelValue()->num.dbl_value); + break; + case RZ_JSON_INTEGER: + val = QString::number(json.toUt64()); + break; + case RZ_JSON_NULL: + val = "null"; + break; + case RZ_JSON_OBJECT: + case RZ_JSON_ARRAY: + break; + } + auto r = new QTreeWidgetItem(QStringList({ key, val })); + if (json.type() == RZ_JSON_ARRAY) { + size_t i = 0; + for (const auto &child : json) { + r->addChild(jsonTreeWidgetItem(QString::number(i++), child)); + } + } else if (json.type() == RZ_JSON_OBJECT) { + for (const auto &child : json) { + r->addChild(jsonTreeWidgetItem(child.key(), child)); } - endResetModel(); - return true; } - return false; -} - -QVariant JsonModel::data(const QModelIndex &index, int role) const -{ - - if (!index.isValid()) - return QVariant(); - - JsonTreeItem *item = static_cast(index.internalPointer()); - - if (role == Qt::DisplayRole) { - - if (index.column() == 0) - return QString("%1").arg(item->key()); - - if (index.column() == 1) - return QString("%1").arg(item->value()); - } - - return QVariant(); -} - -QVariant JsonModel::headerData(int section, Qt::Orientation orientation, int role) const -{ - if (role != Qt::DisplayRole) - return QVariant(); - - if (orientation == Qt::Horizontal) { - - return mHeaders.value(section); - } else - return QVariant(); -} - -QModelIndex JsonModel::index(int row, int column, const QModelIndex &parent) const -{ - if (!hasIndex(row, column, parent)) - return QModelIndex(); - - JsonTreeItem *parentItem; - - if (!parent.isValid()) - parentItem = mRootItem; - else - parentItem = static_cast(parent.internalPointer()); - - JsonTreeItem *childItem = parentItem->child(row); - if (childItem) - return createIndex(row, column, childItem); - else - return QModelIndex(); -} - -QModelIndex JsonModel::parent(const QModelIndex &index) const -{ - if (!index.isValid()) - return QModelIndex(); - - JsonTreeItem *childItem = static_cast(index.internalPointer()); - JsonTreeItem *parentItem = childItem->parent(); - - if (parentItem == mRootItem) - return QModelIndex(); - - return createIndex(parentItem->row(), 0, parentItem); -} - -int JsonModel::rowCount(const QModelIndex &parent) const -{ - JsonTreeItem *parentItem; - if (parent.column() > 0) - return 0; - - if (!parent.isValid()) - parentItem = mRootItem; - else - parentItem = static_cast(parent.internalPointer()); - - return parentItem->childCount(); -} - -int JsonModel::columnCount(const QModelIndex &parent) const -{ - Q_UNUSED(parent) - return 2; + return r; } diff --git a/src/common/JsonModel.h b/src/common/JsonModel.h index 4b5c7cdd..5af02ab6 100644 --- a/src/common/JsonModel.h +++ b/src/common/JsonModel.h @@ -2,37 +2,13 @@ #ifndef JSONMODEL_H #define JSONMODEL_H -#include -#include -#include -#include -#include -#include +#include +#include "CutterJson.h" -#include "JsonTreeItem.h" +namespace Cutter { -class JsonTreeItem; +QTreeWidgetItem *jsonTreeWidgetItem(const QString &key, const CutterJson &json); -class JsonModel : public QAbstractItemModel -{ - -public: - explicit JsonModel(QObject *parent = nullptr); - bool load(QIODevice *device); - bool loadJson(const QByteArray &json); - QVariant data(const QModelIndex &index, int role) const Q_DECL_OVERRIDE; - QVariant headerData(int section, Qt::Orientation orientation, int role) const Q_DECL_OVERRIDE; - QModelIndex index(int row, int column, - const QModelIndex &parent = QModelIndex()) const Q_DECL_OVERRIDE; - QModelIndex parent(const QModelIndex &index) const Q_DECL_OVERRIDE; - int rowCount(const QModelIndex &parent = QModelIndex()) const Q_DECL_OVERRIDE; - int columnCount(const QModelIndex &parent = QModelIndex()) const Q_DECL_OVERRIDE; - ~JsonModel(); - -private: - JsonTreeItem *mRootItem; - QJsonDocument mDocument; - QStringList mHeaders; }; #endif // JSONMODEL_H diff --git a/src/common/JsonTreeItem.cpp b/src/common/JsonTreeItem.cpp deleted file mode 100644 index 3a16dbd7..00000000 --- a/src/common/JsonTreeItem.cpp +++ /dev/null @@ -1,104 +0,0 @@ -#include "JsonTreeItem.h" - -JsonTreeItem::JsonTreeItem(JsonTreeItem *parent) -{ - mParent = parent; -} - -JsonTreeItem::~JsonTreeItem() -{ - qDeleteAll(mChilds); -} - -void JsonTreeItem::appendChild(JsonTreeItem *item) -{ - mChilds.append(item); -} - -JsonTreeItem *JsonTreeItem::child(int row) -{ - return mChilds.value(row); -} - -JsonTreeItem *JsonTreeItem::parent() -{ - return mParent; -} - -int JsonTreeItem::childCount() const -{ - return mChilds.count(); -} - -int JsonTreeItem::row() const -{ - if (mParent) - return mParent->mChilds.indexOf(const_cast(this)); - - return 0; -} - -void JsonTreeItem::setKey(const QString &key) -{ - mKey = key; -} - -void JsonTreeItem::setValue(const QString &value) -{ - mValue = value; -} - -void JsonTreeItem::setType(const QJsonValue::Type &type) -{ - mType = type; -} - -QString JsonTreeItem::key() const -{ - return mKey; -} - -QString JsonTreeItem::value() const -{ - return mValue; -} - -QJsonValue::Type JsonTreeItem::type() const -{ - return mType; -} - -JsonTreeItem *JsonTreeItem::load(const QJsonValue &value, JsonTreeItem *parent) -{ - JsonTreeItem *rootItem = new JsonTreeItem(parent); - rootItem->setKey("root"); - - if (value.isObject()) { - - // Get all QJsonValue childs - for (const QString &key : value.toObject().keys()) { - QJsonValue v = value.toObject().value(key); - JsonTreeItem *child = load(v, rootItem); - child->setKey(key); - child->setType(v.type()); - rootItem->appendChild(child); - } - - } else if (value.isArray()) { - // Get all QJsonValue childs - int index = 0; - for (const QJsonValue &v : value.toArray()) { - - JsonTreeItem *child = load(v, rootItem); - child->setKey(QString::number(index)); - child->setType(v.type()); - rootItem->appendChild(child); - ++index; - } - } else { - rootItem->setValue(value.toVariant().toString()); - rootItem->setType(value.type()); - } - - return rootItem; -} diff --git a/src/common/JsonTreeItem.h b/src/common/JsonTreeItem.h deleted file mode 100644 index 780a3799..00000000 --- a/src/common/JsonTreeItem.h +++ /dev/null @@ -1,39 +0,0 @@ -#ifndef JSONTREEITEM_H -#define JSONTREEITEM_H - -#include -#include -#include -#include -#include -#include - -#include "JsonModel.h" - -class JsonTreeItem -{ -public: - JsonTreeItem(JsonTreeItem *parent = nullptr); - ~JsonTreeItem(); - void appendChild(JsonTreeItem *item); - JsonTreeItem *child(int row); - JsonTreeItem *parent(); - int childCount() const; - int row() const; - void setKey(const QString &key); - void setValue(const QString &value); - void setType(const QJsonValue::Type &type); - QString key() const; - QString value() const; - QJsonValue::Type type() const; - static JsonTreeItem *load(const QJsonValue &value, JsonTreeItem *parent = nullptr); - -private: - QString mKey; - QString mValue; - QJsonValue::Type mType; - QList mChilds; - JsonTreeItem *mParent; -}; - -#endif // JSONTREEITEM_H diff --git a/src/core/Cutter.cpp b/src/core/Cutter.cpp index e2d85297..a8a8c038 100644 --- a/src/core/Cutter.cpp +++ b/src/core/Cutter.cpp @@ -1440,7 +1440,7 @@ bool CutterCore::registerDecompiler(Decompiler *decompiler) return true; } -QString CutterCore::getSignatureInfo() +CutterJson CutterCore::getSignatureInfo() { CORE_LOCK(); RzBinFile *cur = rz_bin_cur(core->bin); @@ -1452,11 +1452,7 @@ QString CutterCore::getSignatureInfo() if (!signature) { return {}; } - auto sig = parseJson(signature, nullptr); - if (sig.size() == 0) { - return {}; - } - return fromOwnedCharPtr(signature); + return parseJson(signature, nullptr); } bool CutterCore::existsFileInfo() diff --git a/src/core/Cutter.h b/src/core/Cutter.h index 9aa90b18..bdddd342 100644 --- a/src/core/Cutter.h +++ b/src/core/Cutter.h @@ -569,7 +569,7 @@ public: bool registerDecompiler(Decompiler *decompiler); RVA getOffsetJump(RVA addr); - QString getSignatureInfo(); + CutterJson getSignatureInfo(); bool existsFileInfo(); void setGraphEmpty(bool empty); bool isGraphEmpty(); diff --git a/src/core/CutterJson.h b/src/core/CutterJson.h index e8dd9819..e733ca6c 100644 --- a/src/core/CutterJson.h +++ b/src/core/CutterJson.h @@ -70,7 +70,6 @@ public: iterator end() const { return iterator(nullptr, nullptr); } bool toBool() const { return value && value->type == RZ_JSON_BOOLEAN && value->num.u_value; } - QString toJson() const { return rz_json_as_string(value); } st64 toSt64() const { return value && value->type == RZ_JSON_INTEGER ? value->num.s_value : 0; } ut64 toUt64() const { return value && value->type == RZ_JSON_INTEGER ? value->num.u_value : 0; } diff --git a/src/widgets/CutterTreeView.cpp b/src/widgets/CutterTreeView.cpp index f26fc08c..233cdd80 100644 --- a/src/widgets/CutterTreeView.cpp +++ b/src/widgets/CutterTreeView.cpp @@ -4,8 +4,13 @@ CutterTreeView::CutterTreeView(QWidget *parent) : QTreeView(parent), ui(new Ui::CutterTreeView()) { ui->setupUi(this); - this->setSelectionMode(QAbstractItemView::ExtendedSelection); - this->setUniformRowHeights(true); + applyCutterStyle(this); } CutterTreeView::~CutterTreeView() {} + +void CutterTreeView::applyCutterStyle(QTreeView *view) +{ + view->setSelectionMode(QAbstractItemView::ExtendedSelection); + view->setUniformRowHeights(true); +} diff --git a/src/widgets/CutterTreeView.h b/src/widgets/CutterTreeView.h index 46dfcccd..33600bc3 100644 --- a/src/widgets/CutterTreeView.h +++ b/src/widgets/CutterTreeView.h @@ -19,6 +19,8 @@ public: explicit CutterTreeView(QWidget *parent = nullptr); ~CutterTreeView(); + static void applyCutterStyle(QTreeView *view); + private: std::unique_ptr ui; }; diff --git a/src/widgets/Dashboard.cpp b/src/widgets/Dashboard.cpp index 3715bc36..f73b4cc1 100644 --- a/src/widgets/Dashboard.cpp +++ b/src/widgets/Dashboard.cpp @@ -2,7 +2,6 @@ #include "ui_Dashboard.h" #include "common/Helpers.h" #include "common/JsonModel.h" -#include "common/JsonTreeItem.h" #include "common/TempConfig.h" #include "dialogs/VersionInfoDialog.h" @@ -162,7 +161,7 @@ void Dashboard::updateContents() ui->verticalLayout_2->addSpacerItem(spacer); // Check if signature info and version info available - if (Core()->getSignatureInfo().isEmpty()) { + if (!Core()->getSignatureInfo().size()) { ui->certificateButton->setEnabled(false); } ui->versioninfoButton->setEnabled(Core()->existsFileInfo()); @@ -170,33 +169,24 @@ void Dashboard::updateContents() void Dashboard::on_certificateButton_clicked() { - static QDialog *viewDialog = nullptr; - static CutterTreeView *view = nullptr; - static JsonModel *model = nullptr; - static QString qstrCertificates; - if (!viewDialog) { - viewDialog = new QDialog(this); - view = new CutterTreeView(viewDialog); - model = new JsonModel(); - qstrCertificates = Core()->getSignatureInfo(); - } - if (!viewDialog->isVisible()) { - std::string strCertificates = qstrCertificates.toUtf8().constData(); - model->loadJson(QByteArray::fromStdString(strCertificates)); - view->setModel(model); - view->expandAll(); - view->resize(900, 600); - QSizePolicy sizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed); - sizePolicy.setHorizontalStretch(0); - sizePolicy.setVerticalStretch(0); - sizePolicy.setHeightForWidth(view->sizePolicy().hasHeightForWidth()); - viewDialog->setSizePolicy(sizePolicy); - viewDialog->setMinimumSize(QSize(900, 600)); - viewDialog->setMaximumSize(QSize(900, 600)); - viewDialog->setSizeGripEnabled(false); - viewDialog->setWindowTitle("Certificates"); - viewDialog->show(); - } + QDialog dialog(this); + auto view = new QTreeWidget(&dialog); + view->setHeaderLabels({ tr("Key"), tr("Value") }); + view->addTopLevelItem(Cutter::jsonTreeWidgetItem(QString("<%1>").arg(tr("root")), + Core()->getSignatureInfo())); + CutterTreeView::applyCutterStyle(view); + view->expandAll(); + view->resize(900, 600); + QSizePolicy sizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed); + sizePolicy.setHorizontalStretch(0); + sizePolicy.setVerticalStretch(0); + sizePolicy.setHeightForWidth(view->sizePolicy().hasHeightForWidth()); + dialog.setSizePolicy(sizePolicy); + dialog.setMinimumSize(QSize(900, 600)); + dialog.setMaximumSize(QSize(900, 600)); + dialog.setSizeGripEnabled(false); + dialog.setWindowTitle("Certificates"); + dialog.exec(); } void Dashboard::on_versioninfoButton_clicked()