From d1cd17713e6d0bd9c7a270ba675704ad244e2b13 Mon Sep 17 00:00:00 2001
From: Sami Rosendahl <ext-sami.1.rosendahl@nokia.com>
Date: Tue, 17 Jan 2012 13:41:20 +0200
Subject: Fix crash in QDBusDemarshaller QStringList extraction

QDBusArgument QStringList extraction operator and QDBusDemarshaller that
implements the extraction do not check the type of the extracted value.
When extracting a QStringList and the value actually is e.g. an array of
bytes the string list extraction will crash as it interprets the bytes as
char pointers.

The fix adds DBus type checks to QDBusArgument QStringList extraction
operator implementations.
The checks are as permissive as possible provided crashes are avoided.

Task-number: QTBUG-22840
Change-Id: I83a98097a7cf36f8448afba81d0ad619cdf864e3
(From Qt5 commit b4398dc4e372dbe829b21423e1a0a93a6a542994)
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/dbus/qdbusargument_p.h                     |  1 +
 src/dbus/qdbusdemarshaller.cpp                 | 13 ++++++-
 tests/auto/qdbusmarshall/tst_qdbusmarshall.cpp | 52 ++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/src/dbus/qdbusargument_p.h b/src/dbus/qdbusargument_p.h
index 36e9f2d..e977c99 100644
--- a/src/dbus/qdbusargument_p.h
+++ b/src/dbus/qdbusargument_p.h
@@ -212,6 +212,7 @@ private:
     QString toStringUnchecked();
     QDBusObjectPath toObjectPathUnchecked();
     QDBusSignature toSignatureUnchecked();
+    QStringList toStringListUnchecked();
 };
 
 inline QDBusMarshaller *QDBusArgumentPrivate::marshaller()
diff --git a/src/dbus/qdbusdemarshaller.cpp b/src/dbus/qdbusdemarshaller.cpp
index 8769846..be19b00 100644
--- a/src/dbus/qdbusdemarshaller.cpp
+++ b/src/dbus/qdbusdemarshaller.cpp
@@ -274,7 +274,7 @@ QVariant QDBusDemarshaller::toVariantInternal()
             // QByteArray
             return toByteArray();
         case DBUS_TYPE_STRING:
-            return toStringList();
+            return toStringListUnchecked();
         case DBUS_TYPE_DICT_ENTRY:
             return QVariant::fromValue(duplicate());
 
@@ -317,7 +317,7 @@ bool QDBusDemarshaller::isCurrentTypeStringLike()
     }
 }
 
-QStringList QDBusDemarshaller::toStringList()
+QStringList QDBusDemarshaller::toStringListUnchecked()
 {
     QStringList list;
 
@@ -330,6 +330,15 @@ QStringList QDBusDemarshaller::toStringList()
     return list;
 }
 
+QStringList QDBusDemarshaller::toStringList()
+{
+    if (q_dbus_message_iter_get_arg_type(&iterator) == DBUS_TYPE_ARRAY
+            && q_dbus_message_iter_get_element_type(&iterator) == DBUS_TYPE_STRING)
+        return toStringListUnchecked();
+    else
+        return QStringList();
+}
+
 QByteArray QDBusDemarshaller::toByteArray()
 {
     DBusMessageIter sub;
diff --git a/tests/auto/qdbusmarshall/tst_qdbusmarshall.cpp b/tests/auto/qdbusmarshall/tst_qdbusmarshall.cpp
index bf7270e..0ac917c 100644
--- a/tests/auto/qdbusmarshall/tst_qdbusmarshall.cpp
+++ b/tests/auto/qdbusmarshall/tst_qdbusmarshall.cpp
@@ -99,6 +99,9 @@ private slots:
     void demarshallStrings_data();
     void demarshallStrings();
 
+    void demarshallInvalidStringList_data();
+    void demarshallInvalidStringList();
+
 private:
     int fileDescriptorForTest();
 
@@ -1383,5 +1386,54 @@ void tst_QDBusMarshall::demarshallStrings()
     QVERIFY(receiveArg.atEnd());
 }
 
+void tst_QDBusMarshall::demarshallInvalidStringList_data()
+{
+    addBasicTypesColumns();
+
+    // None of the basic types should demarshall to a string list
+    basicNumericTypes_data();
+    basicStringTypes_data();
+
+    // Arrays of non-string type should not demarshall to a string list
+    QList<bool> bools;
+    QTest::newRow("emptyboollist") << qVariantFromValue(bools);
+    bools << false << true << false;
+    QTest::newRow("boollist") << qVariantFromValue(bools);
+
+    // Structures should not demarshall to a QByteArray
+    QTest::newRow("struct of strings")
+            << qVariantFromValue(QVariantList() << QString("foo") << QString("bar"));
+    QTest::newRow("struct of mixed types")
+            << qVariantFromValue(QVariantList() << QString("foo") << int(42) << double(3.14));
+}
+
+void tst_QDBusMarshall::demarshallInvalidStringList()
+{
+    QFETCH(QVariant, value);
+
+    QDBusConnection con = QDBusConnection::sessionBus();
+
+    QVERIFY(con.isConnected());
+
+    QDBusMessage msg = QDBusMessage::createMethodCall(serviceName, objectPath,
+                                                      interfaceName, "ping");
+    QDBusArgument sendArg;
+    sendArg.beginStructure();
+    sendArg.appendVariant(value);
+    sendArg.endStructure();
+    msg.setArguments(QVariantList() << qVariantFromValue(sendArg));
+    QDBusMessage reply = con.call(msg);
+
+    const QDBusArgument receiveArg = qvariant_cast<QDBusArgument>(reply.arguments().at(0));
+    receiveArg.beginStructure();
+
+    QStringList receiveValue;
+    receiveArg >> receiveValue;
+    QCOMPARE(receiveValue, QStringList());
+
+    receiveArg.endStructure();
+    QVERIFY(receiveArg.atEnd());
+}
+
 QTEST_MAIN(tst_QDBusMarshall)
 #include "tst_qdbusmarshall.moc"
-- 
cgit v0.12