From: Marcus Eggenberger <egs@quassel-irc.org>
Date: Thu, 10 Oct 2013 11:26:39 +0000 (+0200)
Subject: Fixing security vulnerability with Qt 4.8.5+ and PostgreSQL.
X-Git-Tag: 0.10-beta1~114
X-Git-Url: https://git.quassel-irc.org/?p=quassel.git;a=commitdiff_plain;h=aa1008be162cb27da938cce93ba533f54d228869

Fixing security vulnerability with Qt 4.8.5+ and PostgreSQL.

Properly detects whether Qt performs slash escaping in SQL queries or
not, and then configures PostgreSQL accordingly. This bug was a
introduced due to a bugfix in Qt 4.8.5 disables slash escaping when
binding queries: https://bugreports.qt-project.org/browse/QTBUG-30076
Thanks to brot and Tucos.

[Fixes #1244]
---

diff --git a/src/core/abstractsqlstorage.cpp b/src/core/abstractsqlstorage.cpp
index ad08947d..c3f18413 100644
--- a/src/core/abstractsqlstorage.cpp
+++ b/src/core/abstractsqlstorage.cpp
@@ -91,11 +91,14 @@ void AbstractSqlStorage::addConnectionToPool()
     }
 
     if (!db.open()) {
-        qWarning() << "Unable to open database" << displayName() << "for thread" << QThread::currentThread();
-        qWarning() << "-" << db.lastError().text();
+        quWarning() << "Unable to open database" << displayName() << "for thread" << QThread::currentThread();
+        quWarning() << "-" << db.lastError().text();
     }
     else {
-        initDbSession(db);
+        if (!initDbSession(db)) {
+            quWarning() << "Unable to initialize database" << displayName() << "for thread" << QThread::currentThread();
+            db.close();
+        }
     }
 }
 
diff --git a/src/core/abstractsqlstorage.h b/src/core/abstractsqlstorage.h
index 0cf91b96..4b54f4fd 100644
--- a/src/core/abstractsqlstorage.h
+++ b/src/core/abstractsqlstorage.h
@@ -80,7 +80,7 @@ protected:
      *  When reimplementing this method, don't use logDB() inside this function as
      *  this would cause as we're just about to initialize that DB connection.
      */
-    inline virtual void initDbSession(QSqlDatabase & /* db */) {}
+    inline virtual bool initDbSession(QSqlDatabase & /* db */) { return true; }
 
 private slots:
     void connectionDestroyed();
diff --git a/src/core/postgresqlstorage.cpp b/src/core/postgresqlstorage.cpp
index 39657046..b7e8c9bc 100644
--- a/src/core/postgresqlstorage.cpp
+++ b/src/core/postgresqlstorage.cpp
@@ -96,11 +96,47 @@ QVariantMap PostgreSqlStorage::setupDefaults() const
 }
 
 
-void PostgreSqlStorage::initDbSession(QSqlDatabase &db)
-{
-    // this blows... but unfortunately Qt's PG driver forces us to this...
-    db.exec("set standard_conforming_strings = off");
-    db.exec("set escape_string_warning = off");
+bool PostgreSqlStorage::initDbSession(QSqlDatabase &db)
+{
+    // check whether the Qt driver performs string escaping or not.
+    // i.e. test if it doubles slashes.
+    QSqlField testField;
+    testField.setType(QVariant::String);
+    testField.setValue("\\");
+    QString formattedString = db.driver()->formatValue(testField);
+    switch(formattedString.count('\\')) {
+    case 2:
+        // yes it does... and we cannot do anything to change the behavior of Qt.
+        // If this is a legacy DB (Postgres < 8.2), then everything is already ok,
+        // as this is the expected behavior.
+        // If it is a newer version, switch to legacy mode.
+
+        quWarning() << "Switching Postgres to legacy mode. (set standard conforming strings to off)";
+        // If the following calls fail, it is a legacy DB anyways, so it doesn't matter
+        // and no need to check the outcome.
+        db.exec("set standard_conforming_strings = off");
+        db.exec("set escape_string_warning = off");
+        break;
+    case 1:
+        // ok, so Qt does not escape...
+        // That means we have to ensure that postgres uses standard conforming strings...
+        {
+            QSqlQuery query = db.exec("set standard_conforming_strings = on");
+            if (query.lastError().isValid()) {
+                // We cannot enable standard conforming strings...
+                // since Quassel does no escaping by itself, this would yield a major vulnerability.
+                quError() << "Failed to enable standard_conforming_strings for the Postgres db!";
+                return false;
+            }
+        }
+        break;
+    default:
+        // The slash got replaced with 0 or more than 2 slashes! o_O
+        quError() << "Your version of Qt does something _VERY_ strange to slashes in QSqlQueries! You should consult your trusted doctor!";
+        return false;
+        break;
+    }
+    return true;
 }
 
 
diff --git a/src/core/postgresqlstorage.h b/src/core/postgresqlstorage.h
index 5be3f80f..f9aafd85 100644
--- a/src/core/postgresqlstorage.h
+++ b/src/core/postgresqlstorage.h
@@ -103,7 +103,7 @@ public slots:
     virtual QList<Message> requestAllMsgs(UserId user, MsgId first = -1, MsgId last = -1, int limit = -1);
 
 protected:
-    virtual void initDbSession(QSqlDatabase &db);
+    virtual bool initDbSession(QSqlDatabase &db);
     virtual void setConnectionProperties(const QVariantMap &properties);
     inline virtual QString driverName() { return "QPSQL"; }
     inline virtual QString hostName() { return _hostName; }