Make sure that path passed to WriteData is block device.

Casting and passing the processChannel argument unfiltered to QProcess
could pose a security issue if future additions are made to
QProcess that introduce new behaviour.

src/util/externalcommand.cpp

@@ -160,7 +160,7 @@ bool ExternalCommand::copyBlocks(const CopySource& source, CopyTarget& target)
     connect(interface, &OrgKdeKpmcoreExternalcommandInterface::progress, this, &ExternalCommand::progress);
     connect(interface, &OrgKdeKpmcoreExternalcommandInterface::report, this, &ExternalCommand::reportSignal);
 
-    QDBusPendingCall pcall = interface->CopyBlocks(source.path(), source.firstByte(), source.length(),
+    QDBusPendingCall pcall = interface->CopyFileData(source.path(), source.firstByte(), source.length(),
                                                    target.path(), target.firstByte(), blockSize);
 
     QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);

src/util/externalcommandhelper.cpp

@@ -136,7 +136,7 @@ bool ExternalCommandHelper::CreateFile(const QString &filePath, const QByteArray
         return false;
     }
     // Do not allow using this helper for writing to arbitrary location
-    if ( !filePath.contains(QStringLiteral("/etc/fstab")) )
+    if ( filePath != QStringLiteral("/etc/fstab") )
         return false;
 
     QFile device(filePath);
@@ -156,7 +156,7 @@ bool ExternalCommandHelper::CreateFile(const QString &filePath, const QByteArray
 }
 
 // If targetDevice is empty then return QByteArray with data that was read from disk.
-QVariantMap ExternalCommandHelper::CopyBlocks(const QString& sourceDevice, const qint64 sourceOffset, const qint64 sourceLength, const QString& targetDevice, const qint64 targetOffset, const qint64 blockSize)
+QVariantMap ExternalCommandHelper::CopyFileData(const QString& sourceDevice, const qint64 sourceOffset, const qint64 sourceLength, const QString& targetDevice, const qint64 targetOffset, const qint64 blockSize)
 {
     if (!isCallerAuthorized()) {
         return {};
@@ -172,6 +172,18 @@ QVariantMap ExternalCommandHelper::CopyBlocks(const QString& sourceDevice, const
         return {};
     }
 
+    // Check for relative paths
+    std::filesystem::path sourcePath(sourceDevice.toStdU16String());
+    std::filesystem::path targetPath(targetDevice.toStdU16String());
+    if(sourcePath.is_relative() || targetPath.is_relative()) {
+        return {};
+    }
+
+    // Only allow writing to existing files.
+    if(!std::filesystem::exists(targetPath)) {
+        return {};
+    }
+
     QVariantMap reply;
     reply[QStringLiteral("success")] = true;
 
@@ -279,7 +291,7 @@ QByteArray ExternalCommandHelper::ReadData(const QString& device, const qint64 o
     if (length > MiB) {
         return {};
     }
-    if (!std::filesystem::is_block_file(device.toStdString())) {
+    if (!std::filesystem::is_block_file(device.toStdU16String())) {
         qWarning() << "Not a block device";
         return {};
     }
@@ -301,7 +313,15 @@ bool ExternalCommandHelper::WriteData(const QByteArray& buffer, const QString& t
     if ( targetDevice.left(5) != QStringLiteral("/dev/") )
         return false;
 
-    return writeData(targetDevice, buffer, targetOffset);
+    auto targetPath = std::filesystem::path(targetDevice.toStdU16String());
+    if (!std::filesystem::is_block_file(targetDevice.toStdU16String())) {
+        qWarning() << "Not a block device";
+        return {};
+    }
+
+    auto canonicalTargetPath = std::filesystem::canonical(targetPath);
+
+    return writeData(QLatin1String(canonicalTargetPath.c_str()), buffer, targetOffset);
 }
 
 QVariantMap ExternalCommandHelper::RunCommand(const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode)
@@ -334,6 +354,11 @@ QVariantMap ExternalCommandHelper::RunCommand(const QString& command, const QStr
 
     QProcess cmd;
     cmd.setEnvironment( { QStringLiteral("LVM_SUPPRESS_FD_WARNINGS=1") } );
+
+    if((processChannelMode != QProcess::SeparateChannels) && (processChannelMode != QProcess::MergedChannels)) {
+        reply[QStringLiteral("success")] = false;
+        return reply;
+    }
     cmd.setProcessChannelMode(static_cast<QProcess::ProcessChannelMode>(processChannelMode));
     cmd.start(command, arguments);
     cmd.write(input);

src/util/externalcommandhelper.h

@@ -38,7 +38,7 @@ public:
 
 public Q_SLOTS:
     Q_SCRIPTABLE QVariantMap RunCommand(const QString& command, const QStringList& arguments, const QByteArray& input, const int processChannelMode);
-    Q_SCRIPTABLE QVariantMap CopyBlocks(const QString& sourceDevice, const qint64 sourceOffset, const qint64 sourceLength,
+    Q_SCRIPTABLE QVariantMap CopyFileData(const QString& sourceDevice, const qint64 sourceOffset, const qint64 sourceLength,
                                         const QString& targetDevice, const qint64 targetOffset, const qint64 blockSize);
     Q_SCRIPTABLE QByteArray ReadData(const QString& device, const qint64 offset, const qint64 length);
     Q_SCRIPTABLE bool WriteData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetOffset);