File 0001-QProcess-Unix-ensure-we-don-t-accidentally... of Package libqt5-qtbase

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-QProcess-Unix-ensure-we-don-t-accidentally-execute-s.patch of Package libqt5-qtbase (Revision fcee5e9e51858635fd51454af6ea42aa)

Currently displaying revision fcee5e9e51858635fd51454af6ea42aa , Show latest

From 96cdea05674e782a1f1a3fbb1e77676d314f6900 Mon Sep 17 00:00:00 2001
From: Thiago Macieira <thiago.macieira@intel.com>
Date: Mon, 31 Jan 2022 11:00:19 -0800
Subject: [PATCH] QProcess/Unix: ensure we don't accidentally execute something
 from CWD

Unless "." (or the empty string) is in $PATH, we're not supposed to find
executables in the current directory. This is how the Unix shells behave
and we match their behavior. It's also the behavior Qt had prior to 5.9
(commit 28666d167aa8e602c0bea25ebc4d51b55005db13). On Windows, searching
the current directory is the norm, so we keep that behavior.

This commit does not add an explicit check for an empty return from
QStandardPaths::findExecutable(). Instead, we allow that empty string to
go all the way to execve(2), which will fail with ENOENT. We could catch
it early, before fork(2), but why add code for the error case?

execve() calls in strace of the new test:

[pid 38201] execve("", [""], 0x12100d0 /* 120 vars */) = -1 ENOENT (No such file or directory)
[pid 38201] +++ exited with 255 +++
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=38201, si_uid=1000, si_status=255, si_utime=0, si_stime=0} ---
PASS   : tst_QProcess::startFromCurrentWorkingDir(without-dot-slash)

[pid 38202] execve("./testProcessNormal", ["./testProcessNormal"], 0x12100d0 /* 120 vars */) = 0
[pid 38202] +++ exited with 0 +++
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=38202, si_uid=1000, si_status=0, si_utime=0, si_stime=0} ---
PASS   : tst_QProcess::startFromCurrentWorkingDir(with-dot-slash)

See https://kde.org/info/security/advisory-20220131-1.txt

[ChangeLog][Important Behavior Changes] When passed a simple program
name with no slashes, QProcess on Unix systems will now only search the
current directory if "." is one of the entries in the PATH environment
variable. This bug fix restores the behavior QProcess had before Qt 5.9.
If launching an executable in the current path is intended, pass a
program name starting with "./". To run a binary auxiliary to the
application, instead calculate an absolute path starting from
QCoreApplication::applicationDirPath().

Pick-to: 5.15 6.2 6.3
Change-Id: I54f205f6b7314351b078fffd16cf7013c97ee9fb
---
 src/corelib/io/qprocess_unix.cpp              | 13 +++---
 .../auto/corelib/io/qprocess/tst_qprocess.cpp | 45 ++++++++++++++++++-
 .../corelib/kernel/qobject/tst_qobject.cpp    |  2 +-
 .../kernel/qapplication/tst_qapplication.cpp  |  4 +-
 4 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index 50390e57f5..94d32076d0 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -422,14 +422,15 @@ void QProcessPrivate::startProcess()
     // Add the program name to the argument list.
     argv[0] = nullptr;
     if (!program.contains(QLatin1Char('/'))) {
+        // findExecutable() returns its argument if it's an absolute path,
+        // otherwise it searches $PATH; returns empty if not found (we handle
+        // that case much later)
         const QString &exeFilePath = QStandardPaths::findExecutable(program);
-        if (!exeFilePath.isEmpty()) {
-            const QByteArray &tmp = QFile::encodeName(exeFilePath);
-            argv[0] = ::strdup(tmp.constData());
-        }
-    }
-    if (!argv[0])
+        const QByteArray &tmp = QFile::encodeName(exeFilePath);
+        argv[0] = ::strdup(tmp.constData());
+    } else {
         argv[0] = ::strdup(encodedProgramName.constData());
+    }
 
     // Add every argument to the list
     for (int i = 0; i < arguments.count(); ++i)
diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
index bc9df3f1f3..17b148379f 100644
--- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
+++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
@@ -1,7 +1,7 @@
 /****************************************************************************
 **
 ** Copyright (C) 2020 The Qt Company Ltd.
-** Copyright (C) 2020 Intel Corporation.
+** Copyright (C) 2022 Intel Corporation.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the test suite of the Qt Toolkit.
@@ -149,6 +149,8 @@ private slots:
     void startStopStartStopBuffers();
     void processEventsInAReadyReadSlot_data();
     void processEventsInAReadyReadSlot();
+    void startFromCurrentWorkingDir_data();
+    void startFromCurrentWorkingDir();
 
     // keep these at the end, since they use lots of processes and sometimes
     // caused obscure failures to occur in tests that followed them (esp. on the Mac)
@@ -2732,5 +2734,46 @@ void tst_QProcess::finishProcessBeforeReadingDone_deprecated()
 
 #endif
 
+void tst_QProcess::startFromCurrentWorkingDir_data()
+{
+    QTest::addColumn<QString>("program");
+    QTest::addColumn<bool>("success");  // on Unix
+    QTest::newRow("without-dot-slash") << "testProcessNormal" << false;
+    QTest::newRow("with-dot-slash") << "./testProcessNormal" << true;
+}
+
+void tst_QProcess::startFromCurrentWorkingDir()
+{
+    QFETCH(QString, program);
+    QFETCH(bool, success);
+#ifdef Q_OS_WIN
+    // Windows always searches the current working dir
+    success = true;
+#endif
+
+    QProcess process;
+    qRegisterMetaType<QProcess::ProcessError>();
+    QSignalSpy errorSpy(&process, &QProcess::errorOccurred);
+    QVERIFY(errorSpy.isValid());
+
+    process.setWorkingDirectory("testProcessNormal");
+    process.setProgram(program);
+
+#ifdef Q_OS_UNIX
+    // reset PATH, to be sure it doesn't contain . or the empty path
+    QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
+    env.insert("PATH", "/");    // doesn't matter what it is
+#endif
+
+    process.start();
+    QCOMPARE(process.waitForStarted(), success);
+    QCOMPARE(errorSpy.count(), int(!success));
+    if (success) {
+        QVERIFY(process.waitForFinished());
+    } else {
+        QCOMPARE(process.error(), QProcess::FailedToStart);
+    }
+}
+
 QTEST_MAIN(tst_QProcess)
 #include "tst_qprocess.moc"
diff --git a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
index 63d06497ce..9b6287e885 100644
--- a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
+++ b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
@@ -3024,7 +3024,7 @@ void tst_QObject::recursiveSignalEmission()
 #else
     QProcess proc;
     // signalbug helper app should always be next to this test binary
-    const QString path = QStringLiteral("signalbug_helper");
+    const QString path = QCoreApplication::applicationDirPath() + QDir::separator() + QStringLiteral("signalbug_helper");
     proc.start(path);
     QVERIFY2(proc.waitForStarted(), qPrintable(QString::fromLatin1("Cannot start '%1': %2").arg(path, proc.errorString())));
     QVERIFY(proc.waitForFinished());
diff --git a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp b/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp
index e159e22d2a..820831f9c5 100644
--- a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp
+++ b/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp
@@ -1449,7 +1449,7 @@ void tst_QApplication::desktopSettingsAware()
 {
 #if QT_CONFIG(process)
     QProcess testProcess;
-    testProcess.start("desktopsettingsaware_helper");
+    testProcess.start("./desktopsettingsaware_helper");
     QVERIFY2(testProcess.waitForStarted(),
              qPrintable(QString::fromLatin1("Cannot start 'desktopsettingsaware_helper': %1").arg(testProcess.errorString())));
     QVERIFY(testProcess.waitForFinished(10000));
@@ -2365,7 +2365,7 @@ void tst_QApplication::qtbug_12673()
 #if QT_CONFIG(process)
     QProcess testProcess;
     QStringList arguments;
-    testProcess.start("modal_helper", arguments);
+    testProcess.start("./modal_helper", arguments);
     QVERIFY2(testProcess.waitForStarted(),
              qPrintable(QString::fromLatin1("Cannot start 'modal_helper': %1").arg(testProcess.errorString())));
     QVERIFY(testProcess.waitForFinished(20000));
-- 
2.34.0

Places

File 0001-QProcess-Unix-ensure-we-don-t-accidentally-execute-s.patch of Package libqt5-qtbase (Revision fcee5e9e51858635fd51454af6ea42aa)

Places