pool/libqt5-qtbase
SHA256
2
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

- Add patch to avoid unintentionally using binaries from CWD

Browse Source 
(boo#1195386, CVE-2022-23853):
  * 0001-QProcess-Unix-ensure-we-don-t-accidentally-execute-s.patch

OBS-URL: https://build.opensuse.org/package/show/KDE:Qt5/libqt5-qtbase?expand=0&rev=33
This commit is contained in:
Christophe Giboudeaux 2022-02-03 21:11:04 +00:00 committed by Git OBS Bridge
parent 7babdec880
commit dbcea5cadf
3 changed files with 191 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

182
0001-QProcess-Unix-ensure-we-don-t-accidentally-execute-s.patch Normal file
View 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

7
libqt5-qtbase.changes
View File

@ -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> Fri Jan 21 08:04:30 UTC 2022 - Fabian Vogt <fabian@ritter-vogt.de>

2
libqt5-qtbase.spec
View File

@ -65,6 +65,8 @@ Patch24: fix-fixqt4headers.patch
# patches 2000-3000 and above from upstream qt6/dev branch # # patches 2000-3000 and above from upstream qt6/dev branch #
# Not accepted yet, https://codereview.qt-project.org/c/qt/qtbase/+/255384 # Not accepted yet, https://codereview.qt-project.org/c/qt/qtbase/+/255384
Patch2001: 0002-Synthesize-Enter-LeaveEvent-for-accepted-QTabletEven.patch 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: cups-devel
BuildRequires: double-conversion-devel BuildRequires: double-conversion-devel
BuildRequires: gcc-c++ BuildRequires: gcc-c++