From d56ce750699d0af5d212668869fa2190c0d1c2ee Mon Sep 17 00:00:00 2001
From: ZhaoYu <jiangzhaoyu@kylinos.cn>
Date: Tue, 10 Dec 2024 02:46:12 +0800
Subject: [PATCH] Fix CVE-2024-39936

---
 src/network/access/qhttp2protocolhandler.cpp  |  6 +--
 .../access/qhttpnetworkconnectionchannel.cpp  | 48 ++++++++++++++++++-
 .../access/qhttpnetworkconnectionchannel_p.h  |  6 +++
 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 39dd4608..bc1dac68 100644
--- a/src/network/access/qhttp2protocolhandler.cpp
+++ b/src/network/access/qhttp2protocolhandler.cpp
@@ -371,12 +371,12 @@ bool QHttp2ProtocolHandler::sendRequest()
         }
     }
 
-    if (!prefaceSent && !sendClientPreface())
-        return false;
-
     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 7620ca16..16e2d00a 100644
--- a/src/network/access/qhttpnetworkconnectionchannel.cpp
+++ b/src/network/access/qhttpnetworkconnectionchannel.cpp
@@ -255,6 +255,10 @@ void QHttpNetworkConnectionChannel::abort()
 bool QHttpNetworkConnectionChannel::sendRequest()
 {
     Q_ASSERT(!protocolHandler.isNull());
+    if (waitingForPotentialAbort) {
+    	needInvokeSendRequest = true;
+	return false; // this return value is unused
+    }
     return protocolHandler->sendRequest();
 }
 
@@ -267,21 +271,28 @@ bool QHttpNetworkConnectionChannel::sendRequest()
 void QHttpNetworkConnectionChannel::sendRequestDelayed()
 {
     QMetaObject::invokeMethod(this, [this] {
-        Q_ASSERT(!protocolHandler.isNull());
         if (reply)
-            protocolHandler->sendRequest();
+		sendRequest();
     }, Qt::ConnectionType::QueuedConnection);
 }
 
 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();
 }
 
@@ -1289,7 +1300,18 @@ void QHttpNetworkConnectionChannel::_q_encrypted()
             // Similar to HTTP/1.1 counterpart below:
             const auto &pairs = spdyRequestsToSend.values(); // (request, reply)
             const auto &pair = pairs.first();
+	    waitingForPotentialAbort = true;
             emit pair.second->encrypted();
+
+	    // 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);
@@ -1307,6 +1329,28 @@ void QHttpNetworkConnectionChannel::_q_encrypted()
     }
 }
 
+void QHttpNetworkConnectionChannel::checkAndResumeCommunication()
+{
+    Q_ASSERT(connection->connectionType() == QHttpNetworkConnection::ConnectionTypeSPDY ||
+	     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 && spdyRequestsToSend.isEmpty())
+        abort();
+    waitingForPotentialAbort = false;
+    if (needInvokeReadyRead)
+        _q_readyRead();
+    if (needInvokeReceiveReply)
+	_q_receiveReply();
+    if (needInvokeSendRequest)
+	sendRequest();
+}
+
 void QHttpNetworkConnectionChannel::requeueSpdyRequests()
 {
     QList<HttpMessagePair> spdyPairs = spdyRequestsToSend.values();
diff --git a/src/network/access/qhttpnetworkconnectionchannel_p.h b/src/network/access/qhttpnetworkconnectionchannel_p.h
index d8ac3979..eac44464 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
@@ -187,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 bf8c7799..8ed95b99 100644
--- a/tests/auto/network/access/http2/tst_http2.cpp
+++ b/tests/auto/network/access/http2/tst_http2.cpp
@@ -118,6 +118,8 @@ private slots:
     void redirect_data();
     void redirect();
 
+    void abortOnEncrypted();
+
 protected slots:
     // Slots to listen to our in-process server:
     void serverStarted(quint16 port);
@@ -1024,6 +1026,48 @@ void tst_Http2::redirect()
     QTRY_VERIFY(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<QNetworkReply> 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.43.0