Skip to content

Commit b45ad2a

Browse files
committed
#3591 security: apply some fixes
Signed-off-by: Patrizio Bekerle <patrizio@bekerle.com>
1 parent 6623450 commit b45ad2a

9 files changed

Lines changed: 58 additions & 27 deletions

File tree

src/dialogs/linkdialog.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ QString LinkDialog::getURL() const {
220220

221221
if (!url.isEmpty() && !url.contains(QStringLiteral("://")) &&
222222
!url.startsWith(QStringLiteral("./"))) {
223-
url = QStringLiteral("http://") + url;
223+
url = QStringLiteral("https://") + url;
224224
}
225225

226226
return url;

src/dialogs/updatedialog.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -433,12 +433,13 @@ bool UpdateDialog::initializeMacOSUpdateProcess(const QString &releaseUrl) {
433433
return false;
434434
}
435435

436+
// Set restrictive permissions before writing content to prevent other users
437+
// from reading or replacing the script between creation and execution
438+
tempFile->setPermissions(QFile::ExeOwner | QFile::ReadOwner | QFile::WriteOwner);
439+
436440
// write the script content
437441
tempFile->write(scriptContent.toLatin1());
438442

439-
// setting executable permissions to the updater script
440-
tempFile->setPermissions(QFile::ExeUser | QFile::ReadUser | QFile::WriteUser);
441-
442443
// file->fileName() only holds a value after file->open()
443444
QString updaterFilePath = tempFile->fileName();
444445
tempFile->close();

src/libraries/fakevim/fakevim/fakevimhandler.cpp

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -888,20 +888,28 @@ static QString fromLocalEncoding(const QByteArray &data) {
888888
}
889889

890890
static QString getProcessOutput(const QString &command, const QString &input) {
891-
// ensure the executable exists in some standard path and isn't random
892-
const auto fullCmdPath = QStandardPaths::findExecutable(command);
893-
if (fullCmdPath.isEmpty()) {
894-
return {};
895-
}
896-
897891
QProcess proc;
898892
#if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
899893
QStringList arguments = QProcess::splitCommand(command);
900-
QString executable = arguments.takeFirst();
901-
proc.start(executable, arguments);
894+
if (arguments.isEmpty()) {
895+
return {};
896+
}
897+
const QString executable = arguments.takeFirst();
902898
#else
903-
proc.start(command);
899+
// On Qt < 5.15, manually split on whitespace to avoid shell interpretation
900+
QStringList arguments = command.split(QLatin1Char(' '), Qt::SkipEmptyParts);
901+
if (arguments.isEmpty()) {
902+
return {};
903+
}
904+
const QString executable = arguments.takeFirst();
904905
#endif
906+
// Ensure the executable exists in some standard path and isn't random
907+
const auto fullCmdPath = QStandardPaths::findExecutable(executable);
908+
if (fullCmdPath.isEmpty()) {
909+
return {};
910+
}
911+
912+
proc.start(fullCmdPath, arguments);
905913
proc.waitForStarted();
906914
proc.write(toLocalEncoding(input));
907915
proc.closeWriteChannel();

src/libraries/qmarkdowntextedit

src/services/databaseservice.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,17 @@ bool DatabaseService::mergeNoteFolderDatabase(const QString& path) {
10561056
*/
10571057
QByteArray DatabaseService::generateDatabaseTableSha1Signature(QSqlDatabase& db,
10581058
const QString& table) {
1059+
// Whitelist of valid table names to prevent SQL injection via table name concatenation
1060+
static const QStringList validTables = {
1061+
QStringLiteral("note"), QStringLiteral("noteSubFolder"), QStringLiteral("tag"),
1062+
QStringLiteral("noteTagLink"), QStringLiteral("noteFolder"), QStringLiteral("bookmark"),
1063+
QStringLiteral("calendarItem"),
1064+
};
1065+
if (!validTables.contains(table)) {
1066+
qCritical() << __func__ << ": invalid table name rejected:" << table;
1067+
return QByteArray();
1068+
}
1069+
10591070
QCryptographicHash hash(QCryptographicHash::Sha1);
10601071
QSqlQuery query(db);
10611072
query.prepare(QStringLiteral("SELECT * FROM ") + table);

src/services/mcpservice.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ void McpService::sendHttpResponse(QTcpSocket *socket, int statusCode, const QByt
197197
"HTTP/1.1 %1 %2\r\n"
198198
"Content-Type: %3\r\n"
199199
"Content-Length: %4\r\n"
200-
"Access-Control-Allow-Origin: *\r\n"
200+
"Access-Control-Allow-Origin: http://localhost\r\n"
201201
"Access-Control-Allow-Methods: GET, POST, OPTIONS\r\n"
202202
"Access-Control-Allow-Headers: Content-Type, Authorization\r\n"
203203
"Cache-Control: no-store\r\n"
@@ -214,7 +214,7 @@ void McpService::sendHttpResponse(QTcpSocket *socket, int statusCode, const QByt
214214
void McpService::sendCorsHeaders(QTcpSocket *socket) {
215215
const QByteArray response =
216216
"HTTP/1.1 204 No Content\r\n"
217-
"Access-Control-Allow-Origin: *\r\n"
217+
"Access-Control-Allow-Origin: http://localhost\r\n"
218218
"Access-Control-Allow-Methods: GET, POST, OPTIONS\r\n"
219219
"Access-Control-Allow-Headers: Content-Type, Authorization\r\n"
220220
"Access-Control-Max-Age: 86400\r\n"
@@ -243,7 +243,7 @@ void McpService::openSseStream(QTcpSocket *socket) {
243243
"HTTP/1.1 200 OK\r\n"
244244
"Content-Type: text/event-stream\r\n"
245245
"Cache-Control: no-cache\r\n"
246-
"Access-Control-Allow-Origin: *\r\n"
246+
"Access-Control-Allow-Origin: http://localhost\r\n"
247247
"Connection: keep-alive\r\n"
248248
"\r\n";
249249

src/services/owncloudservice.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,8 @@ void OwnCloudService::startAppVersionTest() {
600600
*/
601601
void OwnCloudService::ignoreSslErrorsIfAllowed(QNetworkReply *reply) {
602602
SettingsService settings;
603-
if (settings.value(QStringLiteral("networking/ignoreSSLErrors"), true).toBool()) {
603+
// Default to false to prevent MITM attacks on fresh installs
604+
if (settings.value(QStringLiteral("networking/ignoreSSLErrors"), false).toBool()) {
604605
QObject::connect(reply, SIGNAL(sslErrors(QList<QSslError>)), reply,
605606
SLOT(ignoreSslErrors()));
606607
}
@@ -988,7 +989,8 @@ void OwnCloudService::restoreTrashedNoteOnServer(const QString &fileName, int ti
988989
q.addQueryItem(QStringLiteral("timestamp"), QString::number(timestamp));
989990
url.setQuery(q);
990991

991-
qDebug() << url;
992+
// Do not log the URL here — it contains the plaintext password
993+
qDebug() << __func__ << "- restoring trashed note:" << fileName;
992994

993995
QNetworkRequest r(url);
994996
addAuthHeader(&r);

src/utils/gui.cpp

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1365,16 +1365,24 @@ bool Utils::Gui::doLinuxDarkModeCheck() {
13651365
// QStringLiteral("string:'org.freedesktop.appearance'
13661366
// string:'color-scheme'");
13671367

1368+
// Use dbus-send directly instead of /bin/sh -c to avoid shell interpretation
1369+
const QString dbusExecutable = QStandardPaths::findExecutable(QStringLiteral("dbus-send"));
1370+
if (dbusExecutable.isEmpty()) {
1371+
qWarning() << __func__ << " - 'dbus-send' not found, doLinuxDarkModeCheck returned false";
1372+
return false;
1373+
}
1374+
13681375
auto parameters = QStringList()
1369-
<< "-c"
1370-
<< "dbus-send --session --print-reply=literal --reply-timeout=1000 "
1371-
"--dest=org.freedesktop.portal.Desktop /org/freedesktop/portal/desktop "
1372-
"org.freedesktop.portal.Settings.Read string:'org.freedesktop.appearance' "
1373-
"string:'color-scheme'";
1376+
<< QStringLiteral("--session") << QStringLiteral("--print-reply=literal")
1377+
<< QStringLiteral("--reply-timeout=1000")
1378+
<< QStringLiteral("--dest=org.freedesktop.portal.Desktop")
1379+
<< QStringLiteral("/org/freedesktop/portal/desktop")
1380+
<< QStringLiteral("org.freedesktop.portal.Settings.Read")
1381+
<< QStringLiteral("string:org.freedesktop.appearance")
1382+
<< QStringLiteral("string:color-scheme");
13741383

13751384
QProcess process;
1376-
// process.start(QStringLiteral("dbus-send"), parameters);
1377-
process.start(QStringLiteral("/bin/sh"), parameters);
1385+
process.start(dbusExecutable, parameters);
13781386

13791387
if (!process.waitForStarted()) {
13801388
qWarning() << __func__ << " - 'doLinuxDarkModeCheck' returned false";

src/widgets/settings/networksettingswidget.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ void NetworkSettingsWidget::readSettings() {
4646
ui->appHeartbeatCheckBox->setChecked(
4747
settings.value(QStringLiteral("appMetrics/disableAppHeartbeat")).toBool());
4848

49+
// Default to false to prevent MITM attacks on fresh installs
4950
bool ignoreSSLErrors =
50-
settings.value(QStringLiteral("networking/ignoreSSLErrors"), true).toBool();
51+
settings.value(QStringLiteral("networking/ignoreSSLErrors"), false).toBool();
5152
ui->ignoreSSLErrorsCheckBox->setChecked(ignoreSSLErrors);
5253
ui->letsEncryptInfoLabel->setVisible(ignoreSSLErrors);
5354

0 commit comments

Comments
 (0)