Skip to content

Commit

Permalink
Merge pull request qTox#5828
Browse files Browse the repository at this point in the history 
Mick Sayson (1):
      fix(history): Prevent invalid history access
  • Loading branch information
@anthonybilinski
anthonybilinski committed Dec 2, 2019
2 parents 9fa6a49 + e3e6e1d commit 7be3277
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 28 deletions.
104 changes: 76 additions & 28 deletions src/persistence/history.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,30 +195,29 @@ 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;

if (!db->execNow(RawDatabase::Query("PRAGMA user_version", [&](const QVector<QVariant>& row) {
databaseSchemaVersion = row[0].toLongLong();
}))) {
qCritical() << "History failed to read user_version";
db.reset();
return;
return false;
}

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;
return false;
} else if (databaseSchemaVersion == SCHEMA_VERSION) {
// No work to do
return;
return true;
}

switch (databaseSchemaVersion) {
Expand All @@ -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;
return false;
}
if (newDb) {
if (!createCurrentSchema(*db)) {
qCritical() << "Failed to create current db schema";
db.reset();
return;
return false;
}
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;
return false;
}
qDebug() << "Database upgraded incrementally to schema version 1";
}
Expand All @@ -255,30 +251,29 @@ 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;
return false;
}
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;
return false;
}
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;
return false;
}
qDebug() << "Database upgraded incrementally to schema version 4";
// etc.
default:
qInfo() << "Database upgrade finished (databaseSchemaVersion" << databaseSchemaVersion
<< "->" << SCHEMA_VERSION << ")";
}

return true;
}

MessageState getMessageState(bool isPending, bool isBroken)
Expand Down Expand Up @@ -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)
: db(db)
History::History(std::shared_ptr<RawDatabase> 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;
}

Expand Down Expand Up @@ -370,6 +366,10 @@ bool History::isValid()
*/
bool History::historyExists(const ToxPk& friendPk)
{
if (historyAccessBlocked()) {
return false;
}

return !getMessagesForFriend(friendPk, 0, 1).empty();
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()) {
qWarning() << "Blocked a message from being added to database while history is disabled";
return;
}
if (!isValid()) {
if (historyAccessBlocked()) {
return;
}

Expand All @@ -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;
Expand All @@ -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 "
Expand All @@ -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!
Expand Down Expand Up @@ -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 "
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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...
Expand Down Expand Up @@ -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;

}
1 change: 1 addition & 0 deletions src/persistence/history.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions src/widget/form/genericchatform.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 7be3277

Please sign in to comment.