From 586139be41d5370196a3d32534634369ec97fbf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Thu, 28 Jul 2022 12:04:11 +0200 Subject: [PATCH] QAuthenticator: Fix crash when using NTLM / Negotiate With NTLM/Negotiate we delete the context used to generate replies once we get SEC_E_OK. Due to some faulty logic in the http backend we could end up trying to generate another response. Qt would then pass references to some offsets of nullptr into the API calls causing it to crash. Add some sanity checks before the "sspi continue" calls to make sure this won't happen, and update the condition in the http backend to check that we have not already sent our credentials. As a drive-by: correct the initialization of the handles to use SecInvalidateHandle instead of memset to 0. Fixes: QTBUG-102359 Change-Id: I884ff8fc70609fe8746b99a1d56eeafcda9d2620 --- src/network/access/qhttpnetworkconnection.cpp | 35 ++++++++++++++----- src/network/kernel/qauthenticator.cpp | 9 +++-- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp index 9745f3b322..647c9b024e 100644 --- a/src/network/access/qhttpnetworkconnection.cpp +++ b/src/network/access/qhttpnetworkconnection.cpp @@ -597,10 +597,20 @@ void QHttpNetworkConnectionPrivate::createAuthorization(QAbstractSocket *socket, // Send "Authorization" header, but not if it's NTLM and the socket is already authenticated. if (channels[i].authMethod != QAuthenticatorPrivate::None) { - if ((channels[i].authMethod != QAuthenticatorPrivate::Ntlm && request.headerField("Authorization").isEmpty()) || channels[i].lastStatus == 401) { - QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(channels[i].authenticator); - if (priv && priv->method != QAuthenticatorPrivate::None) { - QByteArray response = priv->calculateResponse(request.methodName(), request.uri(false), request.url().host()); + QAuthenticatorPrivate *priv = + QAuthenticatorPrivate::getPrivate(channels[i].authenticator); + if (priv && priv->method != QAuthenticatorPrivate::None) { + const bool ntlmNego = priv->method == QAuthenticatorPrivate::Ntlm + || priv->method == QAuthenticatorPrivate::Negotiate; + const bool authNeeded = channels[i].lastStatus == 401; + const bool ntlmNegoOk = ntlmNego && authNeeded + && (priv->phase != QAuthenticatorPrivate::Done + || !channels[i].authenticationCredentialsSent); + const bool otherOk = + !ntlmNego && (authNeeded || request.headerField("Authorization").isEmpty()); + if (ntlmNegoOk || otherOk) { + QByteArray response = priv->calculateResponse( + request.methodName(), request.uri(false), request.url().host()); request.setHeaderField("Authorization", response); channels[i].authenticationCredentialsSent = true; } @@ -610,10 +620,19 @@ void QHttpNetworkConnectionPrivate::createAuthorization(QAbstractSocket *socket, #if QT_CONFIG(networkproxy) // Send "Proxy-Authorization" header, but not if it's NTLM and the socket is already authenticated. if (channels[i].proxyAuthMethod != QAuthenticatorPrivate::None) { - if (!(channels[i].proxyAuthMethod == QAuthenticatorPrivate::Ntlm && channels[i].lastStatus != 407)) { - QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(channels[i].proxyAuthenticator); - if (priv && priv->method != QAuthenticatorPrivate::None) { - QByteArray response = priv->calculateResponse(request.methodName(), request.uri(false), networkProxy.hostName()); + QAuthenticatorPrivate *priv = + QAuthenticatorPrivate::getPrivate(channels[i].proxyAuthenticator); + if (priv && priv->method != QAuthenticatorPrivate::None) { + const bool ntlmNego = channels[i].proxyAuthMethod == QAuthenticatorPrivate::Ntlm + || channels[i].proxyAuthMethod == QAuthenticatorPrivate::Negotiate; + const bool proxyAuthNeeded = channels[i].lastStatus == 407; + const bool ntlmNegoOk = ntlmNego && proxyAuthNeeded + && (priv->phase != QAuthenticatorPrivate::Done + || !channels[i].proxyCredentialsSent); + const bool otherOk = !ntlmNego; + if (ntlmNegoOk || otherOk) { + QByteArray response = priv->calculateResponse( + request.methodName(), request.uri(false), networkProxy.hostName()); request.setHeaderField("Proxy-Authorization", response); channels[i].proxyCredentialsSent = true; } diff --git a/src/network/kernel/qauthenticator.cpp b/src/network/kernel/qauthenticator.cpp index 86242b011f..690921d46e 100644 --- a/src/network/kernel/qauthenticator.cpp +++ b/src/network/kernel/qauthenticator.cpp @@ -599,9 +599,11 @@ QByteArray QAuthenticatorPrivate::calculateResponse(const QByteArray &requestMet } else { QByteArray phase3Token; #if QT_CONFIG(sspi) // SSPI - phase3Token = qSspiContinue(this, method, host, QByteArray::fromBase64(challenge)); + if (sspiWindowsHandles) + phase3Token = qSspiContinue(this, method, host, QByteArray::fromBase64(challenge)); #elif QT_CONFIG(gssapi) // GSSAPI - phase3Token = qGssapiContinue(this, QByteArray::fromBase64(challenge)); + if (gssApiHandles) + phase3Token = qGssapiContinue(this, QByteArray::fromBase64(challenge)); #endif if (!phase3Token.isEmpty()) { response = phase3Token.toBase64(); @@ -1568,7 +1570,8 @@ static QByteArray qSspiStartup(QAuthenticatorPrivate *ctx, QAuthenticatorPrivate if (!ctx->sspiWindowsHandles) ctx->sspiWindowsHandles.reset(new QSSPIWindowsHandles); - memset(&ctx->sspiWindowsHandles->credHandle, 0, sizeof(CredHandle)); + SecInvalidateHandle(&ctx->sspiWindowsHandles->credHandle); + SecInvalidateHandle(&ctx->sspiWindowsHandles->ctxHandle); SEC_WINNT_AUTH_IDENTITY auth; auth.Flags = SEC_WINNT_AUTH_IDENTITY_UNICODE; -- 2.37.1.windows.1