From 575534e7cca310c6d6195ab16d482bf9dfba755e Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Mon, 1 Aug 2022 18:09:35 +0200 Subject: Disallow direct events construction from JSON Direct construction (using makeEvent() or explicitly constructing an event) from JSON may create an event that has a type conflicting with that stored in JSON. There's no such problem with loadEvent(), even though it's considerably slower. Driven by the fact that almost nowhere in the code direct construction is used on checked JSON (one test is the only valid case), this commit moves all JSON-loading constructors to the protected section, thereby disabling usage of makeEvent() in JSON-loading capacity, and switches such cases across the library to loadEvent(). --- lib/connection.cpp | 10 ++++++---- lib/events/callevents.h | 4 ++-- lib/events/event.h | 11 +++++++++-- lib/events/roomevent.cpp | 2 +- lib/events/roomevent.h | 8 ++++---- lib/events/stateevent.h | 15 +++++++++------ 6 files changed, 31 insertions(+), 19 deletions(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index 471dc20d..a33ace51 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -2242,10 +2242,12 @@ void Connection::saveOlmAccount() #ifdef Quotient_E2EE_ENABLED QJsonObject Connection::decryptNotification(const QJsonObject ¬ification) { - auto r = room(notification["room_id"].toString()); - auto event = makeEvent(notification["event"].toObject()); - const auto decrypted = r->decryptMessage(*event); - return decrypted ? decrypted->fullJson() : QJsonObject(); + if (auto r = room(notification["room_id"].toString())) + if (auto event = + loadEvent(notification["event"].toObject())) + if (const auto decrypted = r->decryptMessage(*event)) + return decrypted->fullJson(); + return QJsonObject(); } Database* Connection::database() const diff --git a/lib/events/callevents.h b/lib/events/callevents.h index 6d9feae4..752e331d 100644 --- a/lib/events/callevents.h +++ b/lib/events/callevents.h @@ -15,12 +15,12 @@ public: return mType.startsWith("m.call."); } - explicit CallEvent(const QJsonObject& json); - QUO_CONTENT_GETTER(QString, callId) QUO_CONTENT_GETTER(int, version) protected: + explicit CallEvent(const QJsonObject& json); + static QJsonObject basicJson(const QString& matrixType, const QString& callId, int version, QJsonObject contentJson = {}); diff --git a/lib/events/event.h b/lib/events/event.h index 6a7acf28..9d7c61a9 100644 --- a/lib/events/event.h +++ b/lib/events/event.h @@ -203,6 +203,9 @@ private: // === Event creation facilities === //! \brief Create an event of arbitrary type from its arguments +//! +//! This should not be used to load events from JSON - use loadEvent() for that. +//! \sa loadEvent template inline event_ptr_tt makeEvent(ArgTs&&... args) { @@ -266,8 +269,6 @@ public: return BaseMetaType; } - explicit Event(const QJsonObject& json); - Q_DISABLE_COPY(Event) Event(Event&&) noexcept = default; Event& operator=(Event&&) = delete; @@ -364,6 +365,10 @@ public: [[deprecated("Use is() instead")]] bool isCallEvent() const; protected: + friend class EventMetaType; // To access the below constructor + + explicit Event(const QJsonObject& json); + QJsonObject& editJson() { return _json; } virtual void dumpTo(QDebug dbg) const; @@ -427,6 +432,7 @@ public: //! pointing to that BaseMetaType. //! \sa EventMetaType, EventMetaType::SuppressLoadDerived #define QUO_BASE_EVENT(CppType_, ...) \ + friend class EventMetaType; \ static inline EventMetaType BaseMetaType{ \ #CppType_ __VA_OPT__(,) __VA_ARGS__ }; \ const AbstractEventMetaType& metaType() const override \ @@ -452,6 +458,7 @@ public: //! \sa EventMetaType #define QUO_EVENT(CppType_, MatrixType_, ...) \ static inline const auto& TypeId = MatrixType_##_ls; \ + friend class EventMetaType; \ static inline const EventMetaType MetaType{ \ #CppType_, TypeId, BaseMetaType __VA_OPT__(,) __VA_ARGS__ \ }; \ diff --git a/lib/events/roomevent.cpp b/lib/events/roomevent.cpp index 8928c81c..e98cb591 100644 --- a/lib/events/roomevent.cpp +++ b/lib/events/roomevent.cpp @@ -12,7 +12,7 @@ RoomEvent::RoomEvent(const QJsonObject& json) : Event(json) { if (const auto redaction = unsignedPart(RedactedCauseKeyL); !redaction.isEmpty()) - _redactedBecause = makeEvent(redaction); + _redactedBecause = loadEvent(redaction); } RoomEvent::~RoomEvent() = default; // Let the smart pointer do its job diff --git a/lib/events/roomevent.h b/lib/events/roomevent.h index 47b0b59d..203434f6 100644 --- a/lib/events/roomevent.h +++ b/lib/events/roomevent.h @@ -16,10 +16,7 @@ class QUOTIENT_API RoomEvent : public Event { public: QUO_BASE_EVENT(RoomEvent, {}, Event::BaseMetaType) - // RedactionEvent is an incomplete type here so we cannot inline - // constructors using it and also destructors (with 'using', in particular). - explicit RoomEvent(const QJsonObject& json); - ~RoomEvent() override; + ~RoomEvent() override; // Don't inline this - see the private section QString id() const; QDateTime originTimestamp() const; @@ -66,9 +63,12 @@ public: #endif protected: + explicit RoomEvent(const QJsonObject& json); void dumpTo(QDebug dbg) const override; private: + // RedactionEvent is an incomplete type here so we cannot inline + // constructors using it and also destructors (with 'using', in particular). event_ptr_tt _redactedBecause; #ifdef Quotient_E2EE_ENABLED diff --git a/lib/events/stateevent.h b/lib/events/stateevent.h index 911972f2..ffbce76e 100644 --- a/lib/events/stateevent.h +++ b/lib/events/stateevent.h @@ -24,7 +24,6 @@ public: //! constructors and calls in, e.g., RoomStateView don't include it. static constexpr auto needsStateKey = false; - explicit StateEventBase(const QJsonObject& json); explicit StateEventBase(Type type, const QString& stateKey = {}, const QJsonObject& contentJson = {}); @@ -39,9 +38,11 @@ public: } QString replacedState() const; - void dumpTo(QDebug dbg) const override; - virtual bool repeatsState() const; + +protected: + explicit StateEventBase(const QJsonObject& json); + void dumpTo(QDebug dbg) const override; }; using StateEventPtr = event_ptr_tt; using StateEvents = EventsArray; @@ -129,13 +130,15 @@ private: using base_type = EventTemplate; public: - explicit KeylessStateEventBase(const QJsonObject& fullJson) - : base_type(fullJson) - {} template explicit KeylessStateEventBase(ContentParamTs&&... contentParams) : base_type(QString(), std::forward(contentParams)...) {} + +protected: + explicit KeylessStateEventBase(const QJsonObject& fullJson) + : base_type(fullJson) + {} }; template -- cgit v1.2.3