Move Rfc822NameType, DnsNameType and UniformResourceIdentifierType.

Move these types to QAsn1Element so that they can use the toString() method which guards against malicious ASN.1. Change-Id: I7d6155147a6fc2d41da6f3ae87551b6cb75aa9ce Reviewed-by: Oliver Wolff <[email protected]> Reviewed-by: Daniel Molkentin <[email protected]>
brad0 · Feb 4, 2015 · 91a4816 · 91a4816
1 parent 3bc5f8c
commit 91a4816
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 16 deletions.
diff --git a/src/network/ssl/qasn1element.cpp b/src/network/ssl/qasn1element.cpp
@@ -340,7 +340,9 @@ QString QAsn1Element::toString() const
     if (qstrlen(mValue) < uint(mValue.size()))
         return QString();
 
-    if (mType == PrintableStringType || mType == TeletexStringType)
+    if (mType == PrintableStringType || mType == TeletexStringType
+        || mType == Rfc822NameType || mType == DnsNameType
+        || mType == UniformResourceIdentifierType)
         return QString::fromLatin1(mValue, mValue.size());
     if (mType == Utf8StringType)
         return QString::fromUtf8(mValue, mValue.size());

diff --git a/src/network/ssl/qasn1element_p.h b/src/network/ssl/qasn1element_p.h
@@ -81,6 +81,11 @@ class Q_AUTOTEST_EXPORT QAsn1Element
         SequenceType = 0x30,
         SetType = 0x31,
 
+        // GeneralNameTypes
+        Rfc822NameType = 0x81,
+        DnsNameType = 0x82,
+        UniformResourceIdentifierType = 0x86,
+
         // context specific
         Context0Type = 0xA0,
         Context3Type = 0xA3

diff --git a/src/network/ssl/qsslcertificate_qt.cpp b/src/network/ssl/qsslcertificate_qt.cpp
@@ -53,13 +53,6 @@
 
 QT_BEGIN_NAMESPACE
 
-enum GeneralNameType
-{
-    Rfc822NameType = 0x81,
-    DnsNameType = 0x82,
-    UniformResourceIdentifierType = 0x86
-};
-
 bool QSslCertificate::operator==(const QSslCertificate &other) const
 {
     if (d == other.d)
@@ -407,10 +400,10 @@ bool QSslCertificatePrivate::parse(const QByteArray &data)
                             QDataStream nameStream(sanElem.value());
                             QAsn1Element nameElem;
                             while (nameElem.read(nameStream)) {
-                                if (nameElem.type() == Rfc822NameType) {
-                                    subjectAlternativeNames.insert(QSsl::EmailEntry, QString::fromLatin1(nameElem.value(), nameElem.value().size()));
-                                } else if (nameElem.type() == DnsNameType) {
-                                    subjectAlternativeNames.insert(QSsl::DnsEntry, QString::fromLatin1(nameElem.value(), nameElem.value().size()));
+                                if (nameElem.type() == QAsn1Element::Rfc822NameType) {
+                                    subjectAlternativeNames.insert(QSsl::EmailEntry, nameElem.toString());
+                                } else if (nameElem.type() == QAsn1Element::DnsNameType) {
+                                    subjectAlternativeNames.insert(QSsl::DnsEntry, nameElem.toString());
                                 }
                             }
                         }
@@ -464,10 +457,10 @@ bool QSslCertificatePrivate::parseExtension(const QByteArray &data, QSslCertific
                 return false;
             const QString key = QString::fromLatin1(items.at(0).toObjectName());
             switch (items.at(1).type()) {
-            case Rfc822NameType:
-            case DnsNameType:
-            case UniformResourceIdentifierType:
-                result[key] = QString::fromLatin1(items.at(1).value(), items.at(1).value().size());
+            case QAsn1Element::Rfc822NameType:
+            case QAsn1Element::DnsNameType:
+            case QAsn1Element::UniformResourceIdentifierType:
+                result[key] = items.at(1).toString();
                 break;
             }
         }

diff --git a/tests/auto/network/ssl/qasn1element/tst_qasn1element.cpp b/tests/auto/network/ssl/qasn1element/tst_qasn1element.cpp
@@ -281,6 +281,15 @@ void tst_QAsn1Element::string_data()
     QTest::newRow("utf8string")
         << QAsn1Element(QAsn1Element::Utf8StringType, QByteArray("Hello World"))
         << QStringLiteral("Hello World");
+    QTest::newRow("rfc822name")
+        << QAsn1Element(QAsn1Element::Rfc822NameType, QByteArray("Hello World"))
+        << QStringLiteral("Hello World");
+    QTest::newRow("dnsname")
+        << QAsn1Element(QAsn1Element::DnsNameType, QByteArray("Hello World"))
+        << QStringLiteral("Hello World");
+    QTest::newRow("uri")
+        << QAsn1Element(QAsn1Element::UniformResourceIdentifierType, QByteArray("Hello World"))
+        << QStringLiteral("Hello World");
 
     // Embedded NULs are not allowed and should be rejected
     QTest::newRow("evil_printablestring")
@@ -292,6 +301,15 @@ void tst_QAsn1Element::string_data()
     QTest::newRow("evil_utf8string")
         << QAsn1Element(QAsn1Element::Utf8StringType, QByteArray("Hello\0World", 11))
         << QString();
+    QTest::newRow("evil_rfc822name")
+        << QAsn1Element(QAsn1Element::Rfc822NameType, QByteArray("Hello\0World", 11))
+        << QString();
+    QTest::newRow("evil_dnsname")
+        << QAsn1Element(QAsn1Element::DnsNameType, QByteArray("Hello\0World", 11))
+        << QString();
+    QTest::newRow("evil_uri")
+        << QAsn1Element(QAsn1Element::UniformResourceIdentifierType, QByteArray("Hello\0World", 11))
+        << QString();
 }
 
 void tst_QAsn1Element::string()