From 0f7390e410758c335e3dfc995e7126598da06112 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 Reviewed-by: Qt CI Bot Reviewed-by: Edward Welbourne (cherry picked from commit c684b8e939650b5c007b990c5c39eea750f45970) Reviewed-by: Qt Cherry-pick Bot --- src/network/access/qhttpnetworkconnection.cpp | 20 +++++++++++++++---- src/network/kernel/qauthenticator.cpp | 9 ++++++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp index d82fb46356f..3b08a84a03f 100644 --- a/src/network/access/qhttpnetworkconnection.cpp +++ b/src/network/access/qhttpnetworkconnection.cpp @@ -570,9 +570,15 @@ void QHttpNetworkConnectionPrivate::createAuthorization(QAbstractSocket *socket, QAuthenticatorPrivate *priv = QAuthenticatorPrivate::getPrivate(*authenticator); // Send "Authorization" header, but not if it's NTLM and the socket is already authenticated. if (priv && priv->method != QAuthenticatorPrivate::None) { - if ((priv->method != QAuthenticatorPrivate::Ntlm - && request.headerField("Authorization").isEmpty()) - || channel.lastStatus == 401) { + const bool ntlmNego = priv->method == QAuthenticatorPrivate::Ntlm + || priv->method == QAuthenticatorPrivate::Negotiate; + const bool authNeeded = channel.lastStatus == 401; + const bool ntlmNegoOk = ntlmNego && authNeeded + && (priv->phase != QAuthenticatorPrivate::Done + || !channel.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); @@ -585,7 +591,13 @@ void QHttpNetworkConnectionPrivate::createAuthorization(QAbstractSocket *socket, priv = QAuthenticatorPrivate::getPrivate(*authenticator); // Send "Proxy-Authorization" header, but not if it's NTLM and the socket is already authenticated. if (priv && priv->method != QAuthenticatorPrivate::None) { - if (priv->method != QAuthenticatorPrivate::Ntlm || channel.lastStatus == 407) { + const bool ntlmNego = priv->method == QAuthenticatorPrivate::Ntlm + || priv->method == QAuthenticatorPrivate::Negotiate; + const bool proxyAuthNeeded = channel.lastStatus == 407; + const bool ntlmNegoOk = ntlmNego && proxyAuthNeeded + && (priv->phase != QAuthenticatorPrivate::Done || !channel.proxyCredentialsSent); + const bool otherOk = !ntlmNego; + if (ntlmNegoOk || otherOk) { QByteArray response = priv->calculateResponse(request.methodName(), request.uri(false), networkProxy.hostName()); request.setHeaderField("Proxy-Authorization", response); diff --git a/src/network/kernel/qauthenticator.cpp b/src/network/kernel/qauthenticator.cpp index 3b8aacebeba..4b9ed021be8 100644 --- a/src/network/kernel/qauthenticator.cpp +++ b/src/network/kernel/qauthenticator.cpp @@ -626,9 +626,11 @@ QByteArray QAuthenticatorPrivate::calculateResponse(QByteArrayView requestMethod } 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(); @@ -1583,7 +1585,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;