fix(history): Prevent invalid history access

* When the DB schema was too new we were accessing history anyways. This
has potential to just completely corrupt the DB
* When history was disabled there was a chance we would attempt to write
to history anyways. Added more checks in this area
* Chatform was accessing invalid iterators when there were no displayed
messages. Added a guard for this case

src/persistence/history.cpp

@@ -195,10 +195,11 @@ bool dbSchema3to4(RawDatabase& db)
 
 /**
 * @brief Upgrade the db schema
+* @return True if the schema upgrade succeded, false otherwise
 * @note On future alterations of the database all you have to do is bump the SCHEMA_VERSION
 * variable and add another case to the switch statement below. Make sure to fall through on each case.
 */
-void dbSchemaUpgrade(std::shared_ptr<RawDatabase>& db)
+bool dbSchemaUpgrade(std::shared_ptr<RawDatabase>& db)
 {
     int64_t databaseSchemaVersion;
 
@@ -206,19 +207,17 @@ void dbSchemaUpgrade(std::shared_ptr<RawDatabase>& db)
             databaseSchemaVersion = row[0].toLongLong();
         }))) {
         qCritical() << "History failed to read user_version";
-        db.reset();
+        return false;
-        return;
     }
 
     if (databaseSchemaVersion > SCHEMA_VERSION) {
         qWarning().nospace() << "Database version (" << databaseSchemaVersion <<
             ") is newer than we currently support (" << SCHEMA_VERSION << "). Please upgrade qTox";
         // We don't know what future versions have done, we have to disable db access until we re-upgrade
-        db.reset();
+        return false;
-        return;
     } else if (databaseSchemaVersion == SCHEMA_VERSION) {
         // No work to do
-        return;
+        return true;
     }
 
     switch (databaseSchemaVersion) {
@@ -231,22 +230,19 @@ void dbSchemaUpgrade(std::shared_ptr<RawDatabase>& db)
         const bool newDb = isNewDb(db, success);
         if (!success) {
             qCritical() << "Failed to create current db schema";
-            db.reset();
+            return false;
-            return;
         }
         if (newDb) {
             if (!createCurrentSchema(*db)) {
                 qCritical() << "Failed to create current db schema";
-                db.reset();
+                return false;
-                return;
             }
             qDebug() << "Database created at schema version" << SCHEMA_VERSION;
             break; // new db is the only case where we don't incrementally upgrade through each version
         } else {
             if (!dbSchema0to1(*db)) {
                 qCritical() << "Failed to upgrade db to schema version 1, aborting";
-                db.reset();
+                return false;
-                return;
             }
             qDebug() << "Database upgraded incrementally to schema version 1";
         }
@@ -255,23 +251,20 @@ void dbSchemaUpgrade(std::shared_ptr<RawDatabase>& db)
     case 1:
        if (!dbSchema1to2(*db)) {
             qCritical() << "Failed to upgrade db to schema version 2, aborting";
-            db.reset();
+            return false;
-            return;
        }
        qDebug() << "Database upgraded incrementally to schema version 2";
        //fallthrough
     case 2:
        if (!dbSchema2to3(*db)) {
             qCritical() << "Failed to upgrade db to schema version 3, aborting";
-            db.reset();
+            return false;
-            return;
        }
        qDebug() << "Database upgraded incrementally to schema version 3";
     case 3:
        if (!dbSchema3to4(*db)) {
             qCritical() << "Failed to upgrade db to schema version 4, aborting";
-            db.reset();
+            return false;
-            return;
        }
        qDebug() << "Database upgraded incrementally to schema version 4";
     // etc.
@@ -279,6 +272,8 @@ void dbSchemaUpgrade(std::shared_ptr<RawDatabase>& db)
         qInfo() << "Database upgrade finished (databaseSchemaVersion" << databaseSchemaVersion
                 << "->" << SCHEMA_VERSION << ")";
     }
+
+    return true;
 }
 
 MessageState getMessageState(bool isPending, bool isBroken)
@@ -318,18 +313,19 @@ FileDbInsertionData::FileDbInsertionData()
  * @brief Prepares the database to work with the history.
  * @param db This database will be prepared for use with the history.
  */
-History::History(std::shared_ptr<RawDatabase> db)
+History::History(std::shared_ptr<RawDatabase> db_)
-    : db(db)
+    : db(db_)
 {
     if (!isValid()) {
         qWarning() << "Database not open, init failed";
         return;
     }
 
-    dbSchemaUpgrade(db);
+    const auto upgradeSucceeded = dbSchemaUpgrade(db);
 
     // dbSchemaUpgrade may have put us in an invalid state
-    if (!isValid()) {
+    if (!upgradeSucceeded) {
+        db.reset();
         return;
     }
 
@@ -370,6 +366,10 @@ bool History::isValid()
  */
 bool History::historyExists(const ToxPk& friendPk)
 {
+    if (historyAccessBlocked()) {
+        return false;
+    }
+
     return !getMessagesForFriend(friendPk, 0, 1).empty();
 }
 
@@ -588,6 +588,10 @@ void History::addNewFileMessage(const QString& friendPk, const QString& fileId,
                                 const QString& fileName, const QString& filePath, int64_t size,
                                 const QString& sender, const QDateTime& time, QString const& dispName)
 {
+    if (historyAccessBlocked()) {
+        return;
+    }
+
     // This is an incredibly far from an optimal way of implementing this,
     // but given the frequency that people are going to be initiating a file
     // transfer we can probably live with it.
@@ -644,11 +648,7 @@ void History::addNewMessage(const QString& friendPk, const QString& message, con
                             const QDateTime& time, bool isDelivered, QString dispName,
                             const std::function<void(RowId)>& insertIdCallback)
 {
-    if (!Settings::getInstance().getEnableLogging()) {
+    if (historyAccessBlocked()) {
-        qWarning() << "Blocked a message from being added to database while history is disabled";
-        return;
-    }
-    if (!isValid()) {
         return;
     }
 
@@ -659,6 +659,10 @@ void History::addNewMessage(const QString& friendPk, const QString& message, con
 void History::setFileFinished(const QString& fileId, bool success, const QString& filePath,
                               const QByteArray& fileHash)
 {
+    if (historyAccessBlocked()) {
+        return;
+    }
+
     auto& fileInfo = fileInfos[fileId];
     if (fileInfo.fileId.get() == -1) {
         fileInfo.finished = true;
@@ -674,11 +678,19 @@ void History::setFileFinished(const QString& fileId, bool success, const QString
 
 size_t History::getNumMessagesForFriend(const ToxPk& friendPk)
 {
+    if (historyAccessBlocked()) {
+        return 0;
+    }
+
     return getNumMessagesForFriendBeforeDate(friendPk, QDateTime());
 }
 
 size_t History::getNumMessagesForFriendBeforeDate(const ToxPk& friendPk, const QDateTime& date)
 {
+    if (historyAccessBlocked()) {
+        return 0;
+    }
+
     QString queryText = QString("SELECT COUNT(history.id) "
                                 "FROM history "
                                 "JOIN peers chat ON chat_id = chat.id "
@@ -704,6 +716,10 @@ size_t History::getNumMessagesForFriendBeforeDate(const ToxPk& friendPk, const Q
 QList<History::HistMessage> History::getMessagesForFriend(const ToxPk& friendPk, size_t firstIdx,
                                                           size_t lastIdx)
 {
+    if (historyAccessBlocked()) {
+        return {};
+    }
+
     QList<HistMessage> messages;
 
     // Don't forget to update the rowCallback if you change the selected columns!
@@ -763,6 +779,10 @@ QList<History::HistMessage> History::getMessagesForFriend(const ToxPk& friendPk,
 
 QList<History::HistMessage> History::getUndeliveredMessagesForFriend(const ToxPk& friendPk)
 {
+    if (historyAccessBlocked()) {
+        return {};
+    }
+
     auto queryText =
         QString("SELECT history.id, faux_offline_pending.id, timestamp, chat.public_key, "
                 "aliases.display_name, sender.public_key, message, broken_messages.id "
@@ -809,6 +829,10 @@ QList<History::HistMessage> History::getUndeliveredMessagesForFriend(const ToxPk
 QDateTime History::getDateWhereFindPhrase(const QString& friendPk, const QDateTime& from,
                                           QString phrase, const ParameterSearch& parameter)
 {
+    if (historyAccessBlocked()) {
+        return QDateTime();
+    }
+
     QDateTime result;
     auto rowCallback = [&result](const QVector<QVariant>& row) {
         result = QDateTime::fromMSecsSinceEpoch(row[0].toLongLong());
@@ -903,6 +927,10 @@ QList<History::DateIdx> History::getNumMessagesForFriendBeforeDateBoundaries(con
                                                                              const QDate& from,
                                                                              size_t maxNum)
 {
+    if (historyAccessBlocked()) {
+        return {};
+    }
+
     auto friendPkString = friendPk.toString();
 
     // No guarantee that this is the most efficient way to do this...
@@ -954,9 +982,29 @@ QList<History::DateIdx> History::getNumMessagesForFriendBeforeDateBoundaries(con
  */
 void History::markAsDelivered(RowId messageId)
 {
-    if (!isValid()) {
+    if (historyAccessBlocked()) {
         return;
     }
 
     db->execLater(QString("DELETE FROM faux_offline_pending WHERE id=%1;").arg(messageId.get()));
 }
+
+/**
+* @brief Determines if history access should be blocked
+* @return True if history should not be accessed
+*/
+bool History::historyAccessBlocked()
+{
+    if (!Settings::getInstance().getEnableLogging()) {
+        assert(false);
+        qCritical() << "Blocked history access while history is disabled";
+        return true;
+    }
+
+    if (!isValid()) {
+        return true;
+    }
+
+    return false;
+
+}

src/persistence/history.h

@@ -201,6 +201,7 @@ private slots:
     void onFileInserted(RowId dbId, QString fileId);
 
 private:
+    bool historyAccessBlocked();
     static RawDatabase::Query generateFileFinished(RowId fileId, bool success,
                                                    const QString& filePath, const QByteArray& fileHash);
     std::shared_ptr<RawDatabase> db;

src/widget/form/genericchatform.cpp

@@ -983,6 +983,10 @@ void GenericChatForm::searchInBegin(const QString& phrase, const ParameterSearch
         return;
     }
 
+    if (messages.size() == 0) {
+        return;
+    }
+
     if (chatLog.getNextIdx().get() == messages.rbegin()->first.get() + 1) {
         disableSearchText();
     } else {