From cab6611cda0664dfcabafaf3ec266f979f4465ec Mon Sep 17 00:00:00 2001 From: zhangxianting Date: Mon, 1 Jul 2024 20:25:22 +0800 Subject: [PATCH] cve: CVE-2024-39936 --- CVE-2024-39936.patch | 246 +++++++++++++++++++++++++++++++++++++++++++ qt5-qtbase.spec | 6 +- 2 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 CVE-2024-39936.patch diff --git a/CVE-2024-39936.patch b/CVE-2024-39936.patch new file mode 100644 index 0000000..b6d166a --- /dev/null +++ b/CVE-2024-39936.patch @@ -0,0 +1,246 @@ +From b1e75376cc3adfc7da5502a277dfe9711f3e0536 Mon Sep 17 00:00:00 2001 +From: Mě±…rten Nordheim +Date: Tue, 25 Jun 2024 17:09:35 +0200 +Subject: [PATCH] HTTP2: Delay any communication until encrypted() can be responded to + +We have the encrypted() signal that lets users do extra checks on the +established connection. It is emitted as BlockingQueued, so the HTTP +thread stalls until it is done emitting. Users can potentially call +abort() on the QNetworkReply at that point, which is passed as a Queued +call back to the HTTP thread. That means that any currently queued +signal emission will be processed before the abort() call is processed. + +In the case of HTTP2 it is a little special since it is multiplexed and +the code is built to start requests as they are available. This means +that, while the code worked fine for HTTP1, since one connection only +has one request, it is not working for HTTP2, since we try to send more +requests in-between the encrypted() signal and the abort() call. + +This patch changes the code to delay any communication until the +encrypted() signal has been emitted and processed, for HTTP2 only. +It's done by adding a few booleans, both to know that we have to return +early and so we can keep track of what events arose and what we need to +resume once enough time has passed that any abort() call must have been +processed. + +Fixes: QTBUG-126610 +Pick-to: 6.8 6.7 6.5 6.2 5.15 5.12 +Change-Id: Ic25a600c278203256e35f541026f34a8783235ae +Reviewed-by: Marc Mutz +Reviewed-by: Volker Hilsheimer +--- + src/network/access/qhttp2protocolhandler.cpp | 6 +-- + .../access/qhttpnetworkconnectionchannel.cpp | 47 ++++++++++++++++++- + .../access/qhttpnetworkconnectionchannel_p.h | 7 +++ + tests/auto/network/access/http2/tst_http2.cpp | 44 +++++++++++++++++ + 4 files changed, 99 insertions(+), 5 deletions(-) + +diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp +index df7f87ef..e1ba6abb 100644 +--- a/src/network/access/qhttp2protocolhandler.cpp ++++ b/src/network/access/qhttp2protocolhandler.cpp +@@ -322,13 +322,13 @@ bool QHttp2ProtocolHandler::sendRequest() + return false; + } + +- if (!prefaceSent && !sendClientPreface()) +- return false; +- + auto &requests = m_channel->spdyRequestsToSend; + if (!requests.size()) + return true; + ++ if (!prefaceSent && !sendClientPreface()) ++ return false; ++ + m_channel->state = QHttpNetworkConnectionChannel::WritingState; + // Check what was promised/pushed, maybe we do not have to send a request + // and have a response already? +diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp +index 0ac14c78..76fc7a18 100644 +--- a/src/network/access/qhttpnetworkconnectionchannel.cpp ++++ b/src/network/access/qhttpnetworkconnectionchannel.cpp +@@ -247,20 +247,31 @@ void QHttpNetworkConnectionChannel::abort() + + bool QHttpNetworkConnectionChannel::sendRequest() + { +- Q_ASSERT(!protocolHandler.isNull()); +- return protocolHandler->sendRequest(); ++ if (waitingForPotentialAbort) { ++ needInvokeSendRequest = true; ++ return false; // this return value is unused ++ } ++ return sendRequest(); + } + + + void QHttpNetworkConnectionChannel::_q_receiveReply() + { + Q_ASSERT(!protocolHandler.isNull()); ++ if (waitingForPotentialAbort) { ++ needInvokeReceiveReply = true; ++ return; ++ } + protocolHandler->_q_receiveReply(); + } + + void QHttpNetworkConnectionChannel::_q_readyRead() + { + Q_ASSERT(!protocolHandler.isNull()); ++ if (waitingForPotentialAbort) { ++ needInvokeReadyRead = true; ++ return; ++ } + protocolHandler->_q_readyRead(); + } + +@@ -769,6 +780,27 @@ void QHttpNetworkConnectionChannel::closeAndResendCurrentRequest() + QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection); + } + ++void QHttpNetworkConnectionChannel::checkAndResumeCommunication() ++{ ++ Q_ASSERT(connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2 ++ || connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2Direct); ++ ++ // Because HTTP/2 requires that we send a SETTINGS frame as the first thing we do, and respond ++ // to a SETTINGS frame with an ACK, we need to delay any handling until we can ensure that any ++ // effects from emitting encrypted() have been processed. ++ // This function is called after encrypted() was emitted, so check for changes. ++ ++ if (!reply && h2RequestsToSend.isEmpty()) ++ abort(); ++ waitingForPotentialAbort = false; ++ if (needInvokeReadyRead) ++ _q_readyRead(); ++ if (needInvokeReceiveReply) ++ _q_receiveReply(); ++ if (needInvokeSendRequest) ++ sendRequest(); ++} ++ + void QHttpNetworkConnectionChannel::resendCurrentRequest() + { + requeueCurrentlyPipelinedRequests(); +@@ -1214,6 +1246,7 @@ void QHttpNetworkConnectionChannel::_q_encrypted() + if (!socket) + return; // ### error + state = QHttpNetworkConnectionChannel::IdleState; ++ waitingForPotentialAbort = true; + pendingEncrypt = false; + + if (connection->connectionType() == QHttpNetworkConnection::ConnectionTypeSPDY || +@@ -1221,6 +1254,16 @@ void QHttpNetworkConnectionChannel::_q_encrypted() + connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2Direct) { + // we call setSpdyWasUsed(true) on the replies in the SPDY handler when the request is sent + if (spdyRequestsToSend.count() > 0) { ++ ++ // We don't send or handle any received data until any effects from ++ // emitting encrypted() have been processed. This is necessary ++ // because the user may have called abort(). We may also abort the ++ // whole connection if the request has been aborted and there is ++ // no more requests to send. ++ QMetaObject::invokeMethod(this, ++ &QHttpNetworkConnectionChannel::checkAndResumeCommunication, ++ Qt::QueuedConnection); ++ + // In case our peer has sent us its settings (window size, max concurrent streams etc.) + // let's give _q_receiveReply a chance to read them first ('invokeMethod', QueuedConnection). + QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection); +diff --git a/src/network/access/qhttpnetworkconnectionchannel_p.h b/src/network/access/qhttpnetworkconnectionchannel_p.h +index e9cdae56..44c7b85e 100644 +--- a/src/network/access/qhttpnetworkconnectionchannel_p.h ++++ b/src/network/access/qhttpnetworkconnectionchannel_p.h +@@ -107,6 +107,10 @@ public: + QAbstractSocket *socket; + bool ssl; + bool isInitialized; ++ bool waitingForPotentialAbort = false; ++ bool needInvokeReceiveReply = false; ++ bool needInvokeReadyRead = false; ++ bool needInvokeSendRequest = false; + ChannelState state; + QHttpNetworkRequest request; // current request, only used for HTTP + QHttpNetworkReply *reply; // current reply for this request, only used for HTTP +@@ -127,6 +131,7 @@ public: + // HTTP/2 can be cleartext also, that's why it's + // outside of QT_NO_SSL section. Sorted by priority: + QMultiMap spdyRequestsToSend; ++ QMultiMap h2RequestsToSend; + bool switchedToHttp2 = false; + #ifndef QT_NO_SSL + bool ignoreAllSslErrors; +@@ -186,6 +191,8 @@ public: + void closeAndResendCurrentRequest(); + void resendCurrentRequest(); + ++ void checkAndResumeCommunication(); ++ + bool isSocketBusy() const; + bool isSocketWriting() const; + bool isSocketWaiting() const; +diff --git a/tests/auto/network/access/http2/tst_http2.cpp b/tests/auto/network/access/http2/tst_http2.cpp +index ecf4c581..686abea9 100644 +--- a/tests/auto/network/access/http2/tst_http2.cpp ++++ b/tests/auto/network/access/http2/tst_http2.cpp +@@ -76,6 +76,8 @@ private slots: + void goaway(); + void earlyResponse(); + ++ void abortOnEncrypted(); ++ + protected slots: + // Slots to listen to our in-process server: + void serverStarted(quint16 port); +@@ -481,6 +483,48 @@ void tst_Http2::earlyResponse() + QVERIFY(serverGotSettingsACK); + } + ++void tst_Http2::abortOnEncrypted() ++{ ++#if !QT_CONFIG(ssl) ++ QSKIP("TLS support is needed for this test"); ++#else ++ clearHTTP2State(); ++ serverPort = 0; ++ ++ ServerPtr targetServer(newServer(defaultServerSettings, H2Type::h2Direct)); ++ ++ QMetaObject::invokeMethod(targetServer.data(), "startServer", Qt::QueuedConnection); ++ runEventLoop(); ++ ++ nRequests = 1; ++ nSentRequests = 0; ++ ++ const auto url = requestUrl(H2Type::h2Direct); ++ QNetworkRequest request(url); ++ request.setAttribute(QNetworkRequest::Http2DirectAttribute, true); ++ ++ std::unique_ptr reply{manager->get(request)}; ++ reply->ignoreSslErrors(); ++ connect(reply.get(), &QNetworkReply::encrypted, reply.get(), [reply = reply.get()](){ ++ reply->abort(); ++ }); ++ connect(reply.get(), &QNetworkReply::errorOccurred, this, &tst_Http2::replyFinishedWithError); ++ ++ runEventLoop(); ++ STOP_ON_FAILURE ++ ++ QCOMPARE(nRequests, 0); ++ QCOMPARE(reply->error(), QNetworkReply::OperationCanceledError); ++ ++ const bool res = QTest::qWaitFor( ++ [this, server = targetServer.get()]() { ++ return serverGotSettingsACK || prefaceOK || nSentRequests > 0; ++ }, ++ 500); ++ QVERIFY(!res); ++#endif // QT_CONFIG(ssl) ++} ++ + void tst_Http2::serverStarted(quint16 port) + { + serverPort = port; +-- +2.33.0 + diff --git a/qt5-qtbase.spec b/qt5-qtbase.spec index 32fc6ba..ffd2a74 100644 --- a/qt5-qtbase.spec +++ b/qt5-qtbase.spec @@ -13,7 +13,7 @@ Name: qt5-qtbase Summary: Core component of Qt toolkit Version: 5.11.1 -Release: 22 +Release: 23 License: LGPLv2 with exceptions or GPLv3 with exceptions Url: http://qt-project.org/ Source0: https://download.qt.io/new_archive/qt/5.11/%{version}/submodules/qtbase-everywhere-src-%{version}.tar.xz @@ -58,6 +58,7 @@ Patch6013: qtbase5.11.1-CVE-2023-38197.patch Patch6014: qtbase5.11.1-CVE-2023-43114.patch Patch6015: qtbase5.11.1-CVE-2023-51714.patch Patch6016: CVE-2023-45935.patch +Patch6017: CVE-2024-39936.patch BuildRequires: pkgconfig(libsystemd) cups-devel desktop-file-utils findutils BuildRequires: libjpeg-devel libmng-devel libtiff-devel pkgconfig(alsa) @@ -425,6 +426,9 @@ fi %changelog +* Mon Jul 08 2024 zhangxianting - 5.11.1-23 +- cve: CVE-2024-39936 + * Wed Apr 24 2024 lvfei - 5.11.1-22 - Fix CVE-2023-45935 -- Gitee