Accepting request 951566 from KDE:Qt5
- Add patch to avoid unintentionally using binaries from CWD (boo#1195386, CVE-2022-23853): * 0001-QProcess-Unix-ensure-we-don-t-accidentally-execute-s.patch OBS-URL: https://build.opensuse.org/request/show/951566 OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/libqt5-qtbase?expand=0&rev=126
This commit is contained in:
commit
e0c7b03e9d
182
0001-QProcess-Unix-ensure-we-don-t-accidentally-execute-s.patch
Normal file
182
0001-QProcess-Unix-ensure-we-don-t-accidentally-execute-s.patch
Normal file
@ -0,0 +1,182 @@
|
||||
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
|
||||
|
@ -1,3 +1,10 @@
|
||||
-------------------------------------------------------------------
|
||||
Thu Feb 3 11:28:46 UTC 2022 - Fabian Vogt <fabian@ritter-vogt.de>
|
||||
|
||||
- Add patch to avoid unintentionally using binaries from CWD
|
||||
(boo#1195386, CVE-2022-23853):
|
||||
* 0001-QProcess-Unix-ensure-we-don-t-accidentally-execute-s.patch
|
||||
|
||||
-------------------------------------------------------------------
|
||||
Fri Jan 21 08:04:30 UTC 2022 - Fabian Vogt <fabian@ritter-vogt.de>
|
||||
|
||||
|
@ -65,6 +65,8 @@ Patch24: fix-fixqt4headers.patch
|
||||
# patches 2000-3000 and above from upstream qt6/dev branch #
|
||||
# Not accepted yet, https://codereview.qt-project.org/c/qt/qtbase/+/255384
|
||||
Patch2001: 0002-Synthesize-Enter-LeaveEvent-for-accepted-QTabletEven.patch
|
||||
# Not accepted yet, https://codereview.qt-project.org/c/qt/qtbase/+/393113
|
||||
Patch2002: 0001-QProcess-Unix-ensure-we-don-t-accidentally-execute-s.patch
|
||||
BuildRequires: cups-devel
|
||||
BuildRequires: double-conversion-devel
|
||||
BuildRequires: gcc-c++
|
||||
|
Loading…
Reference in New Issue
Block a user