From a54d44298f6d2ecc1ec4d8c5c42c89c8a06fc5dd Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@kdab.com>
Date: Wed, 14 Sep 2016 16:14:16 +0200
Subject: [PATCH] QLatin1String: Fix UB (nullptr passed) in relational
 operators

Found by UBSan:

  qstring.h:1160:44: runtime error: null pointer passed as argument 1, which is declared to never be null
  qstring.h:1160:44: runtime error: null pointer passed as argument 2, which is declared to never be null

Fix by avoiding the memcmp() calls if there's a chance that they
might be called with nullptr.

While at it, also implement !=, >, <=, >= in terms of ==, <,
and add a test, because this particular UB was not fingered by
any of the QtCore test cases, but by a Qt3D one.

Change-Id: I413792dcc8431ef14f0c79f26e89a3e9fab69465
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
---
 src/corelib/tools/qstring.h                   | 22 +++----
 .../tools/qlatin1string/tst_qlatin1string.cpp | 57 +++++++++++++++++++
 2 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/src/corelib/tools/qstring.h b/src/corelib/tools/qstring.h
index 886973fe10..58ad2caa2b 100644
--- a/src/corelib/tools/qstring.h
+++ b/src/corelib/tools/qstring.h
@@ -1127,21 +1127,21 @@ inline bool operator!=(QString::Null, const QString &s) { return !s.isNull(); }
 inline bool operator!=(const QString &s, QString::Null) { return !s.isNull(); }
 
 inline bool operator==(QLatin1String s1, QLatin1String s2) Q_DECL_NOTHROW
-{ return (s1.size() == s2.size() && !memcmp(s1.latin1(), s2.latin1(), s1.size())); }
+{ return s1.size() == s2.size() && (!s1.size() || !memcmp(s1.latin1(), s2.latin1(), s1.size())); }
 inline bool operator!=(QLatin1String s1, QLatin1String s2) Q_DECL_NOTHROW
-{ return (s1.size() != s2.size() || memcmp(s1.latin1(), s2.latin1(), s1.size())); }
+{ return !operator==(s1, s2); }
 inline bool operator<(QLatin1String s1, QLatin1String s2) Q_DECL_NOTHROW
-{ int r = memcmp(s1.latin1(), s2.latin1(), qMin(s1.size(), s2.size()));
-  return (r < 0) || (r == 0 && s1.size() < s2.size()); }
-inline bool operator<=(QLatin1String s1, QLatin1String s2) Q_DECL_NOTHROW
-{ int r = memcmp(s1.latin1(), s2.latin1(), qMin(s1.size(), s2.size()));
-  return (r < 0) || (r == 0 && s1.size() <= s2.size()); }
+{
+    const int len = qMin(s1.size(), s2.size());
+    const int r = len ? memcmp(s1.latin1(), s2.latin1(), len) : 0;
+    return r < 0 || (r == 0 && s1.size() < s2.size());
+}
 inline bool operator>(QLatin1String s1, QLatin1String s2) Q_DECL_NOTHROW
-{ int r = memcmp(s1.latin1(), s2.latin1(), qMin(s1.size(), s2.size()));
-  return (r > 0) || (r == 0 && s1.size() > s2.size()); }
+{ return operator<(s2, s1); }
+inline bool operator<=(QLatin1String s1, QLatin1String s2) Q_DECL_NOTHROW
+{ return !operator>(s1, s2); }
 inline bool operator>=(QLatin1String s1, QLatin1String s2) Q_DECL_NOTHROW
-{ int r = memcmp(s1.latin1(), s2.latin1(), qMin(s1.size(), s2.size()));
-  return (r > 0) || (r == 0 && s1.size() >= s2.size()); }
+{ return !operator<(s1, s2); }
 
 inline bool QLatin1String::operator==(const QString &s) const Q_DECL_NOTHROW
 { return s == *this; }
diff --git a/tests/auto/corelib/tools/qlatin1string/tst_qlatin1string.cpp b/tests/auto/corelib/tools/qlatin1string/tst_qlatin1string.cpp
index 290c9fc12a..6f16120bd0 100644
--- a/tests/auto/corelib/tools/qlatin1string/tst_qlatin1string.cpp
+++ b/tests/auto/corelib/tools/qlatin1string/tst_qlatin1string.cpp
@@ -35,6 +35,15 @@
 
 #include <QString>
 
+// Preserve QLatin1String-ness (QVariant(QLatin1String) creates a QVariant::String):
+struct QLatin1StringContainer {
+    QLatin1String l1;
+};
+QT_BEGIN_NAMESPACE
+Q_DECLARE_TYPEINFO(QLatin1StringContainer, Q_MOVABLE_TYPE);
+QT_END_NAMESPACE
+Q_DECLARE_METATYPE(QLatin1StringContainer)
+
 class tst_QLatin1String : public QObject
 {
     Q_OBJECT
@@ -42,6 +51,8 @@ class tst_QLatin1String : public QObject
 private Q_SLOTS:
     void nullString();
     void emptyString();
+    void relationalOperators_data();
+    void relationalOperators();
 };
 
 void tst_QLatin1String::nullString()
@@ -119,7 +130,53 @@ void tst_QLatin1String::emptyString()
     }
 }
 
+void tst_QLatin1String::relationalOperators_data()
+{
+    QTest::addColumn<QLatin1StringContainer>("lhs");
+    QTest::addColumn<int>("lhsOrderNumber");
+    QTest::addColumn<QLatin1StringContainer>("rhs");
+    QTest::addColumn<int>("rhsOrderNumber");
 
+    struct Data {
+        QLatin1String l1;
+        int order;
+    } data[] = {
+        { QLatin1String(),     0 },
+        { QLatin1String(""),   0 },
+        { QLatin1String("a"),  1 },
+        { QLatin1String("aa"), 2 },
+        { QLatin1String("b"),  3 },
+    };
+
+    for (Data *lhs = data; lhs != data + sizeof data / sizeof *data; ++lhs) {
+        for (Data *rhs = data; rhs != data + sizeof data / sizeof *data; ++rhs) {
+            QLatin1StringContainer l = { lhs->l1 }, r = { rhs->l1 };
+            QTest::newRow(qPrintable(QString::asprintf("\"%s\" <> \"%s\"",
+                                                       lhs->l1.data() ? lhs->l1.data() : "nullptr",
+                                                       rhs->l1.data() ? rhs->l1.data() : "nullptr")))
+                << l << lhs->order << r << rhs->order;
+        }
+    }
+}
+
+void tst_QLatin1String::relationalOperators()
+{
+    QFETCH(QLatin1StringContainer, lhs);
+    QFETCH(int, lhsOrderNumber);
+    QFETCH(QLatin1StringContainer, rhs);
+    QFETCH(int, rhsOrderNumber);
+
+#define CHECK(op) \
+    QCOMPARE(lhs.l1 op rhs.l1, lhsOrderNumber op rhsOrderNumber) \
+    /*end*/
+    CHECK(==);
+    CHECK(!=);
+    CHECK(< );
+    CHECK(> );
+    CHECK(<=);
+    CHECK(>=);
+#undef CHECK
+}
 
 QTEST_APPLESS_MAIN(tst_QLatin1String)