[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#987075: marked as done (unblock: libquotient/0.6.6-1)



Your message dated Sat, 17 Apr 2021 09:41:58 +0000
with message-id <E1lXhSg-0001kg-4a@respighi.debian.org>
and subject line unblock libquotient
has caused the Debian Bug report #987075,
regarding unblock: libquotient/0.6.6-1
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
987075: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=987075
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Please unblock package libquotient

[ Reason ]
0.6.6 of libquotient fixes a security issue (in the form of a remote DoS).
This doesn't affect stable (the bug was introduced in 0.6.2 according to
https://github.com/quotient-im/libQuotient/issues/456), so it only needs
to be fixed in testing at this point and shouldn't need a DSA.

[ Impact ]
Users of the library can have their matrix clients crash by malformed user
ids.

[ Tests ]
I've let it sit in sid for 20 days to ensure people used it (and I'm
currently using it with the Spectral client).

[ Risks ]
0.6.x is a bugfix branch for libquotient, with the 0.7 branch seeing
active development. Although the update (0.6.4 -> 0.6.6) also includes
changes from the the 0.6.5 release, these are simply additional
backported fixes. Clients currently using libquotient in Debian are
Spectral and Quaternion.

[ Checklist ]
  [ ] all changes are documented in the d/changelog
  [X] I reviewed all changes and I approve them
  [X] attach debdiff against the package in testing

[ Other info ]
(Anything else the release team should know.)

unblock libquotient/0.6.6-1
diff -Nru libquotient-0.6.4/CMakeLists.txt libquotient-0.6.6/CMakeLists.txt
--- libquotient-0.6.4/CMakeLists.txt	2021-01-15 15:53:46.000000000 +0000
+++ libquotient-0.6.6/CMakeLists.txt	2021-03-17 20:23:20.000000000 +0000
@@ -4,7 +4,7 @@
 endif()
 
 set(API_VERSION "0.6")
-project(Quotient VERSION "${API_VERSION}.4" LANGUAGES CXX)
+project(Quotient VERSION "${API_VERSION}.6" LANGUAGES CXX)
 
 option(${PROJECT_NAME}_INSTALL_TESTS "install quotest (former qmc-example) application" ON)
 # https://github.com/quotient-im/libQuotient/issues/369
diff -Nru libquotient-0.6.4/CONTRIBUTING.md libquotient-0.6.6/CONTRIBUTING.md
--- libquotient-0.6.4/CONTRIBUTING.md	2021-01-15 15:53:46.000000000 +0000
+++ libquotient-0.6.6/CONTRIBUTING.md	2021-03-17 20:23:20.000000000 +0000
@@ -88,8 +88,7 @@
 
 Unless a contributor explicitly specifies otherwise, we assume contributors
 to agree that all contributed code is released either under *LGPL v2.1 or later*.
-This is more than just [LGPL v2.1 libQuotient now uses](./COPYING)
-because the project plans to switch to LGPL v3 for library code in the near future.
+The project plans to switch to LGPL v3 for library code in the near future.
 <!-- The below is invalid yet!
 All new contributed material that is not executable, including all text when not executed, is also released under the
 [Creative Commons Attribution 4.0 International (CC BY 4.0) license](https://creativecommons.org/licenses/by/4.0/) or later.
diff -Nru libquotient-0.6.4/debian/changelog libquotient-0.6.6/debian/changelog
--- libquotient-0.6.4/debian/changelog	2021-01-19 22:21:30.000000000 +0000
+++ libquotient-0.6.6/debian/changelog	2021-03-28 03:51:17.000000000 +0000
@@ -1,3 +1,10 @@
+libquotient (0.6.6-1) unstable; urgency=high
+
+  * New upstream release. This fixes a remotely-triggered crash (see
+    https://github.com/quotient-im/libQuotient/issues/456).
+
+ -- Andres Salomon <dilinger@debian.org>  Sat, 27 Mar 2021 23:51:17 -0400
+
 libquotient (0.6.4-2) unstable; urgency=medium
 
   * Remove libqmatrixclient-dev dummy package; Hubert pointed out that
diff -Nru libquotient-0.6.4/lib/connection.cpp libquotient-0.6.6/lib/connection.cpp
--- libquotient-0.6.4/lib/connection.cpp	2021-01-15 15:53:46.000000000 +0000
+++ libquotient-0.6.6/lib/connection.cpp	2021-03-17 20:23:20.000000000 +0000
@@ -133,6 +133,11 @@
         != "json";
     bool lazyLoading = false;
 
+    /// \brief Stop resolving and login flows jobs, and clear login flows
+    ///
+    /// Prepares the class to set or resolve a new homeserver
+    void clearResolvingContext();
+
     /** \brief Check the homeserver and resolve it if needed, before connecting
      *
      * A single entry for functions that need to check whether the homeserver
@@ -270,8 +275,7 @@
 
 void Connection::resolveServer(const QString& mxid)
 {
-    if (isJobRunning(d->resolverJob))
-        d->resolverJob->abandon();
+    d->clearResolvingContext();
 
     auto maybeBaseUrl = QUrl::fromUserInput(serverPart(mxid));
     maybeBaseUrl.setScheme("https"); // Instead of the Qt-default "http"
@@ -687,7 +691,7 @@
 
                 DirectChatsMap remoteAdditions;
                 for (auto it = usersToDCs.begin(); it != usersToDCs.end(); ++it) {
-                    if (auto* u = q->user(it.key())) {
+                    if (auto* const u = q->user(it.key())) {
                         if (!directChats.contains(u, it.value())
                             && !dcLocalRemovals.contains(u, it.value())) {
                             Q_ASSERT(!directChatUsers.contains(it.value(), u));
@@ -920,6 +924,12 @@
                        const QJsonObject& creationContent)
 {
     invites.removeOne(userId()); // The creator is by definition in the room
+    for (const auto& i : invites)
+        if (!user(i)) {
+            qCWarning(MAIN) << "Won't create a room with malformed invitee ids";
+            return nullptr;
+        }
+
     auto job = callApi<CreateRoomJob>(visibility == PublishRoom
                                           ? QStringLiteral("public")
                                           : QStringLiteral("private"),
@@ -954,7 +964,7 @@
 void Connection::doInDirectChat(const QString& userId,
                                 const std::function<void(Room*)>& operation)
 {
-    if (auto* u = user(userId))
+    if (auto* const u = user(userId))
         doInDirectChat(u, operation);
     else
         qCCritical(MAIN)
@@ -1530,13 +1540,19 @@
     return d->data->generateTxnId();
 }
 
+void Connection::Private::clearResolvingContext()
+{
+    if (isJobRunning(resolverJob))
+        resolverJob->abandon();
+    if (isJobRunning(loginFlowsJob))
+        loginFlowsJob->abandon();
+    loginFlows.clear();
+
+}
+
 void Connection::setHomeserver(const QUrl& url)
 {
-    if (isJobRunning(d->resolverJob))
-        d->resolverJob->abandon();
-    if (isJobRunning(d->loginFlowsJob))
-        d->loginFlowsJob->abandon();
-    d->loginFlows.clear();
+    d->clearResolvingContext();
 
     if (homeserver() != url) {
         d->data->setBaseUrl(url);
diff -Nru libquotient-0.6.4/lib/events/roommessageevent.cpp libquotient-0.6.6/lib/events/roommessageevent.cpp
--- libquotient-0.6.4/lib/events/roommessageevent.cpp	2021-01-15 15:53:46.000000000 +0000
+++ libquotient-0.6.6/lib/events/roommessageevent.cpp	2021-03-17 20:23:20.000000000 +0000
@@ -313,7 +313,7 @@
     const auto actualJson = isReplacement(relatesTo)
                                 ? json.value("m.new_content"_ls).toObject()
                                 : json;
-    // Special-casing the custom matrix.org's (actually, Riot's) way
+    // Special-casing the custom matrix.org's (actually, Element's) way
     // of sending HTML messages.
     if (actualJson["format"_ls].toString() == HtmlContentTypeId) {
         mimeType = HtmlMimeType;
@@ -338,12 +338,15 @@
     }
     if (relatesTo) {
         json->insert(QStringLiteral("m.relates_to"),
-                     QJsonObject { { "rel_type", relatesTo->type }, { EventIdKey, relatesTo->eventId } });
+                     relatesTo->type == RelatesTo::ReplyTypeId() ?
+                         QJsonObject { { relatesTo->type, QJsonObject{ { EventIdKey, relatesTo->eventId } } } } :
+                         QJsonObject { { "rel_type", relatesTo->type }, { EventIdKey, relatesTo->eventId } }
+        );
         if (relatesTo->type == RelatesTo::ReplacementTypeId()) {
             QJsonObject newContentJson;
             if (mimeType.inherits("text/html")) {
-                json->insert(FormatKey, HtmlContentTypeId);
-                json->insert(FormattedBodyKey, body);
+                newContentJson.insert(FormatKey, HtmlContentTypeId);
+                newContentJson.insert(FormattedBodyKey, body);
             }
             json->insert(QStringLiteral("m.new_content"), newContentJson);
         }
diff -Nru libquotient-0.6.4/lib/room.cpp libquotient-0.6.6/lib/room.cpp
--- libquotient-0.6.4/lib/room.cpp	2021-01-15 15:53:46.000000000 +0000
+++ libquotient-0.6.6/lib/room.cpp	2021-03-17 20:23:20.000000000 +0000
@@ -1460,7 +1460,9 @@
 
 QString Room::roomMembername(const QString& userId) const
 {
-    return roomMembername(user(userId));
+    if (auto* const u = user(userId))
+        return roomMembername(u);
+    return {};
 }
 
 QString Room::safeMemberName(const QString& userId) const
@@ -2349,7 +2351,7 @@
         // the new message events.
         if (const auto senderId = (*from)->senderId(); !senderId.isEmpty()) {
             auto* const firstWriter = q->user(senderId);
-            if (q->readMarker(firstWriter) != timeline.crend()) {
+            if (firstWriter && q->readMarker(firstWriter) != timeline.crend()) {
                 roomChanges |=
                     promoteReadMarker(firstWriter, rev_iter_t(from) - 1);
                 qCDebug(MESSAGES)
@@ -2418,6 +2420,13 @@
     if (!e.isStateEvent())
         return Change::NoChange;
 
+    auto* const sender = user(e.senderId());
+    if (!sender) {
+        qCWarning(MAIN) << "State event" << e.id()
+                        << "is invalid and won't be processed";
+        return Change::NoChange;
+    }
+
     // Find a value (create an empty one if necessary) and get a reference
     // to it. Can't use getCurrentState<>() because it (creates and) returns
     // a stub if a value is not found, and what's needed here is a "real" event
@@ -2426,9 +2435,9 @@
     // Prepare for the state change
     const auto oldRme = static_cast<const RoomMemberEvent*>(curStateEvent);
     visit(e, [this, &oldRme](const RoomMemberEvent& rme) {
-        auto* u = user(rme.userId());
-        if (!u) { // ???
-            qCCritical(MAIN)
+        auto* const u = user(rme.userId());
+        if (!u) { // Invalid user id?
+            qCWarning(MAIN)
                 << "Could not get a user object for" << rme.userId();
             return;
         }
@@ -2517,9 +2526,11 @@
                 emit avatarChanged();
             return AvatarChange;
         }
-        , [this,oldRme] (const RoomMemberEvent& evt) {
+        , [this,oldRme,sender] (const RoomMemberEvent& evt) {
             // clang-format on
             auto* u = user(evt.userId());
+            if (!u)
+                return NoChange; // Already warned earlier
             // TODO: remove in 0.7
             u->processEvent(evt, this, oldRme == nullptr);
 
@@ -2539,7 +2550,7 @@
                 if (!d->usersInvited.contains(u))
                     d->usersInvited.push_back(u);
                 if (u == localUser() && evt.isDirect())
-                    connection()->addToDirectChats(this, user(evt.senderId()));
+                    connection()->addToDirectChats(this, sender);
                 break;
             case MembershipType::Knock:
             case MembershipType::Ban:
@@ -2599,8 +2610,8 @@
     if (auto* evt = eventCast<TypingEvent>(event)) {
         d->usersTyping.clear();
         for (const QString& userId : qAsConst(evt->users())) {
-            auto u = user(userId);
-            if (memberJoinState(u) == JoinState::Join)
+            auto* const u = user(userId);
+            if (u && memberJoinState(u) == JoinState::Join)
                 d->usersTyping.append(u);
         }
         if (evt->users().size() > 3 || et.nsecsElapsed() >= profilerMinNsecs())
@@ -2625,8 +2636,8 @@
                 for (const Receipt& r : p.receipts) {
                     if (r.userId == connection()->userId())
                         continue; // FIXME, #185
-                    auto u = user(r.userId);
-                    if (memberJoinState(u) == JoinState::Join)
+                    auto* const u = user(r.userId);
+                    if (u && memberJoinState(u) == JoinState::Join)
                         changes |= d->promoteReadMarker(u, newMarker);
                 }
             } else {
@@ -2639,8 +2650,8 @@
                 for (const Receipt& r : p.receipts) {
                     if (r.userId == connection()->userId())
                         continue; // FIXME, #185
-                    auto u = user(r.userId);
-                    if (memberJoinState(u) == JoinState::Join
+                    auto* const u = user(r.userId);
+                    if (u && memberJoinState(u) == JoinState::Join
                         && readMarker(u) == timelineEdge())
                         changes |= d->setLastReadEvent(u, p.evtId);
                 }
diff -Nru libquotient-0.6.4/lib/uri.cpp libquotient-0.6.6/lib/uri.cpp
--- libquotient-0.6.4/lib/uri.cpp	2021-01-15 15:53:46.000000000 +0000
+++ libquotient-0.6.6/lib/uri.cpp	2021-03-17 20:23:20.000000000 +0000
@@ -7,13 +7,19 @@
 using namespace Quotient;
 
 struct ReplacePair { QByteArray uriString; char sigil; };
-/// Defines bi-directional mapping of path prefixes and sigils
+/// \brief Defines bi-directional mapping of path prefixes and sigils
+///
+/// When there are two prefixes for the same sigil, the first matching
+/// entry for a given sigil is used.
 static const auto replacePairs = {
-    ReplacePair { "user/", '@' },
+    ReplacePair { "u/", '@' },
+    { "user/", '@' },
     { "roomid/", '!' },
+    { "r/", '#' },
     { "room/", '#' },
     // The notation for bare event ids is not proposed in MSC2312 but there's
     // https://github.com/matrix-org/matrix-doc/pull/2644
+    { "e/", '$' },
     { "event/", '$' }
 };
 
@@ -94,7 +100,7 @@
         case 2:
             break;
         case 4:
-            if (splitPath[2] == "event")
+            if (splitPath[2] == "event" || splitPath[2] == "e")
                 break;
             [[fallthrough]];
         default:
@@ -147,7 +153,8 @@
 
 Uri::SecondaryType Uri::secondaryType() const
 {
-    return pathSegment(*this, 2) == "event" ? EventId : NoSecondaryId;
+    const auto& type2 = pathSegment(*this, 2);
+    return type2 == "event" || type2 == "e" ? EventId : NoSecondaryId;
 }
 
 QUrl Uri::toUrl(UriForm form) const
diff -Nru libquotient-0.6.4/lib/uriresolver.cpp libquotient-0.6.6/lib/uriresolver.cpp
--- libquotient-0.6.4/lib/uriresolver.cpp	2021-01-15 15:53:46.000000000 +0000
+++ libquotient-0.6.6/lib/uriresolver.cpp	2021-03-17 20:23:20.000000000 +0000
@@ -24,9 +24,9 @@
     case Uri::UserId: {
         if (uri.action() == "join")
             return IncorrectAction;
-        auto* user = account->user(uri.primaryId());
-        Q_ASSERT(user != nullptr);
-        return visitUser(user, uri.action());
+        if (auto* const user = account->user(uri.primaryId()))
+            return visitUser(user, uri.action());
+        return InvalidUri;
     }
     case Uri::RoomId:
     case Uri::RoomAlias: {
diff -Nru libquotient-0.6.4/README.md libquotient-0.6.6/README.md
--- libquotient-0.6.4/README.md	2021-01-15 15:53:46.000000000 +0000
+++ libquotient-0.6.6/README.md	2021-03-17 20:23:20.000000000 +0000
@@ -14,7 +14,7 @@
 for [Matrix](https://matrix.org). libQuotient is a library that enables client
 applications. It is the backbone of
 [Quaternion](https://github.com/quotient-im/Quaternion),
-[Spectral](https://matrix.org/docs/projects/client/spectral.html) and
+[NeoChat](https://matrix.org/docs/projects/client/neo-chat) and
 other projects.
 Versions 0.5.x and older use the previous name - libQMatrixClient.
 
@@ -77,8 +77,8 @@
 libQuotient, assuming the library development files are installed. There's no
 documented procedure to use a preinstalled library with qmake; consider
 introducing a submodule in your source tree and build it along with the rest
-of the application for now. Note also that qmake is considered for phase-out
-in Qt 6 so you should probably think of moving over to CMake eventually.
+of the application for now. Note also that qmake is no more supported
+in libQuotient 0.7 so you should really think of moving over to CMake.
 
 Building with dynamic linkage is only tested on Linux at the moment and is
 a recommended way of linking your application with libQuotient on this platform.
@@ -89,7 +89,8 @@
 most common use cases such as sending messages, uploading files,
 setting room state etc.; for more extensive usage check out the source code
 of [Quaternion](https://github.com/quotient-im/Quaternion)
-(the reference client of Quotient) or [Spectral](https://gitlab.com/b0/spectral).
+(the reference client of Quotient) or
+[NeoChat](https://invent.kde.org/network/neochat).
 
 To ease the first step, `tests/CMakeLists.txt` is a good starting point
 for your own CMake-based project using libQuotient.
@@ -160,7 +161,7 @@
 along with the rest of the library can be skipped
 by setting `Quotient_INSTALL_TESTS` to `OFF`.
 
-### qmake-based
+### qmake-based (deprecated)
 The library provides a .pri file with an intention to be included from a bigger project's .pro file. As a starting point you can use `quotest.pro` that will build a minimal example of library usage for you. In the root directory of the project sources:
 ```shell script
 qmake quotest.pro
@@ -173,8 +174,7 @@
 qmake didn't really know about C++17 until Qt 5.12 so if your Qt is older
 you may have quite a bit of warnings during the compilation process.
 
-Installing the standalone library with qmake is not implemented yet; PRs are
-welcome though.
+Installing the standalone library with qmake has not been implemented.
 
 ## Troubleshooting
 
diff -Nru libquotient-0.6.4/SECURITY.md libquotient-0.6.6/SECURITY.md
--- libquotient-0.6.4/SECURITY.md	2021-01-15 15:53:46.000000000 +0000
+++ libquotient-0.6.6/SECURITY.md	2021-03-17 20:23:20.000000000 +0000
@@ -6,7 +6,6 @@
 | ------- | ------------------ |
 | master  | :white_check_mark: |
 | 0.6.x   | :white_check_mark: |
-| 0.5.x   | :white_check_mark: |
 | older   | :x:                |
 
 ## Reporting a Vulnerability
diff -Nru libquotient-0.6.4/tests/quotest.cpp libquotient-0.6.6/tests/quotest.cpp
--- libquotient-0.6.4/tests/quotest.cpp	2021-01-15 15:53:46.000000000 +0000
+++ libquotient-0.6.6/tests/quotest.cpp	2021-03-17 20:23:20.000000000 +0000
@@ -726,12 +726,15 @@
         roomId, "matrix:roomid/" + roomId.mid(1),
         "https://matrix.to/#/%21"/*`!`*/ + roomId.mid(1),
         roomAlias, "matrix:room/" + roomAlias.mid(1),
+        "matrix:r/" + roomAlias.mid(1),
         "https://matrix.to/#/"; + roomAlias,
     };
     const QStringList userUris { userId, "matrix:user/" + userId.mid(1),
+                                 "matrix:u/" + userId.mid(1),
                                  "https://matrix.to/#/"; + userId };
     const QStringList eventUris {
         "matrix:room/" + roomAlias.mid(1) + "/event/" + eventId.mid(1),
+        "matrix:r/" + roomAlias.mid(1) + "/e/" + eventId.mid(1),
         "https://matrix.to/#/"; + roomId + '/' + eventId
     };
     // Check that reserved characters are correctly processed.
@@ -745,6 +748,7 @@
     static const QStringList joinByAliasUris {
         Uri(joinRoomAlias.toUtf8(), {}, joinQuery.mid(1)).toDisplayString(),
         "matrix:room/" + encodedRoomAliasNoSigil + joinQuery,
+        "matrix:r/" + encodedRoomAliasNoSigil + joinQuery,
         "https://matrix.to/#/%23"/*`#`*/ + encodedRoomAliasNoSigil + joinQuery,
         "https://matrix.to/#/%23"; + joinRoomAlias.mid(1) /* unencoded */ + joinQuery
     };

--- End Message ---
--- Begin Message ---
Unblocked.

--- End Message ---

Reply to: