forked from pool/libqt5-qtbase
Accepting request 954216 from KDE:Qt:5.15
- Update patch after it was merged to dev upstream and fix another place missed in the first version (boo#1195386, CVE-2022-23853): * 0001-QProcess-Unix-ensure-we-don-t-accidentally-execute-s.patch OBS-URL: https://build.opensuse.org/request/show/954216 OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/libqt5-qtbase?expand=0&rev=127
This commit is contained in:
commit
b6b89216df
@ -1,8 +1,11 @@
|
|||||||
From 96cdea05674e782a1f1a3fbb1e77676d314f6900 Mon Sep 17 00:00:00 2001
|
From ab3e0383b9de49c61bc49f5fbef80d1409fcafde Mon Sep 17 00:00:00 2001
|
||||||
From: Thiago Macieira <thiago.macieira@intel.com>
|
From: Thiago Macieira <thiago.macieira@intel.com>
|
||||||
Date: Mon, 31 Jan 2022 11:00:19 -0800
|
Date: Mon, 31 Jan 2022 11:00:19 -0800
|
||||||
Subject: [PATCH] QProcess/Unix: ensure we don't accidentally execute something
|
Subject: [PATCH] QProcess/Unix: ensure we don't accidentally execute something
|
||||||
from CWD
|
from CWD
|
||||||
|
MIME-Version: 1.0
|
||||||
|
Content-Type: text/plain; charset=UTF-8
|
||||||
|
Content-Transfer-Encoding: 8bit
|
||||||
|
|
||||||
Unless "." (or the empty string) is in $PATH, we're not supposed to find
|
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
|
executables in the current directory. This is how the Unix shells behave
|
||||||
@ -15,42 +18,46 @@ QStandardPaths::findExecutable(). Instead, we allow that empty string to
|
|||||||
go all the way to execve(2), which will fail with ENOENT. We could catch
|
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?
|
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
|
See https://kde.org/info/security/advisory-20220131-1.txt
|
||||||
|
|
||||||
[ChangeLog][Important Behavior Changes] When passed a simple program
|
[ChangeLog][Important Behavior Changes] When passed a simple program
|
||||||
name with no slashes, QProcess on Unix systems will now only search the
|
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
|
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.
|
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
|
If launching an executable in the directory set by setWorkingDirectory()
|
||||||
program name starting with "./". To run a binary auxiliary to the
|
or inherited from the parent is intended, pass a program name starting
|
||||||
application, instead calculate an absolute path starting from
|
with "./". For more information and best practices about finding an
|
||||||
QCoreApplication::applicationDirPath().
|
executable, see QProcess' documentation.
|
||||||
|
|
||||||
Pick-to: 5.15 6.2 6.3
|
Pick-to: 5.15 6.2 6.3
|
||||||
Change-Id: I54f205f6b7314351b078fffd16cf7013c97ee9fb
|
Change-Id: I54f205f6b7314351b078fffd16cf7013c97ee9fb
|
||||||
|
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
|
||||||
|
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
|
||||||
|
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
|
||||||
|
(cherry picked from commit 29fceed2ffb41954a63001414bd042611f2d4980)
|
||||||
|
|
||||||
|
Note: This currently breaks various autotests, as they rely on the test
|
||||||
|
helpers (same directory as the test) to be in $PATH. In Qt 6, the CMake
|
||||||
|
test code sets this explicitly, which is not the case in Qt 5 (yet).
|
||||||
---
|
---
|
||||||
src/corelib/io/qprocess_unix.cpp | 13 +++---
|
src/corelib/io/qprocess_unix.cpp | 24 ++---
|
||||||
.../auto/corelib/io/qprocess/tst_qprocess.cpp | 45 ++++++++++++++++++-
|
.../auto/corelib/io/qprocess/tst_qprocess.cpp | 93 ++++++++++++++++++-
|
||||||
.../corelib/kernel/qobject/tst_qobject.cpp | 2 +-
|
|
||||||
.../kernel/qapplication/tst_qapplication.cpp | 4 +-
|
.../kernel/qapplication/tst_qapplication.cpp | 4 +-
|
||||||
4 files changed, 54 insertions(+), 10 deletions(-)
|
3 files changed, 107 insertions(+), 14 deletions(-)
|
||||||
|
|
||||||
diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
|
diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
|
||||||
index 50390e57f5..94d32076d0 100644
|
index 50390e57f5..5bd3b7f297 100644
|
||||||
--- a/src/corelib/io/qprocess_unix.cpp
|
--- a/src/corelib/io/qprocess_unix.cpp
|
||||||
+++ b/src/corelib/io/qprocess_unix.cpp
|
+++ b/src/corelib/io/qprocess_unix.cpp
|
||||||
|
@@ -1,7 +1,7 @@
|
||||||
|
/****************************************************************************
|
||||||
|
**
|
||||||
|
** Copyright (C) 2016 The Qt Company Ltd.
|
||||||
|
-** Copyright (C) 2016 Intel Corporation.
|
||||||
|
+** Copyright (C) 2022 Intel Corporation.
|
||||||
|
** Contact: https://www.qt.io/licensing/
|
||||||
|
**
|
||||||
|
** This file is part of the QtCore module of the Qt Toolkit.
|
||||||
@@ -422,14 +422,15 @@ void QProcessPrivate::startProcess()
|
@@ -422,14 +422,15 @@ void QProcessPrivate::startProcess()
|
||||||
// Add the program name to the argument list.
|
// Add the program name to the argument list.
|
||||||
argv[0] = nullptr;
|
argv[0] = nullptr;
|
||||||
@ -73,8 +80,25 @@ index 50390e57f5..94d32076d0 100644
|
|||||||
|
|
||||||
// Add every argument to the list
|
// Add every argument to the list
|
||||||
for (int i = 0; i < arguments.count(); ++i)
|
for (int i = 0; i < arguments.count(); ++i)
|
||||||
|
@@ -985,11 +986,12 @@ bool QProcessPrivate::startDetached(qint64 *pid)
|
||||||
|
|
||||||
|
QByteArray tmp;
|
||||||
|
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())
|
||||||
|
- tmp = QFile::encodeName(exeFilePath);
|
||||||
|
- }
|
||||||
|
- if (tmp.isEmpty())
|
||||||
|
+ tmp = QFile::encodeName(exeFilePath);
|
||||||
|
+ } else
|
||||||
|
tmp = QFile::encodeName(program);
|
||||||
|
argv[0] = tmp.data();
|
||||||
|
|
||||||
diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
|
diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
|
||||||
index bc9df3f1f3..17b148379f 100644
|
index bc9df3f1f3..33051d3803 100644
|
||||||
--- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
|
--- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
|
||||||
+++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
|
+++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
|
||||||
@@ -1,7 +1,7 @@
|
@@ -1,7 +1,7 @@
|
||||||
@ -95,42 +119,90 @@ index bc9df3f1f3..17b148379f 100644
|
|||||||
|
|
||||||
// keep these at the end, since they use lots of processes and sometimes
|
// 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)
|
// caused obscure failures to occur in tests that followed them (esp. on the Mac)
|
||||||
@@ -2732,5 +2734,46 @@ void tst_QProcess::finishProcessBeforeReadingDone_deprecated()
|
@@ -2732,5 +2734,94 @@ void tst_QProcess::finishProcessBeforeReadingDone_deprecated()
|
||||||
|
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
+enum class ChdirMode {
|
||||||
|
+ None = 0,
|
||||||
|
+ InParent,
|
||||||
|
+ InChild
|
||||||
|
+};
|
||||||
|
+Q_DECLARE_METATYPE(ChdirMode)
|
||||||
|
+
|
||||||
+void tst_QProcess::startFromCurrentWorkingDir_data()
|
+void tst_QProcess::startFromCurrentWorkingDir_data()
|
||||||
+{
|
+{
|
||||||
+ QTest::addColumn<QString>("program");
|
+ qRegisterMetaType<ChdirMode>();
|
||||||
+ QTest::addColumn<bool>("success"); // on Unix
|
+ QTest::addColumn<QString>("programPrefix");
|
||||||
+ QTest::newRow("without-dot-slash") << "testProcessNormal" << false;
|
+ QTest::addColumn<ChdirMode>("chdirMode");
|
||||||
+ QTest::newRow("with-dot-slash") << "./testProcessNormal" << true;
|
+ QTest::addColumn<bool>("success");
|
||||||
|
+
|
||||||
|
+ constexpr bool IsWindows = true
|
||||||
|
+#ifdef Q_OS_UNIX
|
||||||
|
+ && false
|
||||||
|
+#endif
|
||||||
|
+ ;
|
||||||
|
+
|
||||||
|
+ // baseline: trying to execute the directory, this can't possibly succeed!
|
||||||
|
+ QTest::newRow("plain-same-cwd") << QString() << ChdirMode::None << false;
|
||||||
|
+
|
||||||
|
+ // cross-platform behavior: neither OS searches the setWorkingDirectory()
|
||||||
|
+ // dir without "./"
|
||||||
|
+ QTest::newRow("plain-child-chdir") << QString() << ChdirMode::InChild << false;
|
||||||
|
+
|
||||||
|
+ // cross-platform behavior: both OSes search the parent's CWD with "./"
|
||||||
|
+ QTest::newRow("prefixed-parent-chdir") << "./" << ChdirMode::InParent << true;
|
||||||
|
+
|
||||||
|
+ // opposite behaviors: Windows searches the parent's CWD and Unix searches
|
||||||
|
+ // the child's with "./"
|
||||||
|
+ QTest::newRow("prefixed-child-chdir") << "./" << ChdirMode::InChild << !IsWindows;
|
||||||
|
+
|
||||||
|
+ // Windows searches the parent's CWD without "./"
|
||||||
|
+ QTest::newRow("plain-parent-chdir") << QString() << ChdirMode::InParent << IsWindows;
|
||||||
+}
|
+}
|
||||||
+
|
+
|
||||||
+void tst_QProcess::startFromCurrentWorkingDir()
|
+void tst_QProcess::startFromCurrentWorkingDir()
|
||||||
+{
|
+{
|
||||||
+ QFETCH(QString, program);
|
+ QFETCH(QString, programPrefix);
|
||||||
|
+ QFETCH(ChdirMode, chdirMode);
|
||||||
+ QFETCH(bool, success);
|
+ QFETCH(bool, success);
|
||||||
+#ifdef Q_OS_WIN
|
|
||||||
+ // Windows always searches the current working dir
|
|
||||||
+ success = true;
|
|
||||||
+#endif
|
|
||||||
+
|
+
|
||||||
+ QProcess process;
|
+ QProcess process;
|
||||||
+ qRegisterMetaType<QProcess::ProcessError>();
|
+ qRegisterMetaType<QProcess::ProcessError>();
|
||||||
+ QSignalSpy errorSpy(&process, &QProcess::errorOccurred);
|
+ QSignalSpy errorSpy(&process, &QProcess::errorOccurred);
|
||||||
+ QVERIFY(errorSpy.isValid());
|
+ QVERIFY(errorSpy.isValid());
|
||||||
+
|
+
|
||||||
+ process.setWorkingDirectory("testProcessNormal");
|
+ // both the dir name and the executable name
|
||||||
+ process.setProgram(program);
|
+ const QString target = QStringLiteral("testProcessNormal");
|
||||||
|
+ process.setProgram(programPrefix + target);
|
||||||
+
|
+
|
||||||
+#ifdef Q_OS_UNIX
|
+#ifdef Q_OS_UNIX
|
||||||
+ // reset PATH, to be sure it doesn't contain . or the empty path
|
+ // Reset PATH, to be sure it doesn't contain . or the empty path.
|
||||||
+ QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
|
+ // We can't do this on Windows because DLLs are searched in PATH
|
||||||
+ env.insert("PATH", "/"); // doesn't matter what it is
|
+ // and Windows always searches "." anyway.
|
||||||
|
+ auto restoreEnv = qScopeGuard([old = qgetenv("PATH")] {
|
||||||
|
+ qputenv("PATH", old);
|
||||||
|
+ });
|
||||||
|
+ qputenv("PATH", "/");
|
||||||
+#endif
|
+#endif
|
||||||
+
|
+
|
||||||
+ process.start();
|
+ switch (chdirMode) {
|
||||||
|
+ case ChdirMode::InParent: {
|
||||||
|
+ auto restoreCwd = qScopeGuard([old = QDir::currentPath()] {
|
||||||
|
+ QDir::setCurrent(old);
|
||||||
|
+ });
|
||||||
|
+ QVERIFY(QDir::setCurrent(target));
|
||||||
|
+ process.start();
|
||||||
|
+ break;
|
||||||
|
+ }
|
||||||
|
+ case ChdirMode::InChild:
|
||||||
|
+ process.setWorkingDirectory(target);
|
||||||
|
+ Q_FALLTHROUGH();
|
||||||
|
+ case ChdirMode::None:
|
||||||
|
+ process.start();
|
||||||
|
+ break;
|
||||||
|
+ }
|
||||||
|
+
|
||||||
+ QCOMPARE(process.waitForStarted(), success);
|
+ QCOMPARE(process.waitForStarted(), success);
|
||||||
+ QCOMPARE(errorSpy.count(), int(!success));
|
+ QCOMPARE(errorSpy.count(), int(!success));
|
||||||
+ if (success) {
|
+ if (success) {
|
||||||
@ -142,19 +214,6 @@ index bc9df3f1f3..17b148379f 100644
|
|||||||
+
|
+
|
||||||
QTEST_MAIN(tst_QProcess)
|
QTEST_MAIN(tst_QProcess)
|
||||||
#include "tst_qprocess.moc"
|
#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
|
diff --git a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp b/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp
|
||||||
index e159e22d2a..820831f9c5 100644
|
index e159e22d2a..820831f9c5 100644
|
||||||
--- a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp
|
--- a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp
|
||||||
|
@ -1,3 +1,10 @@
|
|||||||
|
-------------------------------------------------------------------
|
||||||
|
Mon Feb 14 10:44:42 UTC 2022 - Fabian Vogt <fabian@ritter-vogt.de>
|
||||||
|
|
||||||
|
- Update patch after it was merged to dev upstream and fix another
|
||||||
|
place missed in the first version (boo#1195386, CVE-2022-23853):
|
||||||
|
* 0001-QProcess-Unix-ensure-we-don-t-accidentally-execute-s.patch
|
||||||
|
|
||||||
-------------------------------------------------------------------
|
-------------------------------------------------------------------
|
||||||
Thu Feb 3 11:28:46 UTC 2022 - Fabian Vogt <fabian@ritter-vogt.de>
|
Thu Feb 3 11:28:46 UTC 2022 - Fabian Vogt <fabian@ritter-vogt.de>
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user