1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Http2ProtocolHandler: fix logic error with potential use-after-free

Browse Source 
We previously asserted that the reply was not nullptr, except in some
special circumstance. But then we proceeded to dereference it anyway.
This was then recently changed to be an if-check, but that just
highlighted the logic-flaw (and made static analyzers warn about it...)

What we want to assert is that the stream object is valid and
conditionally return early if the reply is nullptr, which it is for
promised streams, since no request has been made yet so no reply is
created.

At the same time, update the logic in the QHttp2Stream to not store or
emit header-related signals for a stream that has been reset.

Change-Id: I55d69bbedc027893f6ad125c29468a34e7fb406f
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
(cherry picked from commit 3af5e42bdd6ffcf6d9ca386583add2329372056f)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 43e249cba8af8ab34e39fa63adbbbef29251d673)
This commit is contained in:
Mårten Nordheim 2025-06-11 18:03:12 +02:00 committed by Qt Cherry-pick Bot
parent 6ae2f33128
commit f5eb24d5b8
3 changed files with 8 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/network/access/qhttp2connection.cpp
View File

@ -1930,8 +1930,8 @@ void QHttp2Connection::handleContinuedHEADERS()
return connectionError(FRAME_SIZE_ERROR, "HEADERS frame too large");
}
if (streamIt == m_streams.cend()) // No more processing without a stream from here on.
return;
if (streamWasResetLocally(streamID) || streamIt == m_streams.cend())
return; // No more processing without a stream from here on.
switch (firstFrameType) {
case FrameType::HEADERS:

3
src/network/access/qhttp2protocolhandler.cpp
View File

@ -308,10 +308,11 @@ bool QHttp2ProtocolHandler::sendDATA(QHttp2Stream *stream, QHttpNetworkReply *re
void QHttp2ProtocolHandler::handleHeadersReceived(const HPack::HttpHeader &headers, bool endStream)
{
QHttp2Stream *stream = qobject_cast<QHttp2Stream *>(sender());
Q_ASSERT(stream);
auto &requestPair = requestReplyPairs[stream];
auto *httpReply = requestPair.second;
auto &httpRequest = requestPair.first;
if (!httpReply && (!stream || stream->state() != QHttp2Stream::State::ReservedRemote))
if (!httpReply)
return;
auto *httpReplyPrivate = httpReply->d_func();

4
tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp
View File

@ -745,6 +745,7 @@ void tst_QHttp2Connection::headerFrameAfterRSTOutgoing()
QHttp2Stream *clientStream = connection->createStream().unwrap();
QVERIFY(clientStream);
QSignalSpy client1HeadersSpy{ clientStream, &QHttp2Stream::headersReceived};
QVERIFY(waitForSettingsExchange(connection, serverConnection));
QSignalSpy newIncomingStreamSpy{ serverConnection, &QHttp2Connection::newIncomingStream };
@ -774,6 +775,9 @@ void tst_QHttp2Connection::headerFrameAfterRSTOutgoing()
// causing an error on Qt's side.
QVERIFY(serverRSTReceivedSpy.wait());
// We don't emit any headers for a reset stream
QVERIFY(!client1HeadersSpy.count());
// Create a new stream then send and handle a new request!
QHttp2Stream *clientStream2 = connection->createStream().unwrap();
QSignalSpy client2HeaderReceivedSpy{ clientStream2, &QHttp2Stream::headersReceived };