Package: release.debian.org Severity: normal User: release.debian....@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 <dilin...@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 };