summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKurt Pattyn <pattyn.kurt@gmail.com>2015-09-06 15:30:03 +0200
committerLiang Qi <liang.qi@theqtcompany.com>2015-09-06 15:29:35 +0000
commit12e424f241b29ef26ad2a3a70740d8b320e9e85a (patch)
tree88f84440c9f213acc312f78bc11cd577c00ed2c0
parente8335d48aa8c6323a6b89d29c76f2b340afd1be4 (diff)
downloadqtwebsockets-5.5.1.tar.gz
Fix DoS vulnerabilityv5.5.15.5.1
Add checks on maximum header line length and on the maximum number of header lines. Task-number: QTBUG-48123 Change-Id: I65dbeb53af7aa0dfa137ce31fc2549940559314e Reviewed-by: Richard J. Moore <rich@kde.org>
-rw-r--r--src/websockets/qwebsockethandshakerequest.cpp56
-rw-r--r--src/websockets/qwebsockethandshakerequest_p.h2
-rw-r--r--src/websockets/qwebsocketserver_p.cpp8
-rw-r--r--tests/auto/websockets/handshakerequest/tst_handshakerequest.cpp76
-rw-r--r--tests/auto/websockets/handshakeresponse/tst_handshakeresponse.cpp2
5 files changed, 130 insertions, 14 deletions
diff --git a/src/websockets/qwebsockethandshakerequest.cpp b/src/websockets/qwebsockethandshakerequest.cpp
index 528cb32..e2cbaef 100644
--- a/src/websockets/qwebsockethandshakerequest.cpp
+++ b/src/websockets/qwebsockethandshakerequest.cpp
@@ -183,18 +183,49 @@ QUrl QWebSocketHandshakeRequest::requestUrl() const
}
/*!
+ Reads a line of text from the given textstream (terminated by CR/LF).
+ If an empty line was detected, an empty string is returned.
+ When an error occurs, a null string is returned.
\internal
*/
-void QWebSocketHandshakeRequest::readHandshake(QTextStream &textStream)
+static QString readLine(QTextStream &stream, int maxHeaderLineLength)
+{
+ QString line;
+ char c;
+ while (!stream.atEnd()) {
+ stream >> c;
+ if (stream.status() != QTextStream::Ok)
+ return QString();
+ if (c == char('\r')) {
+ //eat the \n character
+ stream >> c;
+ line.append(QStringLiteral(""));
+ break;
+ } else {
+ line.append(QChar::fromLatin1(c));
+ if (line.length() > maxHeaderLineLength)
+ return QString();
+ }
+ }
+ return line;
+}
+
+/*!
+ \internal
+ */
+void QWebSocketHandshakeRequest::readHandshake(QTextStream &textStream, int maxHeaderLineLength,
+ int maxHeaders)
{
- m_isValid = false;
clear();
if (Q_UNLIKELY(textStream.status() != QTextStream::Ok))
return;
- const QString requestLine = textStream.readLine();
+ const QString requestLine = readLine(textStream, maxHeaderLineLength);
+ if (requestLine.isNull()) {
+ clear();
+ return;
+ }
const QStringList tokens = requestLine.split(' ', QString::SkipEmptyParts);
if (Q_UNLIKELY(tokens.length() < 3)) {
- m_isValid = false;
clear();
return;
}
@@ -206,10 +237,13 @@ void QWebSocketHandshakeRequest::readHandshake(QTextStream &textStream)
if (Q_UNLIKELY(!conversionOk)) {
clear();
- m_isValid = false;
return;
}
- QString headerLine = textStream.readLine();
+ QString headerLine = readLine(textStream, maxHeaderLineLength);
+ if (headerLine.isNull()) {
+ clear();
+ return;
+ }
m_headers.clear();
while (!headerLine.isEmpty()) {
const QStringList headerField = headerLine.split(QStringLiteral(": "),
@@ -219,7 +253,15 @@ void QWebSocketHandshakeRequest::readHandshake(QTextStream &textStream)
return;
}
m_headers.insertMulti(headerField.at(0).toLower(), headerField.at(1));
- headerLine = textStream.readLine();
+ if (m_headers.size() > maxHeaders) {
+ clear();
+ return;
+ }
+ headerLine = readLine(textStream, maxHeaderLineLength);
+ if (headerLine.isNull()) {
+ clear();
+ return;
+ }
}
m_requestUrl = QUrl::fromEncoded(resourceName.toLatin1());
diff --git a/src/websockets/qwebsockethandshakerequest_p.h b/src/websockets/qwebsockethandshakerequest_p.h
index ad2a249..6ad5381 100644
--- a/src/websockets/qwebsockethandshakerequest_p.h
+++ b/src/websockets/qwebsockethandshakerequest_p.h
@@ -78,7 +78,7 @@ public:
QString resourceName() const;
QString host() const;
- void readHandshake(QTextStream &textStream);
+ void readHandshake(QTextStream &textStream, int maxHeaderLineLength, int maxHeaders);
private:
diff --git a/src/websockets/qwebsocketserver_p.cpp b/src/websockets/qwebsocketserver_p.cpp
index bc23674..d1750ce 100644
--- a/src/websockets/qwebsocketserver_p.cpp
+++ b/src/websockets/qwebsocketserver_p.cpp
@@ -49,6 +49,12 @@
QT_BEGIN_NAMESPACE
+//both constants are taken from the default settings of Apache
+//see: http://httpd.apache.org/docs/2.2/mod/core.html#limitrequestfieldsize and
+//http://httpd.apache.org/docs/2.2/mod/core.html#limitrequestfields
+const int MAX_HEADERLINE_LENGTH = 8 * 1024; //maximum length of a http request header line
+const int MAX_HEADERLINES = 100; //maximum number of http request header lines
+
/*!
\internal
*/
@@ -431,7 +437,7 @@ void QWebSocketServerPrivate::handshakeReceived()
QWebSocketHandshakeRequest request(pTcpSocket->peerPort(), isSecure);
QTextStream textStream(pTcpSocket);
- request.readHandshake(textStream);
+ request.readHandshake(textStream, MAX_HEADERLINE_LENGTH, MAX_HEADERLINES);
if (request.isValid()) {
QWebSocketCorsAuthenticator corsAuthenticator(request.origin());
diff --git a/tests/auto/websockets/handshakerequest/tst_handshakerequest.cpp b/tests/auto/websockets/handshakerequest/tst_handshakerequest.cpp
index 9c579bd..2921e57 100644
--- a/tests/auto/websockets/handshakerequest/tst_handshakerequest.cpp
+++ b/tests/auto/websockets/handshakerequest/tst_handshakerequest.cpp
@@ -45,6 +45,9 @@ QT_USE_NAMESPACE
Q_DECLARE_METATYPE(QWebSocketProtocol::CloseCode)
Q_DECLARE_METATYPE(QWebSocketProtocol::OpCode)
+const int MAX_HEADERLINE_LENGTH = 8 * 1024;
+const int MAX_HEADERS = 100;
+
class tst_HandshakeRequest : public QObject
{
Q_OBJECT
@@ -67,6 +70,8 @@ private Q_SLOTS:
void tst_multipleVersions();
void tst_qtbug_39355();
+ void tst_qtbug_48123_data();
+ void tst_qtbug_48123();
};
tst_HandshakeRequest::tst_HandshakeRequest()
@@ -203,7 +208,7 @@ void tst_HandshakeRequest::tst_invalidStream()
textStream << dataStream;
textStream.seek(0);
- request.readHandshake(textStream);
+ request.readHandshake(textStream, MAX_HEADERLINE_LENGTH, MAX_HEADERS);
QVERIFY(!request.isValid());
QCOMPARE(request.port(), 80);
@@ -239,7 +244,7 @@ void tst_HandshakeRequest::tst_multipleValuesInConnectionHeader()
textStream << header;
textStream.seek(0);
- request.readHandshake(textStream);
+ request.readHandshake(textStream, MAX_HEADERLINE_LENGTH, MAX_HEADERS);
QVERIFY(request.isValid());
QCOMPARE(request.port(), 80);
@@ -269,7 +274,7 @@ void tst_HandshakeRequest::tst_multipleVersions()
textStream << header;
textStream.seek(0);
- request.readHandshake(textStream);
+ request.readHandshake(textStream, MAX_HEADERLINE_LENGTH, MAX_HEADERS);
QVERIFY(request.isValid());
QCOMPARE(request.port(), 80);
@@ -305,13 +310,76 @@ void tst_HandshakeRequest::tst_qtbug_39355()
textStream << header;
textStream.seek(0);
- request.readHandshake(textStream);
+ request.readHandshake(textStream, MAX_HEADERLINE_LENGTH, MAX_HEADERS);
QVERIFY(request.isValid());
QCOMPARE(request.port(), 1234);
QCOMPARE(request.host(), QStringLiteral("localhost"));
}
+void tst_HandshakeRequest::tst_qtbug_48123_data()
+{
+ QTest::addColumn<QString>("header");
+ QTest::addColumn<bool>("shouldBeValid");
+ const QString header = QStringLiteral("GET /ABC/DEF/ HTTP/1.1\r\nHost: localhost:1234\r\n") +
+ QStringLiteral("Sec-WebSocket-Version: 13\r\n") +
+ QStringLiteral("Sec-WebSocket-Key: 2Wg20829/4ziWlmsUAD8Dg==\r\n") +
+ QStringLiteral("Upgrade: websocket\r\n") +
+ QStringLiteral("Connection: Upgrade\r\n");
+ const int numHeaderLines = header.count(QStringLiteral("\r\n")) - 1; //-1: exclude requestline
+
+ //a headerline should not be larger than MAX_HEADERLINE_LENGTH characters (excluding CRLF)
+ QString illegalHeader = header;
+ illegalHeader.append(QString(MAX_HEADERLINE_LENGTH + 1, QChar::fromAscii('c')));
+ illegalHeader.append(QStringLiteral("\r\n\r\n"));
+
+ QTest::newRow("headerline too long") << illegalHeader << false;
+
+ QString legalHeader = header;
+ const QString headerKey = QStringLiteral("X-CUSTOM-KEY: ");
+ legalHeader.append(headerKey);
+ legalHeader.append(QString(MAX_HEADERLINE_LENGTH - headerKey.length(), QChar::fromAscii('c')));
+ legalHeader.append(QStringLiteral("\r\n\r\n"));
+
+ QTest::newRow("headerline with maximum length") << legalHeader << true;
+
+ //a header should not contain more than MAX_HEADERS header lines (excluding the request line)
+ //test with MAX_HEADERS + 1
+ illegalHeader = header;
+ const QString headerLine(QStringLiteral("Host: localhost:1234\r\n"));
+ for (int i = 0; i < (MAX_HEADERS - numHeaderLines + 1); ++i) {
+ illegalHeader.append(headerLine);
+ }
+ illegalHeader.append(QStringLiteral("\r\n"));
+
+ QTest::newRow("too many headerlines") << illegalHeader << false;
+
+ //test with MAX_HEADERS header lines (excluding the request line)
+ legalHeader = header;
+ for (int i = 0; i < (MAX_HEADERS - numHeaderLines); ++i) {
+ legalHeader.append(headerLine);
+ }
+ legalHeader.append(QStringLiteral("\r\n"));
+
+ QTest::newRow("just enough headerlines") << legalHeader << true;
+}
+
+void tst_HandshakeRequest::tst_qtbug_48123()
+{
+ QFETCH(QString, header);
+ QFETCH(bool, shouldBeValid);
+
+ QByteArray data;
+ QTextStream textStream(&data);
+ QWebSocketHandshakeRequest request(8080, false);
+
+ textStream << header;
+ textStream.seek(0);
+ request.readHandshake(textStream, MAX_HEADERLINE_LENGTH, MAX_HEADERS);
+
+ QCOMPARE(request.isValid(), shouldBeValid);
+}
+
QTEST_MAIN(tst_HandshakeRequest)
#include "tst_handshakerequest.moc"
diff --git a/tests/auto/websockets/handshakeresponse/tst_handshakeresponse.cpp b/tests/auto/websockets/handshakeresponse/tst_handshakeresponse.cpp
index b5f103b..7d35cd0 100644
--- a/tests/auto/websockets/handshakeresponse/tst_handshakeresponse.cpp
+++ b/tests/auto/websockets/handshakeresponse/tst_handshakeresponse.cpp
@@ -91,7 +91,7 @@ void tst_HandshakeResponse::tst_date_response()
QStringLiteral("Sec-WebSocket-Key: AVDFBDDFF\r\n") +
QStringLiteral("Upgrade: websocket\r\n") +
QStringLiteral("Connection: Upgrade\r\n\r\n");
- request.readHandshake(input);
+ request.readHandshake(input, 8 * 1024, 100);
QWebSocketHandshakeResponse response(request, "example.com", true,
QList<QWebSocketProtocol::Version>() << QWebSocketProtocol::Version13,