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

Http2: flip some checks for PUSH_PROMISE around

Browse Source 
It was correctly following the RFC, but we overlooked that the
RFC is talking about restrictions on _sending_, while we are
verifying on the _receiving_ end. So, some things has to be flipped.

And the dataSize offset was incorrect and could read out-of-bounds.

Amends d17d260948e16549d82f1fdd4dec98d246b0622e

Pick-to: 6.7
Change-Id: Idb7e00637398dff190fc4e5b9776a8649a04f391
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
(cherry picked from commit 02d1d638545fd8600fac9707a8f9af03f99197ec)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Mårten Nordheim 2024-08-20 18:28:03 +02:00 committed by Qt Cherry-pick Bot
parent cf0b87c7c2
commit d2e9a03ee3
1 changed files with 8 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1564,14 +1564,18 @@ void QHttp2Connection::handlePUSH_PROMISE()
#endif #endif
// RFC 9113, 6.6: PUSH_PROMISE frames MUST only be sent on a peer-initiated stream that // RFC 9113, 6.6: PUSH_PROMISE frames MUST only be sent on a peer-initiated stream that
// is in either the "open" or "half-closed (remote)" state. // is in either the "open" or "half-closed (remote)" state.
// I.e. If you are the server then the client must have initiated the stream you are sending
// the promise on. And since this is about _sending_ we have to invert "Remote" to "Local"
// because we are receiving.
if (it == m_streams.constEnd()) if (it == m_streams.constEnd())
return connectionError(ENHANCE_YOUR_CALM, "PUSH_PROMISE with invalid associated stream"); return connectionError(ENHANCE_YOUR_CALM, "PUSH_PROMISE with invalid associated stream");
if ((m_connectionType == Type::Client && (streamID & 1) == 1) || if ((m_connectionType == Type::Client && (streamID & 1) == 0) ||
(m_connectionType == Type::Server && (streamID & 1) == 0)) { (m_connectionType == Type::Server && (streamID & 1) == 1)) {
return connectionError(ENHANCE_YOUR_CALM, "PUSH_PROMISE with invalid associated stream"); return connectionError(ENHANCE_YOUR_CALM, "PUSH_PROMISE with invalid associated stream");
} }
if ((*it)->state() != QHttp2Stream::State::Open && if ((*it)->state() != QHttp2Stream::State::Open &&
(*it)->state() != QHttp2Stream::State::HalfClosedRemote) { (*it)->state() != QHttp2Stream::State::HalfClosedLocal) {
return connectionError(ENHANCE_YOUR_CALM, "PUSH_PROMISE with invalid associated stream"); return connectionError(ENHANCE_YOUR_CALM, "PUSH_PROMISE with invalid associated stream");
} }
@ -1600,7 +1604,7 @@ void QHttp2Connection::handlePUSH_PROMISE()
// Length field. If the length of the padding is the length of the frame payload or greater, // Length field. If the length of the padding is the length of the frame payload or greater,
// the recipient MUST treat this as a connection error (Section 5.4.1) of type PROTOCOL_ERROR. // the recipient MUST treat this as a connection error (Section 5.4.1) of type PROTOCOL_ERROR.
// checked in Frame::validateHeader() // checked in Frame::validateHeader()
Q_ASSERT(inboundFrame.dataSize() > qFromBigEndian<quint8>(inboundFrame.dataBegin() + 32)); Q_ASSERT(inboundFrame.dataSize() > inboundFrame.padding());
const bool endHeaders = inboundFrame.flags().testFlag(FrameFlag::END_HEADERS); const bool endHeaders = inboundFrame.flags().testFlag(FrameFlag::END_HEADERS);
continuedFrames.clear(); continuedFrames.clear();
continuedFrames.push_back(std::move(inboundFrame)); continuedFrames.push_back(std::move(inboundFrame));