1
0
forked from pool/plasma5-disks

Respun tar, includes 0001-pass-device-names-to-the-helper.patch

OBS-URL: https://build.opensuse.org/package/show/KDE:Frameworks5/plasma5-disks?expand=0&rev=3
This commit is contained in:
Fabian Vogt 2020-10-11 11:53:56 +00:00 committed by Git OBS Bridge
parent 9d75348983
commit 754d0b8657
5 changed files with 10 additions and 149 deletions

View File

@ -1,135 +0,0 @@
From b7373d6c3060817a0ecf7f4d9a06c8a9aa16548a Mon Sep 17 00:00:00 2001
From: Harald Sitter <sitter@kde.org>
Date: Thu, 8 Oct 2020 11:45:10 +0200
Subject: [PATCH] pass device names to the helper
paths are somewhat trivial to exploit. instead resolve them to the
actual block device names under /dev/ and pass that into the privileged
helper. the helper then only needs to verify that $name is in fact a
block device under /dev/.
since unprivileged processes cannot create files in /dev/ directly, let
alone block devices, this should give us a very reliable way of
preventing abuse.
---
src/helper.cpp | 68 ++++++++++++++++++++++++++++++++++++++----------
src/smartctl.cpp | 11 +++++++-
2 files changed, 64 insertions(+), 15 deletions(-)
diff --git a/src/helper.cpp b/src/helper.cpp
index 5418b25..5a9ce47 100644
--- a/src/helper.cpp
+++ b/src/helper.cpp
@@ -6,28 +6,68 @@
#include <QDebug>
#include <QProcess>
#include <QFileInfo>
+#include <QScopeGuard>
-QString pathFrom(const QVariantMap &args)
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/types.h>
+#include <dirent.h>
+
+// Append name to /dev/ and ensure it is a trustable block device.
+static QString nameToPath(const QString &name)
{
- const auto devicePath = args.value(QStringLiteral("devicePath")).toString();
- QFileInfo info(devicePath);
- return info.absoluteFilePath();
+ if (name.isEmpty()) {
+ return {};
+ }
+
+ // This also excludes relative path shenanigans as they'd all need to contain a separator.
+ if (name.contains(QLatin1Char('/'))) {
+ qWarning() << "Device names must not contain slashes";
+ return {};
+ }
+
+ const QString path = QStringLiteral("/dev/%1").arg(name);
+
+ int blockFD = open(QFile::encodeName(path), O_PATH | O_NOFOLLOW);
+ auto blockFDClose = qScopeGuard([blockFD] { close(blockFD); });
+ if (blockFD == -1) {
+ const int err = errno;
+ qWarning() << "Failed to open block device" << name << strerror(err);
+ return {};
+ }
+
+ struct stat sb;
+ if (fstat(blockFD, &sb) == -1) {
+ const int err = errno;
+ qWarning() << "Failed to stat block device" << name << strerror(err);
+ return {};
+ }
+
+ if (!S_ISBLK(sb.st_mode)) {
+ qWarning() << "Device is not actually a block device" << name;
+ return {};
+ }
+
+ if (sb.st_uid != 0) {
+ qWarning() << "Device is not owned by root" << name;
+ return {};
+ }
+
+ return path;
}
ActionReply SMARTHelper::smartctl(const QVariantMap &args)
{
- // I may be better overall to also spin up solid on the root end and only allow
- // UDIs as input. We can then assert expected input. Not sure it makes much
- // of a difference though.
- const QString devicePath = pathFrom(args);
- if (devicePath.isEmpty() || !QFile::exists(devicePath)) {
- qDebug() << "bad path";
+ // For security reasons we only accept fully resolved device names which
+ // we use to construct the final /dev/$name path.
+ const QString name = args.value(QStringLiteral("deviceName")).toString();
+ const QString devicePath = nameToPath(name);
+ if (devicePath.isEmpty()) {
return ActionReply::HelperErrorReply();
}
- if (!devicePath.startsWith(QStringLiteral("/dev/"))) {
- qDebug() << "unauthorized path";
- return ActionReply::HelperErrorReply(KAuth::ActionReply::AuthorizationDeniedError);
- }
// PATH is super minimal when invoked through dbus
setenv("PATH", "/usr/sbin:/sbin", 1);
diff --git a/src/smartctl.cpp b/src/smartctl.cpp
index b214fff..a8f66ce 100644
--- a/src/smartctl.cpp
+++ b/src/smartctl.cpp
@@ -4,6 +4,7 @@
#include "smartctl.h"
#include <QDebug>
+#include <QFileInfo>
#include <KAuthAction>
#include <KAuthExecuteJob>
#include <KLocalizedString>
@@ -32,7 +33,15 @@ void SMARTCtl::run(const QString &devicePath)
devicePath) }
});
action.setHelperId(QStringLiteral("org.kde.kded.smart"));
- action.addArgument(QStringLiteral("devicePath"), devicePath);
+
+ // The helper only consumes names, ensure we fully resolve the name of the
+ // device to /dev/$name.
+ const QString canonicalDevicePath = QFileInfo(devicePath).canonicalFilePath();
+ Q_ASSERT(!canonicalDevicePath.isEmpty());
+ const QFileInfo canonicalDeviceInfo(canonicalDevicePath);
+ Q_ASSERT(canonicalDeviceInfo.absolutePath() == QLatin1String("/dev"));
+
+ action.addArgument(QStringLiteral("deviceName"), canonicalDeviceInfo.fileName());
qCDebug(KDED) << action.isValid()
<< action.hasHelper()
<< action.helperId()
--
2.25.1

View File

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:a76b5d9ee0fadc29d5d4127e4cc984fecb17609e17fbecb5cb376969791bc17b
size 60060
oid sha256:187abd14a94864cfadfd19375ac93813194cf5e2d89e6863d52599bd7bf923e8
size 60536

View File

@ -1,11 +1,11 @@
-----BEGIN PGP SIGNATURE-----
iQEzBAABCgAdFiEELR1bBYg1d4fenuIl7JTRj38FmX4FAl9/PKkACgkQ7JTRj38F
mX7BKgf+In9/thSWo5LqlsodUlA7GmDhHMa1+/BwLQUW4TAZGAWDbZwZa+kumwP4
WNJWBO+GmeI/UCG4P8KxUlDQGVqkBqk1f7Y51eeR5V0ymb8hRoer2DLfukZDq4n6
Sjcc2Klex5Azu6BKBBPZjlPCWtrMWjXe/Dj+nzOxpK4t/oTP1IR1TZdXQBlzWrvF
8Sg/2nXkdahrbE44KNOwt4YQfxTZlTs1zo+e8UslBqIl1KGajvtIM0CoeSJ7ffs7
Xx+ai4b8/Ix2p8zClgVP8J+4MGJv7sqKI6xnMg7VMrrJ54bmdj2bjHOCKS0B83Q6
n6wmzlHenbNFxI1VVNN+UsWmzyL6yQ==
=r5sZ
iQEzBAABCgAdFiEELR1bBYg1d4fenuIl7JTRj38FmX4FAl+AZ9AACgkQ7JTRj38F
mX4LNgf+ImC+yTO0eEt4WpgT49xjMLPvazeb6bTBBayvOMWhT8j8Zkue/5ZjPZEO
9JLAVdtr8scNTobtIM/H9WZ8nTxVt/YZGlAdfq71j70vp7VOGlaRzANTr1wDKscH
hYrkQkX/kEiEgpDRC2UljJsRmXKBN5rmBemhQnXp192xYl4CbZve2vn98L5DAnBV
DsuwD3aCv24gWCvA6cCjtrhqvRFYX3RemwtOkpcfrMaNaweFM6aBreUD7mrvat5o
mxoPdFGn498yhdpfe9YXHhf1g4qEPoQhTsidGrHA/d/JfVrbRmHEVrAQeQNGhPUX
yS2V8uckeWRyU9cMuZImPy1Y3ukxkA==
=+NTT
-----END PGP SIGNATURE-----

View File

@ -7,8 +7,6 @@ Thu Oct 8 16:58:05 UTC 2020 - Fabian Vogt <fabian@ritter-vogt.de>
* https://kde.org/announcements/plasma-5.20.0
- Changes since 5.19.90:
* add request queuing to kauth smartctl
- Add patch to harden the kauth helper (boo#1176742):
* 0001-pass-device-names-to-the-helper.patch
-------------------------------------------------------------------
Sat Sep 19 21:00:16 UTC 2020 - Fabian Vogt <fabian@ritter-vogt.de>

View File

@ -31,8 +31,6 @@ Source: plasma-disks-%{version}.tar.xz
Source1: plasma-disks-%{version}.tar.xz.sig
Source2: plasma.keyring
%endif
# PATCH-FIX-UPSTREAM:
Patch1: 0001-pass-device-names-to-the-helper.patch
BuildRequires: cmake >= 3.16
BuildRequires: extra-cmake-modules >= %{kf5_version}
BuildRequires: cmake(KF5CoreAddons) >= %{kf5_version}