From 71554140bdaede27b95dbe4c9b5a028a83c83cce Mon Sep 17 00:00:00 2001
From: Alexander Reinholdt <alexander.reinholdt@kdemail.net>
Date: Wed, 10 May 2017 10:23:34 +0200
Subject: Find the mount/umount commands in the helper
Instead of trusting what we get passed in
CVE-2017-8849
---
core/smb4kglobal.cpp | 65 +++++++++++++++++++++++++++++++++++-
core/smb4kglobal.h | 16 ++++++++-
core/smb4kmounter_p.cpp | 78 ++++----------------------------------------
helpers/CMakeLists.txt | 6 +++-
helpers/smb4kmounthelper.cpp | 51 +++++++++++++++++++++++++++--
5 files changed, 139 insertions(+), 77 deletions(-)
diff --git a/core/smb4kglobal.cpp b/core/smb4kglobal.cpp
index 172016f..818a78a 100644
--- a/core/smb4kglobal.cpp
+++ b/core/smb4kglobal.cpp
@@ -2,7 +2,7 @@
smb4kglobal - This is the global namespace for Smb4K.
-------------------
begin : Sa Apr 2 2005
- copyright : (C) 2005-2014 by Alexander Reinholdt
+ copyright : (C) 2005-2017 by Alexander Reinholdt
email : alexander.reinholdt@kdemail.net
***************************************************************************/
@@ -851,3 +851,66 @@ QStringList Smb4KGlobal::whitelistedMountArguments()
#endif
+const QString Smb4KGlobal::findMountExecutable()
+{
+ QString mount;
+ QStringList paths;
+ paths << "/bin";
+ paths << "/sbin";
+ paths << "/usr/bin";
+ paths << "/usr/sbin";
+ paths << "/usr/local/bin";
+ paths << "/usr/local/sbin";
+
+ for (int i = 0; i < paths.size(); ++i)
+ {
+#if defined(Q_OS_LINUX)
+ mount = KGlobal::dirs()->findExe("mount.cifs", paths.at(i));
+#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)
+ mount = KGlobal::dirs()->findExe("mount_smbfs", paths.at(i));
+#endif
+
+ if (!mount.isEmpty())
+ {
+ break;
+ }
+ else
+ {
+ continue;
+ }
+ }
+
+ return mount;
+}
+
+
+const QString Smb4KGlobal::findUmountExecutable()
+{
+ // Find the umount program.
+ QString umount;
+ QStringList paths;
+ paths << "/bin";
+ paths << "/sbin";
+ paths << "/usr/bin";
+ paths << "/usr/sbin";
+ paths << "/usr/local/bin";
+ paths << "/usr/local/sbin";
+
+ for ( int i = 0; i < paths.size(); ++i )
+ {
+ umount = KGlobal::dirs()->findExe("umount", paths.at(i));
+
+ if (!umount.isEmpty())
+ {
+ break;
+ }
+ else
+ {
+ continue;
+ }
+ }
+
+ return umount;
+}
+
+
diff --git a/core/smb4kglobal.h b/core/smb4kglobal.h
index db1805b..0ef377d 100644
--- a/core/smb4kglobal.h
+++ b/core/smb4kglobal.h
@@ -2,7 +2,7 @@
smb4kglobal - This is the global namespace for Smb4K.
-------------------
begin : Sa Apr 2 2005
- copyright : (C) 2005-2014 by Alexander Reinholdt
+ copyright : (C) 2005-2017 by Alexander Reinholdt
email : alexander.reinholdt@kdemail.net
***************************************************************************/
@@ -455,6 +455,20 @@ namespace Smb4KGlobal
*/
KDE_EXPORT QStringList whitelistedMountArguments();
#endif
+
+ /**
+ * Find the mount executable on the system.
+ *
+ * @returns the path of the mount executable.
+ */
+ KDE_EXPORT const QString findMountExecutable();
+
+ /**
+ * Find the umount executable on the system.
+ *
+ * @returns the path of the umount executable.
+ */
+ KDE_EXPORT const QString findUmountExecutable();
};
#endif
diff --git a/core/smb4kmounter_p.cpp b/core/smb4kmounter_p.cpp
index 63a87ed..342052a 100644
--- a/core/smb4kmounter_p.cpp
+++ b/core/smb4kmounter_p.cpp
@@ -207,30 +207,7 @@ bool Smb4KMountJob::createMountAction(Smb4KShare *share, Action *action)
//
bool Smb4KMountJob::fillArgs(Smb4KShare *share, QMap<QString, QVariant>& 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";
-
- for (int i = 0; i < paths.size(); ++i)
- {
- mount = KGlobal::dirs()->findExe("mount.cifs", paths.at(i));
-
- if (!mount.isEmpty())
- {
- map.insert("mh_command", mount);
- break;
- }
- else
- {
- continue;
- }
- }
+ const QString mount = findMountExecutable();
if (mount.isEmpty())
{
@@ -242,6 +219,8 @@ bool Smb4KMountJob::fillArgs(Smb4KShare *share, QMap<QString, QVariant>& map)
// Do nothing
}
+ map.insert("mh_command", mount);
+
// Mount arguments.
QMap<QString, QString> global_options = globalSambaOptions();
Smb4KCustomOptions *options = Smb4KCustomOptionsManager::self()->findOptions(share);
@@ -729,30 +708,7 @@ bool Smb4KMountJob::fillArgs(Smb4KShare *share, QMap<QString, QVariant>& map)
//
bool Smb4KMountJob::fillArgs(Smb4KShare *share, QMap<QString, QVariant>& 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";
-
- for (int i = 0; i < paths.size(); ++i)
- {
- mount = KGlobal::dirs()->findExe("mount_smbfs", paths.at(i));
-
- if (!mount.isEmpty())
- {
- map.insert("mh_command", mount);
- break;
- }
- else
- {
- continue;
- }
- }
+ const QString mount = findMountExecutable();
if (mount.isEmpty())
{
@@ -764,6 +720,8 @@ bool Smb4KMountJob::fillArgs(Smb4KShare *share, QMap<QString, QVariant>& map)
// Do nothing
}
+ map.insert("mh_command", mount);
+
// Mount arguments.
QMap<QString, QString> global_options = globalSambaOptions();
Smb4KCustomOptions *options = Smb4KCustomOptionsManager::self()->findOptions(share);
@@ -1253,29 +1211,7 @@ bool Smb4KUnmountJob::createUnmountAction(Smb4KShare *share, Action *action)
// Do nothing
}
- // Find the umount program.
- QString umount;
- QStringList paths;
- paths << "/bin";
- paths << "/sbin";
- paths << "/usr/bin";
- paths << "/usr/sbin";
- paths << "/usr/local/bin";
- paths << "/usr/local/sbin";
-
- for ( int i = 0; i < paths.size(); ++i )
- {
- umount = KGlobal::dirs()->findExe("umount", paths.at(i));
-
- if (!umount.isEmpty())
- {
- break;
- }
- else
- {
- continue;
- }
- }
+ const QString umount = findUmountExecutable();
if (umount.isEmpty() && !m_silent)
{
diff --git a/helpers/CMakeLists.txt b/helpers/CMakeLists.txt
index e9e670b..cd4228d 100644
--- a/helpers/CMakeLists.txt
+++ b/helpers/CMakeLists.txt
@@ -1,7 +1,11 @@
+include_directories(
+ ${CMAKE_SOURCE_DIR}/core
+ ${CMAKE_BINARY_DIR}/core )
+
set( smb4kmounthelper_SRCS smb4kmounthelper.cpp )
kde4_add_executable( mounthelper ${smb4kmounthelper_SRCS} )
-target_link_libraries( mounthelper ${KDE4_KDECORE_LIBS} ${KDE4_KIO_LIBS} )
+target_link_libraries( mounthelper smb4kcore ${KDE4_KDECORE_LIBS} ${KDE4_KIO_LIBS} )
install( TARGETS mounthelper DESTINATION ${LIBEXEC_INSTALL_DIR} )
kde4_install_auth_helper_files( mounthelper net.sourceforge.smb4k.mounthelper root )
diff --git a/helpers/smb4kmounthelper.cpp b/helpers/smb4kmounthelper.cpp
index a2f2fed..7959020 100644
--- a/helpers/smb4kmounthelper.cpp
+++ b/helpers/smb4kmounthelper.cpp
@@ -29,6 +29,7 @@
// application specific includes
#include "smb4kmounthelper.h"
+#include "core/smb4kglobal.h"
// Qt includes
#include <QProcessEnvironment>
@@ -43,12 +44,35 @@
#include <kmountpoint.h>
#include <kurl.h>
+using namespace Smb4KGlobal;
+
KDE4_AUTH_HELPER_MAIN( "net.sourceforge.smb4k.mounthelper", Smb4KMountHelper )
ActionReply Smb4KMountHelper::mount(const QVariantMap &args)
{
ActionReply reply;
+
+ //
+ // Get the mount executable
+ //
+ const QString mount = findMountExecutable();
+
+ //
+ // Check the executable
+ //
+ if (mount != args["mh_command"].toString())
+ {
+ // Something weird is going on, bail out.
+ reply.setErrorCode(ActionReply::HelperError);
+ reply.setErrorDescription(i18n("Wrong executable passed. Bailing out."));
+ return reply;
+ }
+ else
+ {
+ // Do nothing
+ }
+
// The mountpoint is a unique and can be used to
// find the share.
reply.addData("mh_mountpoint", args["mh_mountpoint"]);
@@ -75,12 +99,12 @@ ActionReply Smb4KMountHelper::mount(const QVariantMap &args)
// Set the mount command here.
QStringList command;
#if defined(Q_OS_LINUX)
- command << args["mh_command"].toString();
+ command << mount;
command << args["mh_unc"].toString();
command << args["mh_mountpoint"].toString();
command << args["mh_options"].toStringList();
#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)
- command << args["mh_command"].toString();
+ command << mount;
command << args["mh_options"].toStringList();
command << args["mh_unc"].toString();
command << args["mh_mountpoint"].toString();
@@ -161,6 +185,27 @@ ActionReply Smb4KMountHelper::mount(const QVariantMap &args)
ActionReply Smb4KMountHelper::unmount(const QVariantMap &args)
{
ActionReply reply;
+
+ //
+ // Get the umount executable
+ //
+ const QString umount = findUmountExecutable();
+
+ //
+ // Check the executable
+ //
+ if (umount != args["mh_command"].toString())
+ {
+ // Something weird is going on, bail out.
+ reply.setErrorCode(ActionReply::HelperError);
+ reply.setErrorDescription(i18n("Wrong executable passed. Bailing out."));
+ return reply;
+ }
+ else
+ {
+ // Do nothing
+ }
+
// The mountpoint is a unique and can be used to
// find the share.
reply.addData("mh_mountpoint", args["mh_mountpoint"]);
@@ -208,7 +253,7 @@ ActionReply Smb4KMountHelper::unmount(const QVariantMap &args)
// Set the umount command here.
QStringList command;
- command << args["mh_command"].toString();
+ command << umount;
command << args["mh_options"].toStringList();
command << args["mh_mountpoint"].toString();
--
cgit v0.11.2
|
