From f2b58e48514d0c75509d86c06ad6321496904bdb Mon Sep 17 00:00:00 2001 From: Mate Barany Date: Fri, 12 Jun 2026 16:47:55 +0200 Subject: [PATCH] QAuthenticator: harden qExtractServerTime against malformed input Replace QDataStream with direct pointer arithmetic and qFromLittleEndian for walking AV_PAIR structures. Add bounds checks before every pointer advance, validate that MsvAvTimestamp is exactly 8 bytes per MS-NLMP section 2.2.2.1. Malformed or truncated timestamps cause the function to return an empty QByteArray, which makes the existing caller fall back to the current system time — the same path taken when the server omits the timestamp entirely. This preserves the current behaviour rather than failing the authentication outright. Add test cases for malformed targetInfo inputs (wrong avLen, truncated buffer, too-short header, avLen exceeding buffer, empty buffer) to verify no crash and that the fallback path produces a valid Phase 3 response. Pick-to: 6.5 5.15 Task-number: QTBUG-147529 Change-Id: I5bdb4e4440460d0770f5d145238c4db39cad06e3 Reviewed-by: Mårten Nordheim Reviewed-by: Thiago Macieira (cherry picked from commit eb91758e006472b0f40304219aae09e69564c0fa) Reviewed-by: Qt Cherry-pick Bot (cherry picked from commit be08785a74d60dcce75461d8e11646173f0c622c) (cherry picked from commit 3233502337d5e23dad2c3df7d8afa81c506c731f) --- diff --git a/src/network/kernel/qauthenticator.cpp b/src/network/kernel/qauthenticator.cpp index c1a8766..41fbd29 100644 --- a/src/network/kernel/qauthenticator.cpp +++ b/src/network/kernel/qauthenticator.cpp @@ -999,6 +999,8 @@ const int blockSize = 64; //As per RFC2104 Block-size is 512 bits const quint8 respversion = 1; const quint8 hirespversion = 1; +// FILETIME: two 32-bit values = 8 bytes (MS-DTYP section 2.3.3) +static constexpr quint16 NtlmFileTimeSize = 8; /* usage: // fill up ctx with what we know. @@ -1325,24 +1327,29 @@ static QByteArray qExtractServerTime(const QByteArray& targetInfoBuff) { QByteArray timeArray; - QDataStream ds(targetInfoBuff); - ds.setByteOrder(QDataStream::LittleEndian); - + const char *ptr = targetInfoBuff.constBegin(); + const char *end = targetInfoBuff.constEnd(); quint16 avId; quint16 avLen; - ds >> avId; - ds >> avLen; - while(avId != 0) { + while (end - ptr >= 4) { + avId = qFromLittleEndian(ptr + 0); + avLen = qFromLittleEndian(ptr + 2); + ptr += 4; + if (avId == AVTIMESTAMP) { - timeArray.resize(avLen); - //avLen size of QByteArray is allocated - ds.readRawData(timeArray.data(), avLen); + if (avLen != NtlmFileTimeSize) + break; + if (end - ptr < NtlmFileTimeSize) + break; + + timeArray.assign(ptr, ptr + NtlmFileTimeSize); break; } - ds.skipRawData(avLen); - ds >> avId; - ds >> avLen; + + if (avLen > end - ptr) + break; + ptr += avLen; } return timeArray; } diff --git a/tests/auto/network/kernel/qauthenticator/tst_qauthenticator.cpp b/tests/auto/network/kernel/qauthenticator/tst_qauthenticator.cpp index 1cd1b6a..05e6346 100644 --- a/tests/auto/network/kernel/qauthenticator/tst_qauthenticator.cpp +++ b/tests/auto/network/kernel/qauthenticator/tst_qauthenticator.cpp @@ -29,6 +29,11 @@ void equalityOperators(); void isMethodSupported(); + + void ntlmV2KnownVectors_data(); + void ntlmV2KnownVectors(); + void ntlmMalformedTargetInfo_data(); + void ntlmMalformedTargetInfo(); }; tst_QAuthenticator::tst_QAuthenticator() @@ -188,6 +193,268 @@ QVERIFY(!QAuthenticatorPrivate::isMethodSupported("Bearer")); } +void tst_QAuthenticator::ntlmV2KnownVectors_data() +{ + // Verifies the NTLMv2 and LMv2 crypto against the reference test vectors + // from the Microsoft MS-NLMP specification, Section 4.2.4. + // + // All inputs and expected outputs are taken directly from the spec: + // User: "User" + // Domain: "Domain" + // Password: "Password" + // Server challenge: 01 23 45 67 89 ab cd ef + // Client challenge: aa aa aa aa aa aa aa aa (fixed via cnonce below) + // + // Two test rows are provided: + // ms-nlmp-sec-4.2.4 — blob with targetInfo + timestamp, verifies NTProofStr + // ms-nlmp-sec-4.2.4-no-ti — blob without targetInfo, verifies LMv2 response + // + // The Phase 2 blobs are derived from the Chromium NTLM test suite + // (net/ntlm/ntlm_test_data.h), which is itself derived from MS-NLMP + // Section 4.2.4.3. Using independently verified blobs eliminates the risk + // of a hand-constructed blob causing a false test failure. + + QTest::addColumn("phase2blob"); + QTest::addColumn("expectedNtProofStr"); + QTest::addColumn("expectedLmv2Prefix"); + + // --- Row 1: blob with targetInfo + timestamp --- + // Based on kChallengeMsgFromSpecV2 from Chromium net/ntlm/ntlm_test_data.h + // (MS-NLMP Section 4.2.4.3), with MsvAvTimestamp=0 added to targetInfo. + // The timestamp must be present so qExtractServerTime() returns it instead + // of falling back to QDateTime::currentSecsSinceEpoch(), which would make + // the output non-deterministic. The spec uses kServerTimestamp=0. + // When targetInfo is present, LMv2 is intentionally omitted per spec — + // so expectedLmv2Prefix is empty for this row. + QByteArray phase2WithTi = QByteArray::fromHex( + "4e544c4d53535000" // signature "NTLMSSP\0" + "02000000" // type = 2 + "0c000c0038000000" // targetName: len=12, maxLen=12, offset=56 + "33828ae2" // flags + "0123456789abcdef" // server challenge + "0000000000000000" // context (reserved) + "3000300044000000" // targetInfo: len=48, maxLen=48, offset=68 + "060070170000000f" // OS version (ignored by parser) + "530065007200760065007200" // targetName = "Server" UCS-2LE + "02000c0044006f006d00610069006e00" // AvId=2 (MsvAvNbDomainName) "Domain" + "01000c00530065007200760065007200" // AvId=1 (MsvAvNbComputerName) "Server" + "070008000000000000000000" // AvId=7 (MsvAvTimestamp) = 0 + "00000000" // AvId=0 MsvAvEOL + ); + + // NTProofStr recalculated with MsvAvTimestamp=0 in targetInfo. + // Verified independently with Python/pycryptodome. + QByteArray ntProofStrWithTi = + QByteArray::fromHex("fb4ec58d662a1ebc3b5524971d77207e"); + + QTest::newRow("ms-nlmp-sec-4.2.4") + << phase2WithTi << ntProofStrWithTi << QByteArray(); + + // --- Row 2: blob without targetInfo --- + // kChallengeMsgV1 from Chromium net/ntlm/ntlm_test_data.h + // (MS-NLMP Section 4.2.3.3). No targetInfo means qNtlmPhase3 will compute + // the LMv2 response instead of omitting it. NTProofStr is not verifiable + // for this blob because there is no timestamp in targetInfo, so the code + // falls back to QDateTime::currentSecsSinceEpoch() which is non-deterministic. + QByteArray phase2NoTi = QByteArray::fromHex( + "4e544c4d53535000" // signature "NTLMSSP\0" + "02000000" // type = 2 + "0c000c0038000000" // targetName: len=12, maxLen=12, offset=56 + "33820a82" // flags (no NTLMSSP_NEGOTIATE_TARGET_INFO) + "0123456789abcdef" // server challenge + "0000000000000000" // context (reserved) + "0000000000000000" // targetInfo: len=0 (absent) + "060070170000000f" // OS version (ignored by parser) + "530065007200760065007200" // targetName = "Server" UCS-2LE + ); + + // Expected LMv2 response prefix (MS-NLMP Section 4.2.4.2.1): + // HMAC-MD5(v2Hash, server_challenge + client_challenge). + // Verified independently with Python/pycryptodome. + QByteArray lmv2Prefix = + QByteArray::fromHex("86c35097ac9cec102554764a57cccc19"); + + QTest::newRow("ms-nlmp-sec-4.2.4-no-ti") + << phase2NoTi << QByteArray() << lmv2Prefix; +} + +void tst_QAuthenticator::ntlmV2KnownVectors() +{ + QFETCH(QByteArray, phase2blob); + QFETCH(QByteArray, expectedNtProofStr); + QFETCH(QByteArray, expectedLmv2Prefix); + + QAuthenticator auth; + auth.setUser("Domain\\User"); // triggers domain/user split in updateCredentials() + auth.setPassword("Password"); + + auth.detach(); + QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(auth); + + // Fix the cnonce so the output is deterministic and matches the spec. + // clientChallenge() takes the last 8 bytes of cnonce directly as raw bytes, + // so we set them to 0xaa (the MS-NLMP spec client challenge value). + // The leading padding satisfies the Q_ASSERT(cnonce.size() >= 8) check. + priv->cnonce = QByteArray(32, 'x') + QByteArray("\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa", 8); + + // --- Phase 1: negotiate --- + QHttpHeaders headers; + headers.append(QByteArrayLiteral("WWW-Authenticate"), QByteArrayLiteral("NTLM")); + priv->parseHttpResponse(headers, /*isProxy=*/false, {}); + priv->calculateResponse("GET", "/", u""); // discard — only needed to advance state + + // --- Phase 2: challenge --- + headers.clear(); + headers.append(QByteArray("WWW-Authenticate"), + "NTLM " + phase2blob.toBase64()); + priv->parseHttpResponse(headers, /*isProxy=*/false, {}); + + // --- Phase 3: authenticate --- + QByteArray response = priv->calculateResponse("GET", "/", u""); + + // Basic sanity — if the blob was rejected by the parser this will fail, + // telling us immediately that the problem is in the test setup, not the crypto. + QVERIFY2(response.startsWith("NTLM "), "Phase 2 blob was rejected by qNtlmDecodePhase2"); + + // Decode the Phase 3 AUTHENTICATE_MESSAGE from base64 + QByteArray phase3 = QByteArray::fromBase64(response.mid(5)); + QVERIFY(phase3.size() >= 64); // minimum AUTHENTICATE_MESSAGE header size + + // Read the buffer descriptors from the AUTHENTICATE_MESSAGE header. + // Layout (all little-endian): + // offset 0: signature (8 bytes) + // offset 8: type (4 bytes) + // offset 12: LmChallengeResponse descriptor (len u16, maxLen u16, offset u32) + // offset 20: NtChallengeResponse descriptor (len u16, maxLen u16, offset u32) + QDataStream ds(phase3); + ds.setByteOrder(QDataStream::LittleEndian); + ds.skipRawData(12); // skip signature + type + + quint16 lmLen, lmMaxLen; quint32 lmOffset; + ds >> lmLen >> lmMaxLen >> lmOffset; + + quint16 ntLen, ntMaxLen; quint32 ntOffset; + ds >> ntLen >> ntMaxLen >> ntOffset; + + // --- Verify LMv2 response --- + // When targetInfo is present the LMv2 response is intentionally omitted + // per MS-NLMP spec — expectedLmv2Prefix is empty for those rows. + if (!expectedLmv2Prefix.isEmpty()) { + QVERIFY(qsizetype(lmOffset + lmLen) <= phase3.size()); + QByteArray lmResponse = phase3.mid(lmOffset, lmLen); + QCOMPARE(lmResponse.size(), 24); // 16 bytes HMAC + 8 bytes client challenge + QCOMPARE(lmResponse.left(16), expectedLmv2Prefix); + QCOMPARE(lmResponse.right(8), QByteArray("\xaa\xaa\xaa\xaa\xaa\xaa\xaa\xaa", 8)); + } + + // --- Verify NTLMv2 response --- + // When there is no timestamp in targetInfo the code falls back to + // currentSecsSinceEpoch() making NTProofStr non-deterministic — + // expectedNtProofStr is empty for those rows. + if (!expectedNtProofStr.isEmpty()) { + QVERIFY(qsizetype(ntOffset + ntLen) <= phase3.size()); + QByteArray ntResponse = phase3.mid(ntOffset, ntLen); + QVERIFY(ntResponse.size() >= 16); // NTProofStr is always the first 16 bytes + QCOMPARE(ntResponse.left(16), expectedNtProofStr); + } +} + +void tst_QAuthenticator::ntlmMalformedTargetInfo_data() +{ + QTest::addColumn("targetInfo"); + + // Reusable well-formed AV_PAIR components + QByteArray avDomain = QByteArray::fromHex( + "02000c0044006f006d00610069006e00"); // AvId=2, len=12, "Domain" UCS-2LE + QByteArray avServer = QByteArray::fromHex( + "01000c00530065007200760065007200"); // AvId=1, len=12, "Server" UCS-2LE + QByteArray avEol = QByteArray::fromHex("00000000"); + + // AVTIMESTAMP with wrong avLen (4 instead of 8) + QTest::newRow("timestamp-wrong-avlen") + << avDomain + avServer + + QByteArray::fromHex("07000400" "00000000") + + avEol; + + // Buffer truncated mid-AV_PAIR: timestamp header present but data cut short + { + QByteArray full = avDomain + avServer + + QByteArray::fromHex("070008000000000000000000") + avEol; + QTest::newRow("truncated-mid-avpair") + << full.left(full.size() - 6); + } + + // Too short for even one AV_PAIR header (only 2 bytes) + QTest::newRow("too-short-for-header") + << QByteArray::fromHex("0700"); + + // avLen exceeds remaining buffer + QTest::newRow("avlen-exceeds-buffer") + << QByteArray::fromHex("0200FF00") + + QByteArray::fromHex("44006f006d00610069006e00") + + avEol; + + // Empty targetInfo + QTest::newRow("empty-targetinfo") + << QByteArray(); +} + +void tst_QAuthenticator::ntlmMalformedTargetInfo() +{ + QFETCH(QByteArray, targetInfo); + + // Build a Phase 2 challenge blob wrapping the test targetInfo + QByteArray targetName = QByteArray::fromHex( + "530065007200760065007200"); // "Server" UCS-2LE + quint16 targetNameLen = targetName.size(); + quint16 targetInfoLen = targetInfo.size(); + quint32 targetNameOffset = 56; // 48 (min header) + 8 (version) + quint32 targetInfoOffset = targetNameOffset + targetNameLen; + + QByteArray phase2; + QDataStream ds(&phase2, QIODevice::WriteOnly); + ds.setByteOrder(QDataStream::LittleEndian); + + ds.writeRawData("NTLMSSP\0", 8); // signature + ds << quint32(2); // type + ds << targetNameLen << targetNameLen << targetNameOffset; // targetName descriptor + ds << quint32(0xe28a8233); // flags + ds.writeRawData("\x01\x23\x45\x67\x89\xab\xcd\xef", 8); // server challenge + ds << quint32(0) << quint32(0); // context + ds << targetInfoLen << targetInfoLen << targetInfoOffset; // targetInfo descriptor + ds.writeRawData("\x06\x00\x70\x17\x00\x00\x00\x0f", 8); // version + ds.writeRawData(targetName.constData(), targetName.size()); + if (!targetInfo.isEmpty()) + ds.writeRawData(targetInfo.constData(), targetInfo.size()); + + // Run the full NTLM auth flow + QAuthenticator auth; + auth.setUser("Domain\\User"); + auth.setPassword("Password"); + auth.detach(); + + QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(auth); + + // Phase 1: negotiate + QHttpHeaders headers; + headers.append(QByteArrayLiteral("WWW-Authenticate"), QByteArrayLiteral("NTLM")); + priv->parseHttpResponse(headers, false, {}); + priv->calculateResponse("GET", "/", u""); + + // Phase 2: challenge with malformed targetInfo + headers.clear(); + headers.append(QByteArray("WWW-Authenticate"), "NTLM " + phase2.toBase64()); + priv->parseHttpResponse(headers, false, {}); + + // Phase 3: authenticate — must not crash, must produce a response + QByteArray response = priv->calculateResponse("GET", "/", u""); + + QVERIFY2(response.startsWith("NTLM "), + "Auth flow should complete despite malformed targetInfo"); + QByteArray phase3 = QByteArray::fromBase64(response.mid(5)); + QVERIFY(phase3.size() >= 64); // minimum AUTHENTICATE_MESSAGE header size +} + QTEST_MAIN(tst_QAuthenticator); #include "tst_qauthenticator.moc"