Fix stack overwrite in QDBusDemarshaller

QDBusArgument extraction operators and QDBusDemarshaller that implements
the extraction do not check the type of the extracted value.
Helper function template qIterGet in qdbusdemarshaller.cpp that is used
for extracting basic data types only reserves space from the stack for
the expected type as specified by client.
If the actual type in the DBus parameter is larger stack will be
overwritten in the helper function by at most 7 bytes (expected one byte,
received dbus_uint_64_t of size 8 bytes).

The fix always reserves space for the largest basic type dbus_uint64_t
readable by dbus_message_iter_get_basic API.

See also http://dbus.freedesktop.org/doc/api/html/group__DBusMessage.html#ga41c23a05e552d0574d04

Task-number: QTBUG-22735
Change-Id: I9aa25b279852ac8acc40199a39910ea4002042d7
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

1 files changed, 102 insertions, 2 deletions

diff --git a/tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp b/tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp
index 58f42e4513..2843e15758 100644
--- a/tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp
+++ b/tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp
@@ -93,6 +93,9 @@ private slots:
     void receiveUnknownType_data();
     void receiveUnknownType();
 
+    void demarshallPrimitives_data();
+    void demarshallPrimitives();
+
 private:
     int fileDescriptorForTest();
 
@@ -159,13 +162,15 @@ int tst_QDBusMarshall::fileDescriptorForTest()
     return tempFile.handle();
 }
 
-void tst_QDBusMarshall::sendBasic_data()
+void addBasicTypesColumns()
 {
     QTest::addColumn<QVariant>("value");
     QTest::addColumn<QString>("sig");
     QTest::addColumn<QString>("stringResult");
+}
 
-    // basic types:
+void basicNumericTypes_data()
+{
     QTest::newRow("bool") << QVariant(false) << "b" << "false";
     QTest::newRow("bool2") << QVariant(true) << "b" << "true";
     QTest::newRow("byte") << qVariantFromValue(uchar(1)) << "y" << "1";
@@ -176,11 +181,24 @@ void tst_QDBusMarshall::sendBasic_data()
     QTest::newRow("int64") << QVariant(Q_INT64_C(3)) << "x" << "3";
     QTest::newRow("uint64") << QVariant(Q_UINT64_C(4)) << "t" << "4";
     QTest::newRow("double") << QVariant(42.5) << "d" << "42.5";
+}
+
+void basicStringTypes_data()
+{
     QTest::newRow("string") << QVariant("ping") << "s" << "\"ping\"";
     QTest::newRow("objectpath") << qVariantFromValue(QDBusObjectPath("/org/kde")) << "o" << "[ObjectPath: /org/kde]";
     QTest::newRow("signature") << qVariantFromValue(QDBusSignature("g")) << "g" << "[Signature: g]";
     QTest::newRow("emptystring") << QVariant("") << "s" << "\"\"";
     QTest::newRow("nullstring") << QVariant(QString()) << "s" << "\"\"";
+}
+
+void tst_QDBusMarshall::sendBasic_data()
+{
+    addBasicTypesColumns();
+
+    // basic types:
+    basicNumericTypes_data();
+    basicStringTypes_data();
 
     if (fileDescriptorPassing)
         QTest::newRow("file-descriptor") << qVariantFromValue(QDBusUnixFileDescriptor(fileDescriptorForTest())) << "h" << "[Unix FD: valid]";
@@ -1160,5 +1178,87 @@ void tst_QDBusMarshall::receiveUnknownType()
 #endif
 }
 
+void tst_QDBusMarshall::demarshallPrimitives_data()
+{
+    addBasicTypesColumns();
+
+    // Primitive types, excluding strings and FD
+    basicNumericTypes_data();
+}
+
+template<class T>
+QVariant demarshallPrimitiveAs(const QDBusArgument& dbusArg)
+{
+    T val;
+    dbusArg >> val;
+    return qVariantFromValue(val);
+}
+
+QVariant demarshallPrimitiveAs(int typeIndex, const QDBusArgument& dbusArg)
+{
+    switch (typeIndex) {
+    case 0:
+        return demarshallPrimitiveAs<uchar>(dbusArg);
+    case 1:
+        return demarshallPrimitiveAs<bool>(dbusArg);
+    case 2:
+        return demarshallPrimitiveAs<short>(dbusArg);
+    case 3:
+        return demarshallPrimitiveAs<ushort>(dbusArg);
+    case 4:
+        return demarshallPrimitiveAs<int>(dbusArg);
+    case 5:
+        return demarshallPrimitiveAs<uint>(dbusArg);
+    case 6:
+        return demarshallPrimitiveAs<qlonglong>(dbusArg);
+    case 7:
+        return demarshallPrimitiveAs<qulonglong>(dbusArg);
+    case 8:
+        return demarshallPrimitiveAs<double>(dbusArg);
+    default:
+        return QVariant();
+    }
+}
+
+void tst_QDBusMarshall::demarshallPrimitives()
+{
+    QFETCH(QVariant, value);
+    QFETCH(QString, sig);
+
+    QDBusConnection con = QDBusConnection::sessionBus();
+
+    QVERIFY(con.isConnected());
+
+    // Demarshall each test data value to all primitive types to test
+    // demarshalling to the wrong type does not cause a crash
+    for (int typeIndex = 0; true; ++typeIndex) {
+        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();
+        QCOMPARE(receiveArg.currentSignature(), sig);
+
+        const QVariant receiveValue = demarshallPrimitiveAs(typeIndex, receiveArg);
+        if (receiveValue.type() == value.type()) {
+            // Value type is the same, compare the values
+            QCOMPARE(receiveValue, value);
+            QVERIFY(receiveArg.atEnd());
+        }
+
+        receiveArg.endStructure();
+        QVERIFY(receiveArg.atEnd());
+
+        if (!receiveValue.isValid())
+            break;
+    }
+}
+
 QTEST_MAIN(tst_QDBusMarshall)
 #include "tst_qdbusmarshall.moc"