Fix permissions changing on database save

* Saving a database in unsafe mode retains the existing permissions on the kdbx file
* New databases (save as, save backup, new database) and new key files are saved with 0600 permissions (user read/write), fixes #2575
This commit is contained in:
Jonathan White 2020-06-06 09:55:45 -04:00
parent 1ad0184473
commit fbebf30b98
3 changed files with 16 additions and 4 deletions

View File

@ -253,10 +253,14 @@ bool Database::saveAs(const QString& filePath, QString* error, bool atomic, bool
QFileInfo fileInfo(filePath);
auto realFilePath = fileInfo.exists() ? fileInfo.canonicalFilePath() : fileInfo.absoluteFilePath();
bool isNewFile = !QFile::exists(realFilePath);
bool ok = AsyncTask::runAndWaitForFuture([&] { return performSave(realFilePath, error, atomic, backup); });
if (ok) {
markAsClean();
setFilePath(filePath);
if (isNewFile) {
QFile::setPermissions(realFilePath, QFile::ReadUser | QFile::WriteUser);
}
m_fileWatcher->start(realFilePath, 30, 1);
} else {
// Saving failed, don't rewatch file since it does not represent our database
@ -304,6 +308,7 @@ bool Database::performSave(const QString& filePath, QString* error, bool atomic,
}
// Delete the original db and move the temp file in place
auto perms = QFile::permissions(filePath);
QFile::remove(filePath);
// Note: call into the QFile rename instead of QTemporaryFile
@ -312,6 +317,7 @@ bool Database::performSave(const QString& filePath, QString* error, bool atomic,
if (tempFile.QFile::rename(filePath)) {
// successfully saved the database
tempFile.setAutoRemove(false);
QFile::setPermissions(filePath, perms);
return true;
} else if (!backup || !restoreDatabase(filePath)) {
// Failed to copy new database in place, and
@ -454,9 +460,12 @@ bool Database::backupDatabase(const QString& filePath)
auto match = re.match(filePath);
auto backupFilePath = filePath;
auto perms = QFile::permissions(filePath);
backupFilePath = backupFilePath.replace(re, "") + ".old" + match.captured(1);
QFile::remove(backupFilePath);
return QFile::copy(filePath, backupFilePath);
bool res = QFile::copy(filePath, backupFilePath);
QFile::setPermissions(backupFilePath, perms);
return res;
}
/**
@ -472,11 +481,13 @@ bool Database::restoreDatabase(const QString& filePath)
static auto re = QRegularExpression("^(.*?)(\\.[^.]+)?$");
auto match = re.match(filePath);
auto perms = QFile::permissions(filePath);
auto backupFilePath = match.captured(1) + ".old" + match.captured(2);
// Only try to restore if the backup file actually exists
if (QFile::exists(backupFilePath)) {
QFile::remove(filePath);
return QFile::copy(backupFilePath, filePath);
QFile::setPermissions(filePath, perms);
}
return false;
}

View File

@ -200,6 +200,7 @@ bool FileKey::create(const QString& fileName, QString* errorMsg, int size)
}
create(&file, size);
file.close();
file.setPermissions(QFile::ReadUser);
if (file.error()) {
if (errorMsg) {

View File

@ -680,7 +680,7 @@ void TestGuiFdoSecrets::testCollectionCreate()
QCOMPARE(spyCollectionCreated.count(), 1);
{
auto args = spyCollectionCreated.takeFirst();
args = spyCollectionCreated.takeFirst();
QCOMPARE(args.size(), 1);
QCOMPARE(args.at(0).value<Collection*>(), coll);
}
@ -754,7 +754,7 @@ void TestGuiFdoSecrets::testCollectionDelete()
QCOMPARE(spyCollectionDeleted.count(), 1);
{
auto args = spyCollectionDeleted.takeFirst();
args = spyCollectionDeleted.takeFirst();
QCOMPARE(args.size(), 1);
QCOMPARE(args.at(0).value<Collection*>(), rawColl);
}
@ -977,7 +977,7 @@ void TestGuiFdoSecrets::testItemDelete()
QCOMPARE(spyItemDeleted.count(), 1);
{
auto args = spyItemDeleted.takeFirst();
args = spyItemDeleted.takeFirst();
QCOMPARE(args.size(), 1);
QCOMPARE(args.at(0).value<Item*>(), rawItem);
}