From fdd7c47c85d5d6dbf21e05e7a0d6afcf383f1d24 Mon Sep 17 00:00:00 2001 From: Ahmad Samir Date: Tue, 15 Sep 2020 20:06:49 +0200 Subject: [PATCH] OpenUrlJob: handle all text scripts consistently Previously we only handled application/x-shellscript, but there are other scripts; a script is technically a file that inherits both text/plain and application/x-executable, e.g. .sh, .csh, .py, perl scripts ...etc. Treat all those mime types the way we handled shell scripts: - if it's not a local url, or isn't executable we open it in the preferred text editor - if it's executable either show the OpenOrExecute dialog or execute directly depending on how the job is configured The mimetype world is a confusing one: - Executables, this includes .exe files (MS Windows); and "application/x-executable" and "application/x-sharedlib", this depends on various parameters (e.g. stripped executables are x-sharedlib, the same executable if not stripped is x-executable...) - Scripts: shell, python, perl... etc scripts, which are text files that can be executed or opened as text. Adjust the unit test. BUG: 425829 BUG: 425177 FIXED-IN: 5.75 --- autotests/openurljobtest.cpp | 56 +++++++++++++++++++++++-------- autotests/openurljobtest.h | 2 ++ src/gui/openurljob.cpp | 65 ++++++++++++++++++++++-------------- 3 files changed, 85 insertions(+), 38 deletions(-) diff --git a/autotests/openurljobtest.cpp b/autotests/openurljobtest.cpp index 2f2ef8ad..ed2211a8 100644 --- a/autotests/openurljobtest.cpp +++ b/autotests/openurljobtest.cpp @@ -103,14 +103,13 @@ void OpenUrlJobTest::initTestCase() KConfigGroup grp = mimeAppsCfg.group("Default Applications"); grp.writeEntry("text/plain", s_tempServiceName); grp.writeEntry("text/html", s_tempServiceName); - grp.writeEntry("application/x-shellscript", s_tempServiceName); grp.sync(); - for (const char *mimeType : {"text/plain", "application/x-shellscript"}) { - KService::Ptr preferredTextEditor = KApplicationTrader::preferredService(QString::fromLatin1(mimeType)); - QVERIFY(preferredTextEditor); - QCOMPARE(preferredTextEditor->entryPath(), m_fakeService); - } + + // "text/plain" encompasses all scripts (shell, python, perl) + KService::Ptr preferredTextEditor = KApplicationTrader::preferredService(QStringLiteral("text/plain")); + QVERIFY(preferredTextEditor); + QCOMPARE(preferredTextEditor->entryPath(), m_fakeService); // As used for preferredService QVERIFY(KService::serviceByDesktopName("openurljobtest_service")); @@ -230,17 +229,38 @@ void OpenUrlJobTest::invalidUrl() QCOMPARE(job2->errorString(), QStringLiteral("Malformed URL\n/pathonly")); } +void OpenUrlJobTest::refuseRunningNativeExecutables_data() +{ + QTest::addColumn("mimeType"); + + // Executables under e.g. /usr/bin/ can be either of these two mimetypes + // see https://gitlab.freedesktop.org/xdg/shared-mime-info/-/issues/11 + QTest::newRow("x-sharedlib") << "application/x-sharedlib"; + QTest::newRow("x-executable") << "application/x-executable"; +} + void OpenUrlJobTest::refuseRunningNativeExecutables() { - KIO::OpenUrlJob *job = new KIO::OpenUrlJob(QUrl::fromLocalFile(QCoreApplication::applicationFilePath()), QStringLiteral("application/x-executable"), this); + QFETCH(QString, mimeType); + + KIO::OpenUrlJob *job = new KIO::OpenUrlJob(QUrl::fromLocalFile(QCoreApplication::applicationFilePath()), mimeType, this); QVERIFY(!job->exec()); QCOMPARE(job->error(), KJob::UserDefinedError); QVERIFY2(job->errorString().contains("For security reasons, launching executables is not allowed in this context."), qPrintable(job->errorString())); } +void OpenUrlJobTest::refuseRunningRemoteNativeExecutables_data() +{ + QTest::addColumn("mimeType"); + QTest::newRow("x-sharedlib") << "application/x-sharedlib"; + QTest::newRow("x-executable") << "application/x-executable"; +} + void OpenUrlJobTest::refuseRunningRemoteNativeExecutables() { - KIO::OpenUrlJob *job = new KIO::OpenUrlJob(QUrl("protocol://host/path/exe"), QStringLiteral("application/x-executable"), this); + QFETCH(QString, mimeType); + + KIO::OpenUrlJob *job = new KIO::OpenUrlJob(QUrl("protocol://host/path/exe"), mimeType, this); job->setRunExecutables(true); // even with this enabled, an error will occur QVERIFY(!job->exec()); QCOMPARE(job->error(), KJob::UserDefinedError); @@ -273,8 +293,11 @@ void OpenUrlJobTest::runScript_data() { QTest::addColumn("mimeType"); + // All text-based scripts inherit text/plain and application/x-executable, no need to test + // all flavours (python, perl, lua, awk ...etc), this sample should be enough QTest::newRow("shellscript") << "application/x-shellscript"; - QTest::newRow("native") << "application/x-executable"; + QTest::newRow("pythonscript") << "text/x-python"; + QTest::newRow("javascript") << "application/javascript"; } void OpenUrlJobTest::runScript() @@ -305,16 +328,23 @@ void OpenUrlJobTest::runScript() void OpenUrlJobTest::runNativeExecutable_data() { + QTest::addColumn("mimeType"); QTest::addColumn("withHandler"); QTest::addColumn("handlerRetVal"); - QTest::newRow("no_handler") << false << false; - QTest::newRow("handler_false") << true << false; - QTest::newRow("handler_true") << true << true; + QTest::newRow("no_handler_x-sharedlib") << "application/x-sharedlib" << false << false; + QTest::newRow("handler_false_x-sharedlib") << "application/x-sharedlib" << true << false; + QTest::newRow("handler_true_x-sharedlib") << "application/x-sharedlib" << true << true; + + QTest::newRow("no_handler_x-executable") << "application/x-executable" << false << false; + QTest::newRow("handler_false_x-executable") << "application/x-executable" << true << false; + QTest::newRow("handler_true_x-executable") << "application/x-executable" << true << true; + } void OpenUrlJobTest::runNativeExecutable() { + QFETCH(QString, mimeType); QFETCH(bool, withHandler); QFETCH(bool, handlerRetVal); @@ -335,7 +365,7 @@ void OpenUrlJobTest::runNativeExecutable() KIO::setDefaultUntrustedProgramHandler(withHandler ? &s_handler : nullptr); // When using OpenUrlJob to run the executable - KIO::OpenUrlJob *job = new KIO::OpenUrlJob(QUrl::fromLocalFile(scriptFile), QStringLiteral("application/x-executable"), this); + KIO::OpenUrlJob *job = new KIO::OpenUrlJob(QUrl::fromLocalFile(scriptFile), mimeType, this); job->setRunExecutables(true); // startProcess tests the case where this isn't set const bool success = job->exec(); diff --git a/autotests/openurljobtest.h b/autotests/openurljobtest.h index e71987d9..f5b9a5be 100644 --- a/autotests/openurljobtest.h +++ b/autotests/openurljobtest.h @@ -26,7 +26,9 @@ private Q_SLOTS: void noServiceNoHandler(); void invalidUrl(); + void refuseRunningNativeExecutables_data(); void refuseRunningNativeExecutables(); + void refuseRunningRemoteNativeExecutables_data(); void refuseRunningRemoteNativeExecutables(); void notAuthorized(); void runScript_data(); diff --git a/src/gui/openurljob.cpp b/src/gui/openurljob.cpp index 8ac187b4..3e35c95c 100644 --- a/src/gui/openurljob.cpp +++ b/src/gui/openurljob.cpp @@ -73,9 +73,9 @@ public: private: void executeCommand(); - void handleExecutables(const QMimeType &mimeType); + void handleBinaries(const QMimeType &mimeType); void handleDesktopFiles(); - void handleShellscripts(); + void handleScripts(); void openInPreferredApp(); void runLink(const QString &filePath, const QString &urlStr, const QString &optionalServiceName); @@ -439,14 +439,29 @@ void KIO::OpenUrlJobPrivate::emitAccessDenied() q->emitResult(); } -// was: KRun::isExecutable (minus application/x-desktop and application/x-shellscript mimetypes). +// was: KRun::isExecutable (minus application/x-desktop mimetype). // Feel free to make public if needed. -static bool isExecutableMime(const QMimeType &mimeType) +static bool isBinary(const QMimeType &mimeType) { - return (mimeType.inherits(QStringLiteral("application/x-executable")) || - /* e.g. /usr/bin/ls, see https://gitlab.freedesktop.org/xdg/shared-mime-info/-/issues/11 */ - mimeType.inherits(QStringLiteral("application/x-sharedlib")) || - mimeType.inherits(QStringLiteral("application/x-ms-dos-executable"))); + // - Binaries could be e.g.: + // - application/x-executable + // - application/x-sharedlib e.g. /usr/bin/ls, see + // https://gitlab.freedesktop.org/xdg/shared-mime-info/-/issues/11 + // + // - Mimetypes that inherit application/x-executable _and_ text/plain are scripts, these are + // handled by handleScripts() + + return (mimeType.inherits(QStringLiteral("application/x-executable")) + || mimeType.inherits(QStringLiteral("application/x-sharedlib")) + || mimeType.inherits(QStringLiteral("application/x-ms-dos-executable"))); +} + +// Helper function that returns whether a file is a text-based script +// e.g. ".sh", ".csh", ".py", ".js" +static bool isTextScript(const QMimeType &mimeType) +{ + return (mimeType.inherits(QStringLiteral("application/x-executable")) + && mimeType.inherits(QStringLiteral("text/plain"))); } // Helper function that returns whether a file has the execute bit set or not. @@ -456,7 +471,7 @@ static bool hasExecuteBit(const QString &fileName) } // Handle native binaries (.e.g. /usr/bin/*); and .exe files -void KIO::OpenUrlJobPrivate::handleExecutables(const QMimeType &mimeType) +void KIO::OpenUrlJobPrivate::handleBinaries(const QMimeType &mimeType) { if (!KAuthorized::authorize(QStringLiteral("shell_access"))) { emitAccessDenied(); @@ -475,11 +490,9 @@ void KIO::OpenUrlJobPrivate::handleExecutables(const QMimeType &mimeType) const QString localPath = m_url.toLocalFile(); - // Check whether file is an executable script -#ifdef Q_OS_WIN - const bool isNativeBinary = !mimeType.inherits(QStringLiteral("text/plain")); -#else - const bool isNativeBinary = !mimeType.inherits(QStringLiteral("text/plain")) && !mimeType.inherits(QStringLiteral("application/x-ms-dos-executable")); + bool isNativeBinary = true; +#ifndef Q_OS_WIN + isNativeBinary = !mimeType.inherits(QStringLiteral("application/x-ms-dos-executable")); #endif if (m_showOpenOrExecuteDialog) { @@ -497,6 +510,8 @@ void KIO::OpenUrlJobPrivate::handleExecutables(const QMimeType &mimeType) } }; + // Ask the user for confirmation before executing this binary (for binaries + // the dialog will only show Execute/Cancel) showOpenOrExecuteFileDialog(dialogFinished); return; } @@ -601,15 +616,15 @@ void KIO::OpenUrlJobPrivate::runUrlWithMimeType() return; } - // Shell scripts - if (mimeType.inherits(QStringLiteral("application/x-shellscript"))) { - handleShellscripts(); + // Scripts (e.g. .sh, .csh, .py, .js) + if (isTextScript(mimeType)) { + handleScripts(); return; } - // Binaries (e.g. /usr/bin/konsole) and .exe files - if (isExecutableMime(mimeType)) { - handleExecutables(mimeType); + // Binaries (e.g. /usr/bin/{konsole,ls}) and .exe files + if (isBinary(mimeType)) { + handleBinaries(mimeType); return; } @@ -677,8 +692,9 @@ void KIO::OpenUrlJobPrivate::handleDesktopFiles() openInPreferredApp(); } -void KIO::OpenUrlJobPrivate::handleShellscripts() +void KIO::OpenUrlJobPrivate::handleScripts() { + // Executable scripts of any type can run arbitrary shell commands if (!KAuthorized::authorize(QStringLiteral("shell_access"))) { emitAccessDenied(); return; @@ -687,8 +703,7 @@ void KIO::OpenUrlJobPrivate::handleShellscripts() const bool isLocal = m_url.isLocalFile(); const QString localPath = m_url.toLocalFile(); if (!isLocal || !hasExecuteBit(localPath)) { - // Open remote shell scripts or ones without the execute bit, with the - // default application + // Open remote scripts or ones without the execute bit, with the default application openInPreferredApp(); return; } @@ -706,7 +721,7 @@ void KIO::OpenUrlJobPrivate::handleShellscripts() return; } - if (m_runExecutables) { // Local executable shell script, proceed + if (m_runExecutables) { // Local executable script, proceed executeCommand(); } else { // Open in the default (text editor) app openInPreferredApp(); @@ -767,7 +782,7 @@ void KIO::OpenUrlJobPrivate::showOpenOrExecuteFileDialog(std::function