Commit a90289b0 authored by Albert Astals Cid's avatar Albert Astals Cid
Browse files

Find the mount/umount commands in the helper

Instead of trusting what we get passed in

CVE-2017-8849
parent dbc69cd6
......@@ -864,3 +864,35 @@ QStringList Smb4KGlobal::whitelistedMountArguments()
#endif
const QString Smb4KGlobal::findMountExecutable()
{
QStringList paths;
paths << "/bin";
paths << "/sbin";
paths << "/usr/bin";
paths << "/usr/sbin";
paths << "/usr/local/bin";
paths << "/usr/local/sbin";
#if defined(Q_OS_LINUX)
return QStandardPaths::findExecutable("mount.cifs", paths);
#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)
return QStandardPaths::findExecutable("mount_smbfs", paths);
#else
return QString();
#endif
}
const QString Smb4KGlobal::findUmountExecutable()
{
QStringList paths;
paths << "/bin";
paths << "/sbin";
paths << "/usr/bin";
paths << "/usr/sbin";
paths << "/usr/local/bin";
paths << "/usr/local/sbin";
return QStandardPaths::findExecutable("umount", paths);
}
......@@ -454,6 +454,20 @@ namespace Smb4KGlobal
*/
Q_DECL_EXPORT QStringList whitelistedMountArguments();
#endif
/**
* Find the mount executable on the system.
*
* @returns the path of the mount executable.
*/
Q_DECL_EXPORT const QString findMountExecutable();
/**
* Find the umount executable on the system.
*
* @returns the path of the umount executable.
*/
Q_DECL_EXPORT const QString findUmountExecutable();
};
#endif
......@@ -1104,16 +1104,7 @@ void Smb4KMounter::timerEvent(QTimerEvent *)
bool Smb4KMounter::fillMountActionArgs(Smb4KShare *share, QVariantMap& map)
{
// Find the mount program.
QString mount;
QStringList paths;
paths << "/bin";
paths << "/sbin";
paths << "/usr/bin";
paths << "/usr/sbin";
paths << "/usr/local/bin";
paths << "/usr/local/sbin";
mount = QStandardPaths::findExecutable("mount.cifs", paths);
const QString mount = findMountExecutable();
if (!mount.isEmpty())
{
......@@ -1645,16 +1636,7 @@ bool Smb4KMounter::fillMountActionArgs(Smb4KShare *share, QVariantMap& map)
bool Smb4KMounter::fillMountActionArgs(Smb4KShare *share, QVariantMap& map)
{
// Find the mount program.
QString mount;
QStringList paths;
paths << "/bin";
paths << "/sbin";
paths << "/usr/bin";
paths << "/usr/sbin";
paths << "/usr/local/bin";
paths << "/usr/local/sbin";
mount = QStandardPaths::findExecutable("mount_smbfs", paths);
const QString mount = findMountExecutable();
if (!mount.isEmpty())
{
......@@ -1823,15 +1805,7 @@ bool Smb4KMounter::fillUnmountActionArgs(Smb4KShare *share, bool force, bool sil
//
// The umount program
//
QStringList paths;
paths << "/bin";
paths << "/sbin";
paths << "/usr/bin";
paths << "/usr/sbin";
paths << "/usr/local/bin";
paths << "/usr/local/sbin";
QString umount = QStandardPaths::findExecutable("umount", paths);
const QString umount = findUmountExecutable();
if (umount.isEmpty() && !silent)
{
......@@ -1884,15 +1858,7 @@ bool Smb4KMounter::fillUnmountActionArgs(Smb4KShare *share, bool force, bool sil
//
// The umount program
//
QStringList paths;
paths << "/bin";
paths << "/sbin";
paths << "/usr/bin";
paths << "/usr/sbin";
paths << "/usr/local/bin";
paths << "/usr/local/sbin";
QString umount = QStandardPaths::findExecutable("umount", paths);
const QString umount = findUmountExecutable();
if (umount.isEmpty() && !silent)
{
......
include_directories(
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_BINARY_DIR}
${CMAKE_SOURCE_DIR}/core
${CMAKE_BINARY_DIR}/core)
set(smb4kmounthelper_SRCS smb4kmounthelper.cpp)
add_executable(mounthelper ${smb4kmounthelper_SRCS})
target_link_libraries(mounthelper
smb4kcore
Qt5::Core
KF5::Auth
KF5::CoreAddons
......
......@@ -2,7 +2,7 @@
The helper that mounts and unmounts shares.
-------------------
begin : Sa Okt 16 2010
copyright : (C) 2010-2016 by Alexander Reinholdt
copyright : (C) 2010-2017 by Alexander Reinholdt
email : alexander.reinholdt@kdemail.net
***************************************************************************/
......@@ -29,6 +29,7 @@
// application specific includes
#include "smb4kmounthelper.h"
#include "../core/smb4kglobal.h"
// Qt includes
#include <QProcessEnvironment>
......@@ -42,13 +43,23 @@
#include <KI18n/KLocalizedString>
#include <KIOCore/KMountPoint>
using namespace Smb4KGlobal;
KAUTH_HELPER_MAIN("org.kde.smb4k.mounthelper", Smb4KMountHelper);
ActionReply Smb4KMountHelper::mount(const QVariantMap &args)
{
//
// The action reply
//
ActionReply reply;
//
// Get the mount executable
//
const QString mount = findMountExecutable();
//
// Iterate through the entries.
//
......@@ -60,6 +71,20 @@ ActionReply Smb4KMountHelper::mount(const QVariantMap &args)
QString index = it.key();
QVariantMap entry = it.value().toMap();
//
// Check the executable
//
if (mount != entry["mh_command"].toString())
{
// Something weird is going on, bail out.
reply.setType(ActionReply::HelperErrorType);
return reply;
}
else
{
// Do nothing
}
//
// The process
//
......@@ -87,12 +112,12 @@ ActionReply Smb4KMountHelper::mount(const QVariantMap &args)
//
QStringList command;
#if defined(Q_OS_LINUX)
command << entry["mh_command"].toString();
command << mount;
command << entry["mh_unc"].toString();
command << entry["mh_mountpoint"].toString();
command << entry["mh_options"].toStringList();
#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)
command << entry["mh_command"].toString();
command << mount;
command << entry["mh_options"].toStringList();
command << entry["mh_unc"].toString();
command << entry["mh_mountpoint"].toString();
......@@ -207,6 +232,11 @@ ActionReply Smb4KMountHelper::unmountOneByOne(const QVariantMap& args)
//
ActionReply reply;
//
// Get the mount executable
//
const QString umount = findUmountExecutable();
//
// Iterate through the entries.
//
......@@ -217,6 +247,20 @@ ActionReply Smb4KMountHelper::unmountOneByOne(const QVariantMap& args)
it.next();
QString index = it.key();
QVariantMap entry = it.value().toMap();
//
// Check the executable
//
if (umount != entry["mh_command"].toString())
{
// Something weird is going on, bail out.
reply.setType(ActionReply::HelperErrorType);
return reply;
}
else
{
// Do nothing
}
//
// Check if the mountpoint is valid and the filesystem is correct.
......@@ -261,7 +305,7 @@ ActionReply Smb4KMountHelper::unmountOneByOne(const QVariantMap& args)
// The command
//
QStringList command;
command << entry["mh_command"].toString();
command << umount;
command << entry["mh_options"].toStringList();
command << entry["mh_mountpoint"].toString();
......@@ -341,6 +385,11 @@ ActionReply Smb4KMountHelper::unmountAtOnce(const QVariantMap& args)
//
ActionReply reply;
//
// Get the mount executable
//
const QString umount = findUmountExecutable();
//
// Check the mountpoints and put the valid ones into a string list
//
......@@ -351,7 +400,7 @@ ActionReply Smb4KMountHelper::unmountAtOnce(const QVariantMap& args)
{
it.next();
QVariantMap entry = it.value().toMap();
bool mountPointOk = false;
KMountPoint::List mountPoints = KMountPoint::currentMountPoints(KMountPoint::BasicInfoNeeded|KMountPoint::NeedMountOptions);
......@@ -399,9 +448,23 @@ ActionReply Smb4KMountHelper::unmountAtOnce(const QVariantMap& args)
if (!validMountPoints.isEmpty())
{
//
// Check the executable
//
if (umount != args.first().toMap().value("mh_command").toString())
{
// Something weird is going on, bail output
reply.setType(ActionReply::HelperErrorType);
return reply;
}
else
{
// Do nothing
}
// The command
QStringList command;
command << args.first().toMap().value("mh_command").toString();
command << umount;
command << args.first().toMap().value("mh_options").toStringList();
command << validMountPoints;
......
......@@ -2,7 +2,7 @@
The helper that mounts and unmounts shares.
-------------------
begin : Sa Okt 16 2010
copyright : (C) 2010-2016 by Alexander Reinholdt
copyright : (C) 2010-2017 by Alexander Reinholdt
email : alexander.reinholdt@kdemail.net
***************************************************************************/
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment