Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP2:Update
libqt5-qtbase
0005-QStandardPaths-Unix-improve-the-XDG_RUNTIM...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 0005-QStandardPaths-Unix-improve-the-XDG_RUNTIME_DIR-crea.patch of Package libqt5-qtbase
From b6e7d011df945a695718e35546c3e4bc5ebe6592 Mon Sep 17 00:00:00 2001 From: Thiago Macieira <thiago.macieira@intel.com> Date: Thu, 23 Jul 2020 18:17:19 -0700 Subject: [PATCH 5/5] QStandardPaths/Unix: improve the XDG_RUNTIME_DIR creation/detection First, use QT_MKDIR instead of QFileSystemEngine::createDirectory(), as the latter can't create a directory with the right permissions. That would allow an attacker to briefly obtain access to the runtime dir between the mkdir() and chmod() system calls. Second, make sure that if the target already exists that it is a directory and not a symlink (even to a directory). If it is a symlink that belongs to another user, it can be changed to point to another place, which we won't like. And as a bonus, we're printing more information to the user in case something went wrong. Sample outputs: QStandardPaths: runtime directory '/root' is not owned by UID 1000, but a directory permissions 0700 owned by UID 0 GID 0 QStandardPaths: runtime directory '/dev/null' is not a directory, but a character device, socket or FIFO permissions 0666 owned by UID 0 GID 0 QStandardPaths: runtime directory '/etc/passwd' is not a directory, but a regular file permissions 0644 owned by UID 0 GID 0 QStandardPaths: XDG_RUNTIME_DIR not set, defaulting to '/tmp/runtime-tjmaciei' QStandardPaths: runtime directory '/tmp/runtime-tjmaciei' is not a directory, but a symbolic link to a directory permissions 0755 owned by UID 1000 GID 100 Change-Id: Iea47e0f8fc8b40378df7fffd16248b663794c613 Reviewed-by: David Faure <david.faure@kdab.com> (cherry picked from commit ad5a65b6a2bfca1658634e380559d14ea1e904a4) Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io> (cherry picked from commit 99280f4ee9ad03412578147d0da0c6630ecc324c) --- src/corelib/io/qstandardpaths_unix.cpp | 171 ++++++++---- .../io/qstandardpaths/tst_qstandardpaths.cpp | 248 +++++++++++++++--- 2 files changed, 334 insertions(+), 85 deletions(-) diff --git a/src/corelib/io/qstandardpaths_unix.cpp b/src/corelib/io/qstandardpaths_unix.cpp index da24a268cd..18c2c4cca6 100644 --- a/src/corelib/io/qstandardpaths_unix.cpp +++ b/src/corelib/io/qstandardpaths_unix.cpp @@ -1,7 +1,8 @@ /**************************************************************************** ** -** Copyright (C) 2015 The Qt Company Ltd. -** Contact: http://www.qt.io/licensing/ +** Copyright (C) 2020 The Qt Company Ltd. +** Copyright (C) 2020 Intel Corporation. +** Contact: https://www.qt.io/licensing/ ** ** This file is part of the QtCore module of the Qt Toolkit. ** @@ -62,6 +63,117 @@ static void appendOrganizationAndApp(QString &path) #endif } +// From Qt 5.9.7's qglobal.h +#ifndef qUtf16Printable +# define qUtf16Printable(string) \ + static_cast<const wchar_t*>(static_cast<const void*>(QString(string).utf16())) +#endif + +static bool checkXdgRuntimeDir(const QString &xdgRuntimeDir) +{ + auto describeMetaData = [](const QFileSystemMetaData &metaData) -> QByteArray { + if (!metaData.exists()) + return "a broken symlink"; + + QByteArray description; + if (metaData.isLink()) + description = "a symbolic link to "; + + if (metaData.isFile()) + description += "a regular file"; + else if (metaData.isDirectory()) + description += "a directory"; + else if (metaData.isSequential()) + description += "a character device, socket or FIFO"; + else + description += "a block device"; + + // convert QFileSystemMetaData permissions back to Unix + mode_t perms = 0; + if (metaData.permissions() & QFile::ReadOwner) + perms |= S_IRUSR; + if (metaData.permissions() & QFile::WriteOwner) + perms |= S_IWUSR; + if (metaData.permissions() & QFile::ExeOwner) + perms |= S_IXUSR; + if (metaData.permissions() & QFile::ReadGroup) + perms |= S_IRGRP; + if (metaData.permissions() & QFile::WriteGroup) + perms |= S_IWGRP; + if (metaData.permissions() & QFile::ExeGroup) + perms |= S_IXGRP; + if (metaData.permissions() & QFile::ReadOther) + perms |= S_IROTH; + if (metaData.permissions() & QFile::WriteOther) + perms |= S_IWOTH; + if (metaData.permissions() & QFile::ExeOther) + perms |= S_IXOTH; + description += " permissions 0" + QByteArray::number(perms, 8); + + return description + + " owned by UID " + QByteArray::number(metaData.userId()) + + " GID " + QByteArray::number(metaData.groupId()); + }; + + // http://standards.freedesktop.org/basedir-spec/latest/ + const uint myUid = uint(geteuid()); + const QFile::Permissions wantedPerms = QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner; + const QFileSystemMetaData::MetaDataFlags statFlags = QFileSystemMetaData::PosixStatFlags + | QFileSystemMetaData::LinkType; + QFileSystemMetaData metaData; + QFileSystemEntry entry(xdgRuntimeDir); + + // Check that the xdgRuntimeDir is a directory by attempting to create it. + // A stat() before mkdir() that concluded it doesn't exist is a meaningless + // result: we'd race against someone else attempting to create it. + // ### QFileSystemEngine::createDirectory cannot take the extra mode argument. + if (QT_MKDIR(entry.nativeFilePath(), 0700) == 0) + return true; + if (errno != EEXIST) { + qErrnoWarning("QStandardPaths: error creating runtime directory '%ls'", + qUtf16Printable(xdgRuntimeDir)); + return false; + } + + // We use LinkType to force an lstat(), but fillMetaData() still returns error + // on broken symlinks. + if (!QFileSystemEngine::fillMetaData(entry, metaData, statFlags) && !metaData.isLink()) { + qErrnoWarning("QStandardPaths: error obtaining permissions of runtime directory '%ls'", + qUtf16Printable(xdgRuntimeDir)); + return false; + } + + // Checks: + // - is a directory + // - is not a symlink (even is pointing to a directory) + if (metaData.isLink() || !metaData.isDirectory()) { + qWarning("QStandardPaths: runtime directory '%ls' is not a directory, but %s", + qUtf16Printable(xdgRuntimeDir), describeMetaData(metaData).constData()); + return false; + } + + // - "The directory MUST be owned by the user" + if (metaData.userId() != myUid) { + qWarning("QStandardPaths: runtime directory '%ls' is not owned by UID %d, but %s", + qUtf16Printable(xdgRuntimeDir), myUid, describeMetaData(metaData).constData()); + return false; + } + + // "and he MUST be the only one having read and write access to it. Its Unix access mode MUST be 0700." + if (metaData.permissions() != wantedPerms) { + // attempt to correct: + QSystemError error; + if (!QFileSystemEngine::setPermissions(entry, wantedPerms, error)) { + qErrnoWarning("QStandardPaths: could not set correct permissions on runtime directory " + "'%ls', which is %s", qUtf16Printable(xdgRuntimeDir), + describeMetaData(metaData).constData()); + return false; + } + } + + return true; +} + QString QStandardPaths::writableLocation(StandardLocation type) { switch (type) { @@ -111,57 +223,22 @@ QString QStandardPaths::writableLocation(StandardLocation type) } case RuntimeLocation: { - // http://standards.freedesktop.org/basedir-spec/latest/ - const uint myUid = uint(geteuid()); - // since the current user is the owner, set both xxxUser and xxxOwner - const QFile::Permissions wantedPerms = QFile::ReadUser | QFile::WriteUser | QFile::ExeUser - | QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner; - QFileInfo fileInfo; QString xdgRuntimeDir = QFile::decodeName(qgetenv("XDG_RUNTIME_DIR")); - if (xdgRuntimeDir.isEmpty()) { + bool fromEnv = !xdgRuntimeDir.isEmpty(); + if (xdgRuntimeDir.isEmpty() || !checkXdgRuntimeDir(xdgRuntimeDir)) { + // environment variable not set or is set to something unsuitable + const uint myUid = uint(geteuid()); const QString userName = QFileSystemEngine::resolveUserName(myUid); xdgRuntimeDir = QDir::tempPath() + QLatin1String("/runtime-") + userName; - fileInfo.setFile(xdgRuntimeDir); + + if (!fromEnv) { #ifndef Q_OS_WASM - qWarning("QStandardPaths: XDG_RUNTIME_DIR not set, defaulting to '%s'", qPrintable(xdgRuntimeDir)); + qWarning("QStandardPaths: XDG_RUNTIME_DIR not set, defaulting to '%ls'", qUtf16Printable(xdgRuntimeDir)); #endif - } else { - fileInfo.setFile(xdgRuntimeDir); - } - if (fileInfo.exists()) { - if (!fileInfo.isDir()) { - qWarning("QStandardPaths: XDG_RUNTIME_DIR points to '%s' which is not a directory", - qPrintable(xdgRuntimeDir)); - return QString(); - } - } else { - QFileSystemEntry entry(xdgRuntimeDir); - if (!QFileSystemEngine::createDirectory(entry, false)) { - if (errno != EEXIST) { - qWarning("QStandardPaths: error creating runtime directory %s: %s", - qPrintable(xdgRuntimeDir), qPrintable(qt_error_string(errno))); - return QString(); - } - } else { - QSystemError error; - if (!QFileSystemEngine::setPermissions(entry, wantedPerms, error)) { - qWarning("QStandardPaths: could not set correct permissions on runtime directory %s: %s", - qPrintable(xdgRuntimeDir), qPrintable(error.toString())); - return QString(); - } } - } - // "The directory MUST be owned by the user" - if (fileInfo.ownerId() != myUid) { - qWarning("QStandardPaths: wrong ownership on runtime directory %s, %d instead of %d", - qPrintable(xdgRuntimeDir), fileInfo.ownerId(), myUid); - return QString(); - } - // "and he MUST be the only one having read and write access to it. Its Unix access mode MUST be 0700." - if (fileInfo.permissions() != wantedPerms) { - qWarning("QStandardPaths: wrong permissions on runtime directory %s, %x instead of %x", - qPrintable(xdgRuntimeDir), uint(fileInfo.permissions()), uint(wantedPerms)); - return QString(); + + if (!checkXdgRuntimeDir(xdgRuntimeDir)) + xdgRuntimeDir.clear(); } return xdgRuntimeDir; diff --git a/tests/auto/corelib/io/qstandardpaths/tst_qstandardpaths.cpp b/tests/auto/corelib/io/qstandardpaths/tst_qstandardpaths.cpp index 1e7dc94f06..6097707f40 100644 --- a/tests/auto/corelib/io/qstandardpaths/tst_qstandardpaths.cpp +++ b/tests/auto/corelib/io/qstandardpaths/tst_qstandardpaths.cpp @@ -1,7 +1,8 @@ /**************************************************************************** ** -** Copyright (C) 2015 The Qt Company Ltd. -** Contact: http://www.qt.io/licensing/ +** Copyright (C) 2020 The Qt Company Ltd. +** Copyright (C) 2020 Intel Corporation. +** Contact: https://www.qt.io/licensing/ ** ** This file is part of the test suite of the Qt Toolkit. ** @@ -31,17 +32,17 @@ ** ****************************************************************************/ -#include <QtTest/QtTest> #include <qstandardpaths.h> +#include <QtTest/QtTest> #include <qdebug.h> -#include <qstandardpaths.h> #include <qfileinfo.h> -#include <qsysinfo.h> +#include <qplatformdefs.h> #include <qregexp.h> #ifdef Q_OS_UNIX #include <unistd.h> #include <sys/types.h> +#include <pwd.h> #endif #if defined(Q_OS_UNIX) && !defined(Q_OS_MAC) && !defined(Q_OS_BLACKBERRY) && !defined(Q_OS_ANDROID) @@ -68,6 +69,7 @@ private slots: void testFindExecutable(); void testFindExecutableLinkToDirectory(); void testRuntimeDirectory(); + void testCustomRuntimeDirectory_data(); void testCustomRuntimeDirectory(); void testAllWritableLocations_data(); void testAllWritableLocations(); @@ -441,6 +443,9 @@ void tst_qstandardpaths::testFindExecutableLinkToDirectory() #endif } +using RuntimeDirSetup = QString (*)(QDir &); +Q_DECLARE_METATYPE(RuntimeDirSetup); + void tst_qstandardpaths::testRuntimeDirectory() { #ifdef Q_XDG_PLATFORM @@ -449,6 +454,174 @@ void tst_qstandardpaths::testRuntimeDirectory() #endif } +#ifdef Q_XDG_PLATFORM +static QString fallbackXdgRuntimeDir() +{ + static QString username = [] { + struct passwd *pw = getpwuid(geteuid()); + return QString::fromLocal8Bit(pw->pw_name); + }(); + + // QDir::temp() might change from call to call + return QDir::temp().filePath("runtime-" + username); +} +#endif + +static QString updateRuntimeDir(const QString &path) +{ + qputenv("XDG_RUNTIME_DIR", QFile::encodeName(path)); + return path; +} + +static void clearRuntimeDir() +{ + qunsetenv("XDG_RUNTIME_DIR"); +#ifdef Q_XDG_PLATFORM +#ifndef Q_OS_WASM + QTest::ignoreMessage(QtWarningMsg, + qPrintable("QStandardPaths: XDG_RUNTIME_DIR not set, defaulting to '" + + fallbackXdgRuntimeDir() + '\'')); +#endif +#endif +} + +void tst_qstandardpaths::testCustomRuntimeDirectory_data() +{ +#if defined(Q_XDG_PLATFORM) + QTest::addColumn<RuntimeDirSetup>("setup"); + auto addRow = [](const char *name, RuntimeDirSetup f) { + QTest::newRow(name) << f; + }; + + +# if defined(Q_OS_UNIX) + if (::getuid() == 0) + QSKIP("Running this test as root doesn't make sense"); +# endif + + addRow("environment:non-existing", [](QDir &d) { + return updateRuntimeDir(d.filePath("runtime")); + }); + + addRow("environment:existing", [](QDir &d) { + QString p = d.filePath("runtime"); + d.mkdir("runtime"); + QFile::setPermissions(p, QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner); + return updateRuntimeDir(p); + }); + + addRow("environment-to-existing-wrong-perm", [](QDir &d) { + QString p = d.filePath("runtime"); + d.mkdir("runtime"); + QFile::setPermissions(p, QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner | + QFile::ExeGroup | QFile::ExeOther); + return updateRuntimeDir(p); + }); + + addRow("environment:wrong-owner", [](QDir &) { + QT_STATBUF st; + QT_STAT("/", &st); + + updateRuntimeDir("/"); + QTest::ignoreMessage(QtWarningMsg, + QString("QStandardPaths: runtime directory '/' is not owned by UID " + "%1, but a directory permissions %2 owned by UID %3 GID %4") + .arg(getuid()) + .arg(st.st_mode & 07777, 4, 8, QChar('0')) + .arg(st.st_uid) + .arg(st.st_gid).toLatin1()); + return fallbackXdgRuntimeDir(); + }); + + addRow("environment:file", [](QDir &d) { + QString p = d.filePath("file"); + QFile f(p); + f.open(QIODevice::WriteOnly); + f.setPermissions(QFile::ReadOwner | QFile::WriteOwner); + + updateRuntimeDir(p); + QTest::ignoreMessage(QtWarningMsg, + QString("QStandardPaths: runtime directory '%1' is not a directory, " + "but a regular file permissions 0600 owned by UID %2 GID %3") + .arg(p).arg(getuid()).arg(getgid()).toLatin1()); + return fallbackXdgRuntimeDir(); + }); + + addRow("environment:broken-symlink", [](QDir &d) { + QString p = d.filePath("link"); + QFile::link(d.filePath("this-goes-nowhere"), p); + updateRuntimeDir(p); + QTest::ignoreMessage(QtWarningMsg, + QString("QStandardPaths: runtime directory '%1' is not a directory, " + "but a broken symlink") + .arg(p).toLatin1()); + return fallbackXdgRuntimeDir(); + }); + + addRow("environment:symlink-to-dir", [](QDir &d) { + QString p = d.filePath("link"); + d.mkdir("dir"); + QFile::link(d.filePath("dir"), p); + QFile::setPermissions(p, QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner); + updateRuntimeDir(p); + QTest::ignoreMessage(QtWarningMsg, + QString("QStandardPaths: runtime directory '%1' is not a directory, " + "but a symbolic link to a directory permissions 0700 owned by UID %2 GID %3") + .arg(p).arg(getuid()).arg(getgid()).toLatin1()); + return fallbackXdgRuntimeDir(); + }); + + addRow("no-environment:non-existing", [](QDir &) { + clearRuntimeDir(); + return fallbackXdgRuntimeDir(); + }); + + addRow("no-environment:existing", [](QDir &d) { + clearRuntimeDir(); + QString p = fallbackXdgRuntimeDir(); + d.mkdir(p); // probably has wrong permissions + return p; + }); + + addRow("no-environment:fallback-is-file", [](QDir &) { + QString p = fallbackXdgRuntimeDir(); + QFile f(p); + f.open(QIODevice::WriteOnly); + f.setPermissions(QFile::ReadOwner | QFile::WriteOwner); + + clearRuntimeDir(); + QTest::ignoreMessage(QtWarningMsg, + QString("QStandardPaths: runtime directory '%1' is not a directory, " + "but a regular file permissions 0600 owned by UID %2 GID %3") + .arg(p).arg(getuid()).arg(getgid()).toLatin1()); + return QString(); + }); + + addRow("environment-and-fallback-are-files", [](QDir &d) { + QString p = d.filePath("file1"); + QFile f(p); + f.open(QIODevice::WriteOnly); + f.setPermissions(QFile::ReadOwner | QFile::WriteOwner | QFile::ReadGroup); + updateRuntimeDir(p); + QTest::ignoreMessage(QtWarningMsg, + QString("QStandardPaths: runtime directory '%1' is not a directory, " + "but a regular file permissions 0640 owned by UID %2 GID %3") + .arg(p).arg(getuid()).arg(getgid()).toLatin1()); + + f.close(); + f.setFileName(fallbackXdgRuntimeDir()); + f.open(QIODevice::WriteOnly); + f.setPermissions(QFile::ReadOwner | QFile::WriteOwner | QFile::ReadGroup); + QTest::ignoreMessage(QtWarningMsg, + QString("QStandardPaths: runtime directory '%1' is not a directory, " + "but a regular file permissions 0640 owned by UID %2 GID %3") + .arg(f.fileName()).arg(getuid()).arg(getgid()).toLatin1()); + + return QString(); + }); +#endif +} + void tst_qstandardpaths::testCustomRuntimeDirectory() { #if defined(Q_OS_UNIX) @@ -459,12 +632,17 @@ void tst_qstandardpaths::testCustomRuntimeDirectory() #ifdef Q_XDG_PLATFORM struct EnvVarRestorer { - EnvVarRestorer() : origRuntimeDir(qgetenv("XDG_RUNTIME_DIR")) {} - ~EnvVarRestorer() { qputenv("XDG_RUNTIME_DIR", origRuntimeDir.constData()); } - const QByteArray origRuntimeDir; + ~EnvVarRestorer() + { + qputenv("XDG_RUNTIME_DIR", origRuntimeDir); + qputenv("TMPDIR", origTempDir); + } + const QByteArray origRuntimeDir = qgetenv("XDG_RUNTIME_DIR"); + const QByteArray origTempDir = qgetenv("TMPDIR"); }; EnvVarRestorer restorer; +<<<<<<< HEAD // When $XDG_RUNTIME_DIR points to a directory with wrong ownership, QStandardPaths should warn qputenv("XDG_RUNTIME_DIR", QFile::encodeName("/tmp")); // It's very unlikely that /tmp is 0600 or that we can chmod it @@ -476,36 +654,30 @@ void tst_qstandardpaths::testCustomRuntimeDirectory() qPrintable(QString::fromLatin1("QStandardPaths: wrong ownership on runtime directory /tmp, 0 instead of %1").arg(uid))); const QString runtimeDir = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation); QVERIFY2(runtimeDir.isEmpty(), qPrintable(runtimeDir)); - - // When $XDG_RUNTIME_DIR points to a directory with wrong permissions, QStandardPaths should warn - const QByteArray wrongPermissionFileName = "wrong_permissions"; - QDir::current().mkdir(wrongPermissionFileName); - QFile wrongPermissionFile(wrongPermissionFileName); - const QFile::Permissions wantedPerms = QFile::ReadUser | QFile::WriteUser | QFile::ExeUser; - QVERIFY(wrongPermissionFile.setPermissions(wantedPerms | QFile::ExeGroup)); - - qputenv("XDG_RUNTIME_DIR", wrongPermissionFileName); - QTest::ignoreMessage(QtWarningMsg, - qPrintable(QString::fromLatin1("QStandardPaths: wrong permissions on runtime directory " + wrongPermissionFileName + ", 7710 instead of 7700"))); - const QString wrongPermissionRuntimeDir = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation); - QVERIFY(wrongPermissionRuntimeDir.isEmpty()); - QDir::current().rmdir(wrongPermissionFileName); - - // When $XDG_RUNTIME_DIR points to a non-existing directory, QStandardPaths should create it first - const QByteArray nonExistingDir = "does_not_exist"; - qputenv("XDG_RUNTIME_DIR", nonExistingDir); - const QString nonExistingRuntimeDir = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation); - QVERIFY2(!nonExistingRuntimeDir.compare(nonExistingDir), qPrintable(nonExistingRuntimeDir)); - QVERIFY(QDir::current().exists(nonExistingRuntimeDir)); - QDir::current().rmdir(nonExistingRuntimeDir); - - // When $XDG_RUNTIME_DIR points to a file, QStandardPaths should warn - const QString file = QFINDTESTDATA("tst_qstandardpaths.cpp"); - QVERIFY(!file.isEmpty()); - qputenv("XDG_RUNTIME_DIR", QFile::encodeName(file)); - QTest::ignoreMessage(QtWarningMsg, qPrintable(QString::fromLatin1("QStandardPaths: XDG_RUNTIME_DIR points to '%1' which is not a directory").arg(file))); - const QString noRuntimeDir = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation); - QVERIFY2(noRuntimeDir.isEmpty(), qPrintable(file)); +======= + // set up the environment to point to a place we control + QTemporaryDir tempDir; + QVERIFY2(tempDir.isValid(), qPrintable(tempDir.errorString())); + + QDir d(tempDir.path()); + qputenv("TMPDIR", QFile::encodeName(tempDir.path())); +>>>>>>> 6027271485... QStandardPaths/Unix: improve the XDG_RUNTIME_DIR creation/detection + + QFETCH(RuntimeDirSetup, setup); + QString expected = setup(d); + + QString runtimeDir = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation); + QCOMPARE(runtimeDir, expected); + + if (!runtimeDir.isEmpty()) { + QFileInfo runtimeInfo(runtimeDir); + QVERIFY(runtimeInfo.isDir()); + QVERIFY(!runtimeInfo.isSymLink()); + auto expectedPerms = QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner + | QFile::ReadUser | QFile::WriteUser | QFile::ExeUser; + QCOMPARE(QString::number(runtimeInfo.permissions(), 16), + QString::number(expectedPerms, 16)); + } #endif } -- 2.25.1
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor