Fix crash in QDBusDemarshaller basic string-like type extraction
QDBusArgument string extraction operators and QDBusDemarshaller that implements the extraction do not check the type of the extracted value. When extracting string-like basic DBus type that actually is e.g. an integer the string extraction will crash as it blindly attempts to use the integer as a pointer to char. The fix adds DBus type checks to QDBusArgument string type extraction operator implementations. The checks are as permissive as possible provided crashes are avoided. Previously supported functionality of extracting an object path or type signature to a string type is retained. Task-number: QTBUG-22840 Change-Id: I29be1ae592658ca268c65ed692e1d42619d52280 Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
parent
bca775edaa
commit
8f19f14274
@ -201,6 +201,7 @@ public:
|
||||
|
||||
QVariant toVariantInternal();
|
||||
QDBusArgument::ElementType currentType();
|
||||
bool isCurrentTypeStringLike();
|
||||
|
||||
public:
|
||||
DBusMessageIter iterator;
|
||||
@ -208,6 +209,9 @@ public:
|
||||
|
||||
private:
|
||||
Q_DISABLE_COPY(QDBusDemarshaller)
|
||||
QString toStringUnchecked();
|
||||
QDBusObjectPath toObjectPathUnchecked();
|
||||
QDBusSignature toSignatureUnchecked();
|
||||
};
|
||||
|
||||
inline QDBusMarshaller *QDBusArgumentPrivate::marshaller()
|
||||
|
@ -130,19 +130,43 @@ inline double QDBusDemarshaller::toDouble()
|
||||
return qIterGet<double>(&iterator);
|
||||
}
|
||||
|
||||
inline QString QDBusDemarshaller::toString()
|
||||
inline QString QDBusDemarshaller::toStringUnchecked()
|
||||
{
|
||||
return QString::fromUtf8(qIterGet<char *>(&iterator));
|
||||
}
|
||||
|
||||
inline QString QDBusDemarshaller::toString()
|
||||
{
|
||||
if (isCurrentTypeStringLike())
|
||||
return toStringUnchecked();
|
||||
else
|
||||
return QString();
|
||||
}
|
||||
|
||||
inline QDBusObjectPath QDBusDemarshaller::toObjectPathUnchecked()
|
||||
{
|
||||
return QDBusObjectPath(QString::fromUtf8(qIterGet<char *>(&iterator)));
|
||||
}
|
||||
|
||||
inline QDBusObjectPath QDBusDemarshaller::toObjectPath()
|
||||
{
|
||||
return QDBusObjectPath(QString::fromUtf8(qIterGet<char *>(&iterator)));
|
||||
if (isCurrentTypeStringLike())
|
||||
return toObjectPathUnchecked();
|
||||
else
|
||||
return QDBusObjectPath();
|
||||
}
|
||||
|
||||
inline QDBusSignature QDBusDemarshaller::toSignatureUnchecked()
|
||||
{
|
||||
return QDBusSignature(QString::fromUtf8(qIterGet<char *>(&iterator)));
|
||||
}
|
||||
|
||||
inline QDBusSignature QDBusDemarshaller::toSignature()
|
||||
{
|
||||
return QDBusSignature(QString::fromUtf8(qIterGet<char *>(&iterator)));
|
||||
if (isCurrentTypeStringLike())
|
||||
return toSignatureUnchecked();
|
||||
else
|
||||
return QDBusSignature();
|
||||
}
|
||||
|
||||
inline QDBusUnixFileDescriptor QDBusDemarshaller::toUnixFileDescriptor()
|
||||
@ -236,11 +260,11 @@ QVariant QDBusDemarshaller::toVariantInternal()
|
||||
case DBUS_TYPE_UINT64:
|
||||
return toULongLong();
|
||||
case DBUS_TYPE_STRING:
|
||||
return toString();
|
||||
return toStringUnchecked();
|
||||
case DBUS_TYPE_OBJECT_PATH:
|
||||
return QVariant::fromValue(toObjectPath());
|
||||
return QVariant::fromValue(toObjectPathUnchecked());
|
||||
case DBUS_TYPE_SIGNATURE:
|
||||
return QVariant::fromValue(toSignature());
|
||||
return QVariant::fromValue(toSignatureUnchecked());
|
||||
case DBUS_TYPE_VARIANT:
|
||||
return QVariant::fromValue(toVariant());
|
||||
|
||||
@ -280,6 +304,19 @@ QVariant QDBusDemarshaller::toVariantInternal()
|
||||
};
|
||||
}
|
||||
|
||||
bool QDBusDemarshaller::isCurrentTypeStringLike()
|
||||
{
|
||||
const int type = q_dbus_message_iter_get_arg_type(&iterator);
|
||||
switch (type) {
|
||||
case DBUS_TYPE_STRING: //FALLTHROUGH
|
||||
case DBUS_TYPE_OBJECT_PATH: //FALLTHROUGH
|
||||
case DBUS_TYPE_SIGNATURE:
|
||||
return true;
|
||||
default:
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
QStringList QDBusDemarshaller::toStringList()
|
||||
{
|
||||
QStringList list;
|
||||
@ -288,7 +325,7 @@ QStringList QDBusDemarshaller::toStringList()
|
||||
q_dbus_message_iter_recurse(&iterator, &sub.iterator);
|
||||
q_dbus_message_iter_next(&iterator);
|
||||
while (!sub.atEnd())
|
||||
list.append(sub.toString());
|
||||
list.append(sub.toStringUnchecked());
|
||||
|
||||
return list;
|
||||
}
|
||||
|
@ -96,6 +96,9 @@ private slots:
|
||||
void demarshallPrimitives_data();
|
||||
void demarshallPrimitives();
|
||||
|
||||
void demarshallStrings_data();
|
||||
void demarshallStrings();
|
||||
|
||||
private:
|
||||
int fileDescriptorForTest();
|
||||
|
||||
@ -1260,5 +1263,117 @@ void tst_QDBusMarshall::demarshallPrimitives()
|
||||
}
|
||||
}
|
||||
|
||||
void tst_QDBusMarshall::demarshallStrings_data()
|
||||
{
|
||||
QTest::addColumn<QVariant>("value");
|
||||
QTest::addColumn<char>("targetSig");
|
||||
QTest::addColumn<QVariant>("expectedValue");
|
||||
|
||||
// All primitive types demarshall to null string types
|
||||
typedef QPair<QVariant, char> ValSigPair;
|
||||
const QList<ValSigPair> nullStringTypes
|
||||
= QList<ValSigPair>()
|
||||
<< ValSigPair(qVariantFromValue(QString()), 's')
|
||||
<< ValSigPair(qVariantFromValue(QDBusObjectPath()), 'o')
|
||||
<< ValSigPair(qVariantFromValue(QDBusSignature()), 'g');
|
||||
foreach (ValSigPair valSigPair, nullStringTypes) {
|
||||
QTest::newRow("bool(false)") << QVariant(false) << valSigPair.second << valSigPair.first;
|
||||
QTest::newRow("bool(true)") << QVariant(true) << valSigPair.second << valSigPair.first;
|
||||
QTest::newRow("byte") << qVariantFromValue(uchar(1)) << valSigPair.second << valSigPair.first;
|
||||
QTest::newRow("int16") << qVariantFromValue(short(2)) << valSigPair.second << valSigPair.first;
|
||||
QTest::newRow("uint16") << qVariantFromValue(ushort(3)) << valSigPair.second << valSigPair.first;
|
||||
QTest::newRow("int") << QVariant(1) << valSigPair.second << valSigPair.first;
|
||||
QTest::newRow("uint") << QVariant(2U) << valSigPair.second << valSigPair.first;
|
||||
QTest::newRow("int64") << QVariant(Q_INT64_C(3)) << valSigPair.second << valSigPair.first;
|
||||
QTest::newRow("uint64") << QVariant(Q_UINT64_C(4)) << valSigPair.second << valSigPair.first;
|
||||
QTest::newRow("double") << QVariant(42.5) << valSigPair.second << valSigPair.first;
|
||||
}
|
||||
|
||||
// String types should demarshall to each other. This is a regression test
|
||||
// to check released functionality is maintained even after checks have
|
||||
// been added to string demarshalling
|
||||
QTest::newRow("empty string->invalid objectpath") << QVariant("")
|
||||
<< 'o' << qVariantFromValue(QDBusObjectPath());
|
||||
QTest::newRow("null string->invalid objectpath") << QVariant(QString())
|
||||
<< 'o' << qVariantFromValue(QDBusObjectPath());
|
||||
QTest::newRow("string->invalid objectpath") << QVariant("invalid objectpath")
|
||||
<< 'o' << qVariantFromValue(QDBusObjectPath());
|
||||
QTest::newRow("string->valid objectpath") << QVariant("/org/kde")
|
||||
<< 'o' << qVariantFromValue(QDBusObjectPath("/org/kde"));
|
||||
|
||||
QTest::newRow("empty string->invalid signature") << QVariant("")
|
||||
<< 'g' << qVariantFromValue(QDBusSignature());
|
||||
QTest::newRow("null string->invalid signature") << QVariant(QString())
|
||||
<< 'g' << qVariantFromValue(QDBusSignature());
|
||||
QTest::newRow("string->invalid signature") << QVariant("_invalid signature")
|
||||
<< 'g' << qVariantFromValue(QDBusSignature());
|
||||
QTest::newRow("string->valid signature") << QVariant("s")
|
||||
<< 'g' << qVariantFromValue(QDBusSignature("s"));
|
||||
|
||||
QTest::newRow("objectpath->string") << qVariantFromValue(QDBusObjectPath("/org/kde"))
|
||||
<< 's' << qVariantFromValue(QString("/org/kde"));
|
||||
QTest::newRow("objectpath->invalid signature") << qVariantFromValue(QDBusObjectPath("/org/kde"))
|
||||
<< 'g' << qVariantFromValue(QDBusSignature());
|
||||
|
||||
QTest::newRow("signature->string") << qVariantFromValue(QDBusSignature("s"))
|
||||
<< 's' << qVariantFromValue(QString("s"));
|
||||
QTest::newRow("signature->invalid objectpath") << qVariantFromValue(QDBusSignature("s"))
|
||||
<< 'o' << qVariantFromValue(QDBusObjectPath());
|
||||
}
|
||||
|
||||
QVariant demarshallAsString(const QDBusArgument& dbusArg, char targetSig)
|
||||
{
|
||||
switch (targetSig) {
|
||||
case 's': {
|
||||
QString s;
|
||||
dbusArg >> s;
|
||||
return s;
|
||||
}
|
||||
case 'o': {
|
||||
QDBusObjectPath op;
|
||||
dbusArg >> op;
|
||||
return qVariantFromValue(op);
|
||||
}
|
||||
case 'g' : {
|
||||
QDBusSignature sig;
|
||||
dbusArg >> sig;
|
||||
return qVariantFromValue(sig);
|
||||
}
|
||||
default: {
|
||||
return QVariant();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void tst_QDBusMarshall::demarshallStrings()
|
||||
{
|
||||
QFETCH(QVariant, value);
|
||||
QFETCH(char, targetSig);
|
||||
QFETCH(QVariant, expectedValue);
|
||||
|
||||
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();
|
||||
|
||||
QVariant receiveValue = demarshallAsString(receiveArg, targetSig);
|
||||
QVERIFY2(receiveValue.isValid(), "Invalid targetSig in demarshallStrings_data()");
|
||||
QVERIFY(compare(receiveValue, expectedValue));
|
||||
|
||||
receiveArg.endStructure();
|
||||
QVERIFY(receiveArg.atEnd());
|
||||
}
|
||||
|
||||
QTEST_MAIN(tst_QDBusMarshall)
|
||||
#include "tst_qdbusmarshall.moc"
|
||||
|
Loading…
Reference in New Issue
Block a user