#3221: Crash reported on Windows in X509Certificate verification

pocoproject · Nov 5, 2021 · 270c264 · 270c264
1 parent 5902bb1
commit 270c264
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 6 deletions.
diff --git a/NetSSL_Win/src/SecureSocketImpl.cpp b/NetSSL_Win/src/SecureSocketImpl.cpp
@@ -1317,7 +1317,7 @@ void SecureSocketImpl::verifyCertificateChainClient(PCCERT_CONTEXT pServerCert)
 
 			// Revocation check of the root certificate may fail due to missing CRL points, etc.
 			// We ignore all errors checking the root certificate except CRYPT_E_REVOKED.
-			if (!ok && (revStat.dwIndex < certs.size() - 1 || revStat.dwError == CRYPT_E_REVOKED))
+			if (!ok && revStat.dwIndex < certs.size() - 1 && revStat.dwError == CRYPT_E_REVOKED)
 			{
 				VerificationErrorArgs args(cert, revStat.dwIndex, revStat.dwReason, Utility::formatError(revStat.dwError));
 				SSLManager::instance().ClientVerificationError(this, args);
@@ -1421,7 +1421,10 @@ void SecureSocketImpl::serverVerifyCertificate()
 						CERT_VERIFY_REV_CHAIN_FLAG,
 						NULL,
 						&revStat);
-		if (!ok && (revStat.dwIndex < certs.size() - 1 || revStat.dwError == CRYPT_E_REVOKED))
+
+		// Revocation check of the root certificate may fail due to missing CRL points, etc.
+		// We ignore all errors checking the root certificate except CRYPT_E_REVOKED.
+		if (!ok && revStat.dwIndex < certs.size() - 1 && revStat.dwError == CRYPT_E_REVOKED)
 		{
 			VerificationErrorArgs args(cert, revStat.dwIndex, revStat.dwReason, Utility::formatError(revStat.dwReason));
 			SSLManager::instance().ServerVerificationError(this, args);

diff --git a/NetSSL_Win/src/X509Certificate.cpp b/NetSSL_Win/src/X509Certificate.cpp
@@ -278,10 +278,14 @@ void X509Certificate::extractNames(std::string& cmnName, std::set<std::string>&
 				PCERT_ALT_NAME_INFO pNameInfo = reinterpret_cast<PCERT_ALT_NAME_INFO>(buffer.begin());
 				for (int i = 0; i < pNameInfo->cAltEntry; i++)
 				{
-					std::wstring waltName(pNameInfo->rgAltEntry[i].pwszDNSName);
-					std::string altName;
-					Poco::UnicodeConverter::toUTF8(waltName, altName);
-					domainNames.insert(altName);
+					// Some certificates have Subject Alternative Name entries that are not DNS Name. Skip them.
+					if (pNameInfo->rgAltEntry[i].dwAltNameChoice == CERT_ALT_NAME_DNS_NAME)
+					{
+						std::wstring waltName(pNameInfo->rgAltEntry[i].pwszDNSName);
+						std::string altName;
+						Poco::UnicodeConverter::toUTF8(waltName, altName);
+						domainNames.insert(altName);
+					}
 				}
 			}
 		}