Skip to content

Commit

Permalink
Fixing security vulnerability with Qt 4.8.5+ and PostgreSQL.
Browse files Browse the repository at this point in the history
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]
  • Loading branch information
MrEgs committed Oct 10, 2013
1 parent 61bf666 commit aa1008b
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 10 deletions.
9 changes: 6 additions & 3 deletions src/core/abstractsqlstorage.cpp
Expand Up @@ -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();
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/core/abstractsqlstorage.h
Expand Up @@ -80,7 +80,7 @@ public slots:
* 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();
Expand Down
46 changes: 41 additions & 5 deletions src/core/postgresqlstorage.cpp
Expand Up @@ -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;
}


Expand Down
2 changes: 1 addition & 1 deletion src/core/postgresqlstorage.h
Expand Up @@ -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; }
Expand Down

0 comments on commit aa1008b

Please sign in to comment.