From 701735f8d72ed4e07d7a00eb13e2f1f9f12356d5 Mon Sep 17 00:00:00 2001
From: Timur Pocheptsov <timur.pocheptsov@qt.io>
Date: Mon, 21 Dec 2020 14:19:36 +0100
Subject: QSslCertificate(OpenSSL) - harden protection against nullpointers

An invalid (as input data) certificate may have non-zero number of invalid (nullptr)
extensions (if OpenSSL failed to parse them, for example).

Fixes: QTBUG-89547
Change-Id: I4b93ac9f482f850f02d01b0aea10560dc11b688d
Reviewed-by: Lars Schmertmann <lars.schmertmann@governikus.de>
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>
(cherry picked from commit f31997448838902eb5237b567f0c80f423f2406e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/network/ssl/qsslcertificate_openssl.cpp | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/src/network/ssl/qsslcertificate_openssl.cpp b/src/network/ssl/qsslcertificate_openssl.cpp
index fa87cfeaaf..baf3e4accf 100644
--- a/src/network/ssl/qsslcertificate_openssl.cpp
+++ b/src/network/ssl/qsslcertificate_openssl.cpp
@@ -294,9 +294,12 @@ static QVariant x509UnknownExtensionToValue(X509_EXTENSION *ext)
     // we cast away the const-ness here because some versions of openssl
     // don't use const for the parameters in the functions pointers stored
     // in the object.
+    Q_ASSERT(ext);
+
     X509V3_EXT_METHOD *meth = const_cast<X509V3_EXT_METHOD *>(q_X509V3_EXT_get(ext));
     if (!meth) {
         ASN1_OCTET_STRING *value = q_X509_EXTENSION_get_data(ext);
+        Q_ASSERT(value);
         QByteArray result( reinterpret_cast<const char *>(q_ASN1_STRING_get0_data(value)),
                            q_ASN1_STRING_length(value));
         return result;
@@ -330,7 +333,6 @@ static QVariant x509UnknownExtensionToValue(X509_EXTENSION *ext)
         else
             return list;
     } else if (meth->i2s && ext_internal) {
-        //qCDebug(lcSsl) << meth->i2s(meth, ext_internal);
         QVariant result(QString::fromUtf8(meth->i2s(meth, ext_internal)));
         return result;
     } else if (meth->i2r && ext_internal) {
@@ -367,6 +369,8 @@ static QVariant x509ExtensionToValue(X509_EXTENSION *ext)
     case NID_basic_constraints:
         {
             BASIC_CONSTRAINTS *basic = reinterpret_cast<BASIC_CONSTRAINTS *>(q_X509V3_EXT_d2i(ext));
+            if (!basic)
+                return QVariant();
 
             QVariantMap result;
             result[QLatin1String("ca")] = basic->ca ? true : false;
@@ -380,6 +384,8 @@ static QVariant x509ExtensionToValue(X509_EXTENSION *ext)
     case NID_info_access:
         {
             AUTHORITY_INFO_ACCESS *info = reinterpret_cast<AUTHORITY_INFO_ACCESS *>(q_X509V3_EXT_d2i(ext));
+            if (!info)
+                return QVariant();
 
             QVariantMap result;
             for (int i=0; i < q_SKM_sk_num(ACCESS_DESCRIPTION, info); i++) {
@@ -409,7 +415,8 @@ static QVariant x509ExtensionToValue(X509_EXTENSION *ext)
     case NID_subject_key_identifier:
         {
             void *ext_internal = q_X509V3_EXT_d2i(ext);
-
+            if (!ext_internal)
+                return QVariant();
             // we cast away the const-ness here because some versions of openssl
             // don't use const for the parameters in the functions pointers stored
             // in the object.
@@ -421,6 +428,8 @@ static QVariant x509ExtensionToValue(X509_EXTENSION *ext)
     case NID_authority_key_identifier:
         {
             AUTHORITY_KEYID *auth_key = reinterpret_cast<AUTHORITY_KEYID *>(q_X509V3_EXT_d2i(ext));
+            if (!auth_key)
+                return QVariant();
 
             QVariantMap result;
 
@@ -449,9 +458,16 @@ static QVariant x509ExtensionToValue(X509_EXTENSION *ext)
 
 QSslCertificateExtension QSslCertificatePrivate::convertExtension(X509_EXTENSION *ext)
 {
+    Q_ASSERT(ext);
+
     QSslCertificateExtension result;
 
     ASN1_OBJECT *obj = q_X509_EXTENSION_get_object(ext);
+    if (!obj) {
+        qCWarning(lcSsl, "Invalid (nullptr) ASN1_OBJECT");
+        return result;
+    }
+
     QByteArray oid = QSslCertificatePrivate::asn1ObjectId(obj);
     QByteArray name = QSslCertificatePrivate::asn1ObjectName(obj);
 
@@ -488,10 +504,17 @@ QList<QSslCertificateExtension> QSslCertificate::extensions() const
         return result;
 
     int count = q_X509_get_ext_count(d->x509);
+    if (count <= 0)
+        return result;
+
     result.reserve(count);
 
     for (int i = 0; i < count; i++) {
         X509_EXTENSION *ext = q_X509_get_ext(d->x509, i);
+        if (!ext) {
+            qCWarning(lcSsl) << "Invalid (nullptr) extension at index" << i;
+            continue;
+        }
         result << QSslCertificatePrivate::convertExtension(ext);
     }
 
-- 
cgit v1.2.3